Title: [119272] trunk/Source/WebCore
Revision
119272
Author
jchaffr...@webkit.org
Date
2012-06-01 13:15:01 -0700 (Fri, 01 Jun 2012)

Log Message

Prepare table collapsed border computation to support mixed directionality on row group
https://bugs.webkit.org/show_bug.cgi?id=88110

Reviewed by Ojan Vafai.

No expected change in behavior.

One big issue with supporting mixed directionality inside a table is that the start / end
borders don't align between table parts anymore: the start border of a ltr table will have
to match the end border of a rtl row group for the purpose of collapsed border computation.

This change adds the concept of adjoining borders in the table direction so that we can safely
hide which exact borders we pick up for the collapsed border computation.

* rendering/RenderTable.cpp:
(WebCore::RenderTable::calcBorderStart):
(WebCore::RenderTable::calcBorderEnd):
Refactored those functions to use proper naming along with the new APIs. The name 'adjoining' is
used extensively as we cannot make any assumptions on which borders we will get.

(WebCore::RenderTable::recalcBordersInRowDirection):
Added a FIXME found during testing.

* rendering/RenderTableCell.h:
(WebCore::RenderTableCell::borderAdjoiningTableStart):
(WebCore::RenderTableCell::borderAdjoiningTableEnd):
* rendering/RenderTableRow.h:
(WebCore::RenderTableRow::borderAdjoiningTableStart):
(WebCore::RenderTableRow::borderAdjoiningTableEnd):
* rendering/RenderTableSection.h:
(WebCore::RenderTableSection::borderAdjoiningTableStart):
(WebCore::RenderTableSection::borderAdjoiningTableEnd):
Those functions are the same at the moment to match the existing code. They
will be changed to use the proper directionality in a follow up patch.

* rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::firstRowCellAdjoiningTableStart):
(WebCore::RenderTableSection::firstRowCellAdjoiningTableEnd):
Those functions return the cells that is adjoining a table edge. Due to us flipping
the cells at layout to match the section's direction, those functions will need to
account for mixed direction in determining the right cell to consider.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (119271 => 119272)


--- trunk/Source/WebCore/ChangeLog	2012-06-01 20:13:15 UTC (rev 119271)
+++ trunk/Source/WebCore/ChangeLog	2012-06-01 20:15:01 UTC (rev 119272)
@@ -1,3 +1,47 @@
+2012-06-01  Julien Chaffraix  <jchaffr...@webkit.org>
+
+        Prepare table collapsed border computation to support mixed directionality on row group
+        https://bugs.webkit.org/show_bug.cgi?id=88110
+
+        Reviewed by Ojan Vafai.
+
+        No expected change in behavior.
+
+        One big issue with supporting mixed directionality inside a table is that the start / end
+        borders don't align between table parts anymore: the start border of a ltr table will have
+        to match the end border of a rtl row group for the purpose of collapsed border computation.
+
+        This change adds the concept of adjoining borders in the table direction so that we can safely
+        hide which exact borders we pick up for the collapsed border computation.
+
+        * rendering/RenderTable.cpp:
+        (WebCore::RenderTable::calcBorderStart):
+        (WebCore::RenderTable::calcBorderEnd):
+        Refactored those functions to use proper naming along with the new APIs. The name 'adjoining' is
+        used extensively as we cannot make any assumptions on which borders we will get.
+
+        (WebCore::RenderTable::recalcBordersInRowDirection):
+        Added a FIXME found during testing.
+
+        * rendering/RenderTableCell.h:
+        (WebCore::RenderTableCell::borderAdjoiningTableStart):
+        (WebCore::RenderTableCell::borderAdjoiningTableEnd):
+        * rendering/RenderTableRow.h:
+        (WebCore::RenderTableRow::borderAdjoiningTableStart):
+        (WebCore::RenderTableRow::borderAdjoiningTableEnd):
+        * rendering/RenderTableSection.h:
+        (WebCore::RenderTableSection::borderAdjoiningTableStart):
+        (WebCore::RenderTableSection::borderAdjoiningTableEnd):
+        Those functions are the same at the moment to match the existing code. They
+        will be changed to use the proper directionality in a follow up patch.
+
+        * rendering/RenderTableSection.cpp:
+        (WebCore::RenderTableSection::firstRowCellAdjoiningTableStart):
+        (WebCore::RenderTableSection::firstRowCellAdjoiningTableEnd):
+        Those functions return the cells that is adjoining a table edge. Due to us flipping
+        the cells at layout to match the section's direction, those functions will need to
+        account for mixed direction in determining the right cell to consider.
+
 2012-06-01  Shezan Baig  <shezbaig...@gmail.com>
 
         Indenting a paragraph that begins with a link 3 times breaks the paragraph into two paragraphs

Modified: trunk/Source/WebCore/rendering/RenderTable.cpp (119271 => 119272)


--- trunk/Source/WebCore/rendering/RenderTable.cpp	2012-06-01 20:13:15 UTC (rev 119271)
+++ trunk/Source/WebCore/rendering/RenderTable.cpp	2012-06-01 20:15:01 UTC (rev 119272)
@@ -842,113 +842,116 @@
 
 int RenderTable::calcBorderStart() const
 {
-    if (collapseBorders()) {
-        // Determined by the first cell of the first row. See the CSS 2.1 spec, section 17.6.2.
-        if (!numEffCols())
-            return 0;
+    if (!collapseBorders())
+        return RenderBlock::borderStart();
 
-        unsigned borderWidth = 0;
+    // Determined by the first cell of the first row. See the CSS 2.1 spec, section 17.6.2.
+    if (!numEffCols())
+        return 0;
 
-        const BorderValue& tb = style()->borderStart();
-        if (tb.style() == BHIDDEN)
+    unsigned borderWidth = 0;
+
+    const BorderValue& tableStartBorder = style()->borderStart();
+    if (tableStartBorder.style() == BHIDDEN)
+        return 0;
+    if (tableStartBorder.style() > BHIDDEN)
+        borderWidth = tableStartBorder.width();
+
+    if (RenderTableCol* column = colElement(0)) {
+        // FIXME: We don't account for direction on columns and column groups.
+        const BorderValue& columnAdjoiningBorder = column->style()->borderStart();
+        if (columnAdjoiningBorder.style() == BHIDDEN)
             return 0;
-        if (tb.style() > BHIDDEN)
-            borderWidth = tb.width();
+        if (columnAdjoiningBorder.style() > BHIDDEN)
+            borderWidth = max(borderWidth, columnAdjoiningBorder.width());
+        // FIXME: This logic doesn't properly account for the first column in the first column-group case.
+    }
 
-        if (RenderTableCol* colGroup = colElement(0)) {
-            const BorderValue& gb = colGroup->style()->borderStart();
-            if (gb.style() == BHIDDEN)
-                return 0;
-            if (gb.style() > BHIDDEN)
-                borderWidth = max(borderWidth, gb.width());
-        }
-        
-        if (const RenderTableSection* topNonEmptySection = this->topNonEmptySection()) {
-            const BorderValue& sb = topNonEmptySection->style()->borderStart();
-            if (sb.style() == BHIDDEN)
-                return 0;
+    if (const RenderTableSection* topNonEmptySection = this->topNonEmptySection()) {
+        const BorderValue& sectionAdjoiningBorder = topNonEmptySection->borderAdjoiningTableStart();
+        if (sectionAdjoiningBorder.style() == BHIDDEN)
+            return 0;
 
-            if (sb.style() > BHIDDEN)
-                borderWidth = max(borderWidth, sb.width());
+        if (sectionAdjoiningBorder.style() > BHIDDEN)
+            borderWidth = max(borderWidth, sectionAdjoiningBorder.width());
 
-            const RenderTableSection::CellStruct& cs = topNonEmptySection->cellAt(0, 0);
-            
-            if (cs.hasCells()) {
-                const BorderValue& cb = cs.primaryCell()->style()->borderStart(); // FIXME: Make this work with perpendicualr and flipped cells.
-                if (cb.style() == BHIDDEN)
-                    return 0;
+        if (const RenderTableCell* adjoiningStartCell = topNonEmptySection->firstRowCellAdjoiningTableStart()) {
+            // FIXME: Make this work with perpendicular and flipped cells.
+            const BorderValue& startCellAdjoiningBorder = adjoiningStartCell->borderAdjoiningTableStart();
+            if (startCellAdjoiningBorder.style() == BHIDDEN)
+                return 0;
 
-                const BorderValue& rb = cs.primaryCell()->parent()->style()->borderStart();
-                if (rb.style() == BHIDDEN)
-                    return 0;
+            const BorderValue& firstRowAdjoiningBorder = adjoiningStartCell->row()->borderAdjoiningTableStart();
+            if (firstRowAdjoiningBorder.style() == BHIDDEN)
+                return 0;
 
-                if (cb.style() > BHIDDEN)
-                    borderWidth = max(borderWidth, cb.width());
-                if (rb.style() > BHIDDEN)
-                    borderWidth = max(borderWidth, rb.width());
-            }
+            if (startCellAdjoiningBorder.style() > BHIDDEN)
+                borderWidth = max(borderWidth, startCellAdjoiningBorder.width());
+            if (firstRowAdjoiningBorder.style() > BHIDDEN)
+                borderWidth = max(borderWidth, firstRowAdjoiningBorder.width());
         }
-        return (borderWidth + (style()->isLeftToRightDirection() ? 0 : 1)) / 2;
     }
-    return RenderBlock::borderStart();
+    return (borderWidth + (style()->isLeftToRightDirection() ? 0 : 1)) / 2;
 }
 
 int RenderTable::calcBorderEnd() const
 {
-    if (collapseBorders()) {
-        // Determined by the last cell of the first row. See the CSS 2.1 spec, section 17.6.2.
-        if (!numEffCols())
-            return 0;
+    if (!collapseBorders())
+        return RenderBlock::borderEnd();
 
-        unsigned borderWidth = 0;
+    // Determined by the last cell of the first row. See the CSS 2.1 spec, section 17.6.2.
+    if (!numEffCols())
+        return 0;
 
-        const BorderValue& tb = style()->borderEnd();
-        if (tb.style() == BHIDDEN)
+    unsigned borderWidth = 0;
+
+    const BorderValue& tableEndBorder = style()->borderEnd();
+    if (tableEndBorder.style() == BHIDDEN)
+        return 0;
+    if (tableEndBorder.style() > BHIDDEN)
+        borderWidth = tableEndBorder.width();
+
+    unsigned endColumn = numEffCols() - 1;
+    if (RenderTableCol* column = colElement(endColumn)) {
+        // FIXME: We don't account for direction on columns and column groups.
+        const BorderValue& columnAdjoiningBorder = column->style()->borderEnd();
+        if (columnAdjoiningBorder.style() == BHIDDEN)
             return 0;
-        if (tb.style() > BHIDDEN)
-            borderWidth = tb.width();
+        if (columnAdjoiningBorder.style() > BHIDDEN)
+            borderWidth = max(borderWidth, columnAdjoiningBorder.width());
+        // FIXME: This logic doesn't properly account for the last column in the last column-group case.
+    }
 
-        unsigned endColumn = numEffCols() - 1;
-        if (RenderTableCol* colGroup = colElement(endColumn)) {
-            const BorderValue& gb = colGroup->style()->borderEnd();
-            if (gb.style() == BHIDDEN)
+    if (const RenderTableSection* topNonEmptySection = this->topNonEmptySection()) {
+        const BorderValue& sectionAdjoiningBorder = topNonEmptySection->borderAdjoiningTableEnd();
+        if (sectionAdjoiningBorder.style() == BHIDDEN)
+            return 0;
+
+        if (sectionAdjoiningBorder.style() > BHIDDEN)
+            borderWidth = max(borderWidth, sectionAdjoiningBorder.width());
+
+        if (const RenderTableCell* adjoiningEndCell = topNonEmptySection->firstRowCellAdjoiningTableEnd()) {
+            // FIXME: Make this work with perpendicular and flipped cells.
+            const BorderValue& endCellAdjoiningBorder = adjoiningEndCell->borderAdjoiningTableEnd();
+            if (endCellAdjoiningBorder.style() == BHIDDEN)
                 return 0;
-            if (gb.style() > BHIDDEN)
-                borderWidth = max(borderWidth, gb.width());
-        }
 
-        if (const RenderTableSection* topNonEmptySection = this->topNonEmptySection()) {
-            const BorderValue& sb = topNonEmptySection->style()->borderEnd();
-            if (sb.style() == BHIDDEN)
+            const BorderValue& firstRowAdjoiningBorder = adjoiningEndCell->row()->borderAdjoiningTableEnd();
+            if (firstRowAdjoiningBorder.style() == BHIDDEN)
                 return 0;
 
-            if (sb.style() > BHIDDEN)
-                borderWidth = max(borderWidth, sb.width());
-
-            const RenderTableSection::CellStruct& cs = topNonEmptySection->cellAt(0, endColumn);
-            
-            if (cs.hasCells()) {
-                const BorderValue& cb = cs.primaryCell()->style()->borderEnd(); // FIXME: Make this work with perpendicular and flipped cells.
-                if (cb.style() == BHIDDEN)
-                    return 0;
-
-                const BorderValue& rb = cs.primaryCell()->parent()->style()->borderEnd();
-                if (rb.style() == BHIDDEN)
-                    return 0;
-
-                if (cb.style() > BHIDDEN)
-                    borderWidth = max(borderWidth, cb.width());
-                if (rb.style() > BHIDDEN)
-                    borderWidth = max(borderWidth, rb.width());
-            }
+            if (endCellAdjoiningBorder.style() > BHIDDEN)
+                borderWidth = max(borderWidth, endCellAdjoiningBorder.width());
+            if (firstRowAdjoiningBorder.style() > BHIDDEN)
+                borderWidth = max(borderWidth, firstRowAdjoiningBorder.width());
         }
-        return (borderWidth + (style()->isLeftToRightDirection() ? 1 : 0)) / 2;
     }
-    return RenderBlock::borderEnd();
+    return (borderWidth + (style()->isLeftToRightDirection() ? 1 : 0)) / 2;
 }
 
 void RenderTable::recalcBordersInRowDirection()
 {
+    // FIXME: We need to compute the collapsed before / after borders in the same fashion.
     m_borderStart = calcBorderStart();
     m_borderEnd = calcBorderEnd();
 }

Modified: trunk/Source/WebCore/rendering/RenderTableCell.h (119271 => 119272)


--- trunk/Source/WebCore/rendering/RenderTableCell.h	2012-06-01 20:13:15 UTC (rev 119271)
+++ trunk/Source/WebCore/rendering/RenderTableCell.h	2012-06-01 20:15:01 UTC (rev 119272)
@@ -145,6 +145,16 @@
         return table()->style();
     }
 
+    const BorderValue& borderAdjoiningTableStart() const
+    {
+        return style()->borderStart();
+    }
+
+    const BorderValue& borderAdjoiningTableEnd() const
+    {
+        return style()->borderEnd();
+    }
+
 protected:
     virtual void styleDidChange(StyleDifference, const RenderStyle* oldStyle);
 

Modified: trunk/Source/WebCore/rendering/RenderTableRow.h (119271 => 119272)


--- trunk/Source/WebCore/rendering/RenderTableRow.h	2012-06-01 20:13:15 UTC (rev 119271)
+++ trunk/Source/WebCore/rendering/RenderTableRow.h	2012-06-01 20:15:01 UTC (rev 119272)
@@ -66,6 +66,16 @@
         return m_rowIndex;
     }
 
+    const BorderValue& borderAdjoiningTableStart() const
+    {
+        return style()->borderStart();
+    }
+
+    const BorderValue& borderAdjoiningTableEnd() const
+    {
+        return style()->borderEnd();
+    }
+
 private:
     virtual RenderObjectChildList* virtualChildren() { return children(); }
     virtual const RenderObjectChildList* virtualChildren() const { return children(); }

Modified: trunk/Source/WebCore/rendering/RenderTableSection.cpp (119271 => 119272)


--- trunk/Source/WebCore/rendering/RenderTableSection.cpp	2012-06-01 20:13:15 UTC (rev 119271)
+++ trunk/Source/WebCore/rendering/RenderTableSection.cpp	2012-06-01 20:15:01 UTC (rev 119272)
@@ -1276,6 +1276,17 @@
     return result + 1;
 }
 
+const RenderTableCell* RenderTableSection::firstRowCellAdjoiningTableStart() const
+{
+    return cellAt(0, 0).primaryCell();
+}
+
+const RenderTableCell* RenderTableSection::firstRowCellAdjoiningTableEnd() const
+{
+    return cellAt(0, table()->numEffCols() - 1).primaryCell();
+}
+
+
 void RenderTableSection::appendColumn(unsigned pos)
 {
     ASSERT(!m_needsCellRecalc);

Modified: trunk/Source/WebCore/rendering/RenderTableSection.h (119271 => 119272)


--- trunk/Source/WebCore/rendering/RenderTableSection.h	2012-06-01 20:13:15 UTC (rev 119271)
+++ trunk/Source/WebCore/rendering/RenderTableSection.h	2012-06-01 20:15:01 UTC (rev 119272)
@@ -114,6 +114,19 @@
         Length logicalHeight;
     };
 
+    const BorderValue& borderAdjoiningTableStart() const
+    {
+        return style()->borderStart();
+    }
+
+    const BorderValue& borderAdjoiningTableEnd() const
+    {
+        return style()->borderEnd();
+    }
+
+    const RenderTableCell* firstRowCellAdjoiningTableStart() const;
+    const RenderTableCell* firstRowCellAdjoiningTableEnd() const;
+
     CellStruct& cellAt(unsigned row,  unsigned col) { return m_grid[row].row[col]; }
     const CellStruct& cellAt(unsigned row, unsigned col) const { return m_grid[row].row[col]; }
     RenderTableCell* primaryCellAt(unsigned row, unsigned col)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to