Diff
Modified: trunk/LayoutTests/ChangeLog (133438 => 133439)
--- trunk/LayoutTests/ChangeLog 2012-11-05 07:52:52 UTC (rev 133438)
+++ trunk/LayoutTests/ChangeLog 2012-11-05 07:55:28 UTC (rev 133439)
@@ -1,3 +1,15 @@
+2012-11-04 Julien Chaffraix <jchaffr...@webkit.org>
+
+ Fix the collapsing border code to handle mixed directionality at the row level
+ https://bugs.webkit.org/show_bug.cgi?id=101060
+
+ Reviewed by Ojan Vafai.
+
+ * fast/table/border-collapsing/table-ltr-rows-mixed-direction-expected.html: Added.
+ * fast/table/border-collapsing/table-ltr-rows-mixed-direction.html: Added.
+ * fast/table/border-collapsing/table-rtl-row-mixed-direction-expected.html: Added.
+ * fast/table/border-collapsing/table-rtl-row-mixed-direction.html: Added.
+
2012-11-04 Andrey Adaikin <aand...@chromium.org>
Web Inspector: [Canvas] do not blow up the capturing log
Added: trunk/LayoutTests/fast/table/border-collapsing/table-ltr-rows-mixed-direction-expected.html (0 => 133439)
--- trunk/LayoutTests/fast/table/border-collapsing/table-ltr-rows-mixed-direction-expected.html (rev 0)
+++ trunk/LayoutTests/fast/table/border-collapsing/table-ltr-rows-mixed-direction-expected.html 2012-11-05 07:55:28 UTC (rev 133439)
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+table {
+ border-collapse: collapse;
+ border: 3px solid green;
+}
+
+td {
+ height: 100px;
+ width: 100px;
+}
+</style>
+<head>
+<body>
+<p>Test for <a href="" Fix the collapsing border code to handle mixed directionality at the row level</p>
+<p>This test checks that a table with dir="ltr" and mixed directionality on the row works as expected.</p>
+<p>The table below should have a 3px green outer border.</p>
+<table>
+ <tbody>
+ <tr>
+ <td></td>
+ <td></td>
+ </tr>
+ <tr>
+ <td></td>
+ <td></td>
+ </tr>
+ <tr>
+ <td></td>
+ <td></td>
+ </tr>
+ </tbody>
+</table>
+</table>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/border-collapsing/table-ltr-rows-mixed-direction-expected.html
___________________________________________________________________
Added: svn:executable
Added: trunk/LayoutTests/fast/table/border-collapsing/table-ltr-rows-mixed-direction.html (0 => 133439)
--- trunk/LayoutTests/fast/table/border-collapsing/table-ltr-rows-mixed-direction.html (rev 0)
+++ trunk/LayoutTests/fast/table/border-collapsing/table-ltr-rows-mixed-direction.html 2012-11-05 07:55:28 UTC (rev 133439)
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+table {
+ border-collapse: collapse;
+ border-left: 2px solid red;
+ border-right: 2px solid red;
+ border-bottom: 3px solid green;
+ border-top: 3px solid green;
+}
+
+.leftGreenBorder {
+ border-left: 3px solid green;
+}
+
+.rightGreenBorder {
+ border-right: 3px solid green;
+}
+
+td {
+ height: 100px;
+ width: 100px;
+}
+</style>
+<head>
+<body>
+<p>Test for <a href="" Fix the collapsing border code to handle mixed directionality at the row level</p>
+<p>This test checks that a table with dir="ltr" and mixed directionality on the row works as expected.</p>
+<p>The table below should have a 3px green outer border.</p>
+<table>
+ <tbody class="rightGreenBorder">
+ <tr dir="rtl">
+ <td class="leftGreenBorder"></td>
+ <td></td>
+ </tr>
+ <tr class="leftGreenBorder">
+ <td></td>
+ <td></td>
+ </tr>
+ <tr>
+ <td class="leftGreenBorder"></td>
+ <td></td>
+ </tr>
+ </tbody>
+</table>
+</table>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/border-collapsing/table-ltr-rows-mixed-direction.html
___________________________________________________________________
Added: svn:executable
Added: trunk/LayoutTests/fast/table/border-collapsing/table-rtl-row-mixed-direction-expected.html (0 => 133439)
--- trunk/LayoutTests/fast/table/border-collapsing/table-rtl-row-mixed-direction-expected.html (rev 0)
+++ trunk/LayoutTests/fast/table/border-collapsing/table-rtl-row-mixed-direction-expected.html 2012-11-05 07:55:28 UTC (rev 133439)
@@ -0,0 +1,41 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+table {
+ border-collapse: collapse;
+ border: 3px solid green;
+}
+
+td {
+ height: 100px;
+ width: 100px;
+}
+</style>
+<head>
+<body>
+<p>Test for <a href="" Fix the collapsing border code to handle mixed directionality at the row level</p>
+<p>This test checks that a table with dir="rtl" and mixed directionality on the row works as expected.</p>
+<p>The table below should have a 3px green outer border.</p>
+<table>
+ <tbody>
+ <tr>
+ <td></td>
+ <td></td>
+ </tr>
+ <tr>
+ <td></td>
+ <td></td>
+ </tr>
+ <tr>
+ <td></td>
+ <td></td>
+ </tr>
+ <tr>
+ <td></td>
+ <td></td>
+ </tr>
+ </tbody>
+</table>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/border-collapsing/table-rtl-row-mixed-direction-expected.html
___________________________________________________________________
Added: svn:executable
Added: trunk/LayoutTests/fast/table/border-collapsing/table-rtl-row-mixed-direction.html (0 => 133439)
--- trunk/LayoutTests/fast/table/border-collapsing/table-rtl-row-mixed-direction.html (rev 0)
+++ trunk/LayoutTests/fast/table/border-collapsing/table-rtl-row-mixed-direction.html 2012-11-05 07:55:28 UTC (rev 133439)
@@ -0,0 +1,52 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+table {
+ border-collapse: collapse;
+ border-left: 2px solid red;
+ border-right: 2px solid red;
+ border-bottom: 3px solid green;
+ border-top: 3px solid green;
+}
+
+.leftGreenBorder {
+ border-left: 3px solid green;
+}
+
+.rightGreenBorder {
+ border-right: 3px solid green;
+}
+
+td {
+ height: 100px;
+ width: 100px;
+}
+</style>
+<head>
+<body>
+<p>Test for <a href="" Fix the collapsing border code to handle mixed directionality at the row level</p>
+<p>This test checks that a table with dir="rtl" and mixed directionality on the row works as expected.</p>
+<p>The table below should have a 3px green outer border.</p>
+<table dir="rtl">
+ <tbody class="rightGreenBorder">
+ <tr dir="ltr">
+ <td></td>
+ <td class="leftGreenBorder"></td>
+ </tr>
+ <tr dir="rtl">
+ <td></td>
+ <td class="leftGreenBorder"></td>
+ </tr>
+ <tr dir="ltr" class="leftGreenBorder">
+ <td></td>
+ <td></td>
+ </tr>
+ <tr dir="rtl" class="leftGreenBorder">
+ <td></td>
+ <td></td>
+ </tr>
+ </tbody>
+</table>
+</body>
+</html>
Property changes on: trunk/LayoutTests/fast/table/border-collapsing/table-rtl-row-mixed-direction.html
___________________________________________________________________
Added: svn:executable
Modified: trunk/Source/WebCore/ChangeLog (133438 => 133439)
--- trunk/Source/WebCore/ChangeLog 2012-11-05 07:52:52 UTC (rev 133438)
+++ trunk/Source/WebCore/ChangeLog 2012-11-05 07:55:28 UTC (rev 133439)
@@ -1,3 +1,61 @@
+2012-11-04 Julien Chaffraix <jchaffr...@webkit.org>
+
+ Fix the collapsing border code to handle mixed directionality at the row level
+ https://bugs.webkit.org/show_bug.cgi?id=101060
+
+ Reviewed by Ojan Vafai.
+
+ After bug 87900, we support mixed directionality at the row-group level. For coherency
+ - as the underlying code didn't support it - we were artificially ignoring 'direction'
+ below the row-group. This change relaxes the restriction and patches the collapsing
+ borders code to query the right style and border.
+
+ Tests: fast/table/border-collapsing/table-ltr-rows-mixed-direction.html
+ fast/table/border-collapsing/table-rtl-row-mixed-direction.html
+
+ * rendering/RenderTable.cpp:
+ (WebCore::RenderTable::tableStartBorderAdjoiningCell):
+ (WebCore::RenderTable::tableEndBorderAdjoiningCell):
+ Changed to query the row's direction.
+
+ * rendering/RenderTableCell.cpp:
+ (WebCore::RenderTableCell::hasStartBorderAdjoiningTable):
+ (WebCore::RenderTableCell::hasEndBorderAdjoiningTable):
+ Added 2 helper functions. They determine if a specific cell's border
+ adjoins the table. This code is required as the last cell's end border
+ can be resolved against the start border.
+
+ (WebCore::RenderTableCell::computeCollapsedStartBorder):
+ (WebCore::RenderTableCell::computeCollapsedEndBorder):
+ Updated these functions now that being the start / end column doesn't mean
+ that we have to resolve against the row / row-group / table's border.
+
+ * rendering/RenderTableCell.h:
+ (WebCore::RenderTableCell::styleForCellFlow):
+ Updated to return the row's style.
+
+ * rendering/RenderTableRow.cpp:
+ (WebCore::RenderTableRow::borderAdjoiningStartCell):
+ (WebCore::RenderTableRow::borderAdjoiningEndCell):
+ * rendering/RenderTableSection.cpp:
+ (WebCore::RenderTableSection::borderAdjoiningStartCell):
+ (WebCore::RenderTableSection::borderAdjoiningEndCell):
+ Updated these functions to work with mixed directionality.
+
+ * rendering/RenderTableSection.cpp:
+ (WebCore::RenderTableSection::setLogicalPositionForCell):
+ Changed this function to use the section's direction. This is wrong and should be changed
+ once we properly fix the collapsing border code.
+
+ (WebCore::RenderTableSection::logicalRectForWritingModeAndDirection):
+ Added a FIXME.
+
+ * rendering/style/CollapsedBorderValue.h:
+ (WebCore::CollapsedBorderValue::width):
+ This is a bug in our implementation: we used to return a non-zero width for inexistant borders (per CSS 2.1,
+ 'border-style: off | hidden' should have a 0 width). This is covered by our existing tests (among others by
+ fast/table/border-collapsing/last-cell-left-border-hidden-table-ltr-section-rtl.html).
+
2012-11-04 Andrey Adaikin <aand...@chromium.org>
Web Inspector: [Canvas] do not blow up the capturing log
Modified: trunk/Source/WebCore/rendering/RenderTable.cpp (133438 => 133439)
--- trunk/Source/WebCore/rendering/RenderTable.cpp 2012-11-05 07:52:52 UTC (rev 133438)
+++ trunk/Source/WebCore/rendering/RenderTable.cpp 2012-11-05 07:55:28 UTC (rev 133439)
@@ -1349,7 +1349,7 @@
const BorderValue& RenderTable::tableStartBorderAdjoiningCell(const RenderTableCell* cell) const
{
ASSERT(cell->isFirstOrLastCellInRow());
- if (hasSameDirectionAs(cell->section()))
+ if (hasSameDirectionAs(cell->row()))
return style()->borderStart();
return style()->borderEnd();
@@ -1358,7 +1358,7 @@
const BorderValue& RenderTable::tableEndBorderAdjoiningCell(const RenderTableCell* cell) const
{
ASSERT(cell->isFirstOrLastCellInRow());
- if (hasSameDirectionAs(cell->section()))
+ if (hasSameDirectionAs(cell->row()))
return style()->borderEnd();
return style()->borderStart();
Modified: trunk/Source/WebCore/rendering/RenderTableCell.cpp (133438 => 133439)
--- trunk/Source/WebCore/rendering/RenderTableCell.cpp 2012-11-05 07:52:52 UTC (rev 133438)
+++ trunk/Source/WebCore/rendering/RenderTableCell.cpp 2012-11-05 07:55:28 UTC (rev 133439)
@@ -460,6 +460,28 @@
return border.style() == BHIDDEN ? CollapsedBorderValue() : border;
}
+bool RenderTableCell::hasStartBorderAdjoiningTable() const
+{
+ bool isStartColumn = !col();
+ bool isEndColumn = table()->colToEffCol(col() + colSpan() - 1) == table()->numEffCols() - 1;
+ bool hasSameDirectionAsTable = hasSameDirectionAs(table());
+
+ // The table direction determines the row direction. In mixed directionality, we cannot guarantee that
+ // we have a common border with the table (think a ltr table with rtl start cell).
+ return (isStartColumn && hasSameDirectionAsTable) || (isEndColumn && !hasSameDirectionAsTable);
+}
+
+bool RenderTableCell::hasEndBorderAdjoiningTable() const
+{
+ bool isStartColumn = !col();
+ bool isEndColumn = table()->colToEffCol(col() + colSpan() - 1) == table()->numEffCols() - 1;
+ bool hasSameDirectionAsTable = hasSameDirectionAs(table());
+
+ // The table direction determines the row direction. In mixed directionality, we cannot guarantee that
+ // we have a common border with the table (think a ltr table with ltr end cell).
+ return (isStartColumn && !hasSameDirectionAsTable) || (isEndColumn && hasSameDirectionAsTable);
+}
+
CollapsedBorderValue RenderTableCell::collapsedStartBorder(IncludeBorderColorOrNot includeColor) const
{
CollapsedBorderValue result = computeCollapsedStartBorder(includeColor);
@@ -486,7 +508,10 @@
result = chooseBorder(cellBeforeAdjoiningBorder, result);
if (!result.exists())
return result;
- } else {
+ }
+
+ bool startBorderAdjoinsTable = hasStartBorderAdjoiningTable();
+ if (startBorderAdjoinsTable) {
// (3) Our row's start border.
result = chooseBorder(result, CollapsedBorderValue(row()->borderAdjoiningStartCell(this), includeColor ? parent()->style()->visitedDependentColor(startColorProperty) : Color(), BROW));
if (!result.exists())
@@ -545,7 +570,9 @@
}
}
}
- } else {
+ }
+
+ if (startBorderAdjoinsTable) {
// (7) The table's start border.
result = chooseBorder(result, CollapsedBorderValue(table->tableStartBorderAdjoiningCell(this), includeColor ? table->style()->visitedDependentColor(startColorProperty) : Color(), BTABLE));
if (!result.exists())
@@ -584,7 +611,10 @@
if (!result.exists())
return result;
}
- } else {
+ }
+
+ bool endBorderAdjoinsTable = hasEndBorderAdjoiningTable();
+ if (endBorderAdjoinsTable) {
// (3) Our row's end border.
result = chooseBorder(result, CollapsedBorderValue(row()->borderAdjoiningEndCell(this), includeColor ? parent()->style()->visitedDependentColor(endColorProperty) : Color(), BROW));
if (!result.exists())
@@ -642,7 +672,9 @@
}
}
}
- } else {
+ }
+
+ if (endBorderAdjoinsTable) {
// (7) The table's end border.
result = chooseBorder(result, CollapsedBorderValue(table->tableEndBorderAdjoiningCell(this), includeColor ? table->style()->visitedDependentColor(endColorProperty) : Color(), BTABLE));
if (!result.exists())
Modified: trunk/Source/WebCore/rendering/RenderTableCell.h (133438 => 133439)
--- trunk/Source/WebCore/rendering/RenderTableCell.h 2012-11-05 07:52:52 UTC (rev 133438)
+++ trunk/Source/WebCore/rendering/RenderTableCell.h 2012-11-05 07:55:28 UTC (rev 133439)
@@ -164,7 +164,7 @@
// on all table parts and writing-mode on cells.
const RenderStyle* styleForCellFlow() const
{
- return section()->style();
+ return row()->style();
}
const BorderValue& borderAdjoiningTableStart() const
@@ -240,6 +240,9 @@
void setIntrinsicPaddingAfter(int p) { m_intrinsicPaddingAfter = p; }
void setIntrinsicPadding(int before, int after) { setIntrinsicPaddingBefore(before); setIntrinsicPaddingAfter(after); }
+ bool hasStartBorderAdjoiningTable() const;
+ bool hasEndBorderAdjoiningTable() const;
+
CollapsedBorderValue collapsedStartBorder(IncludeBorderColorOrNot = IncludeBorderColor) const;
CollapsedBorderValue collapsedEndBorder(IncludeBorderColorOrNot = IncludeBorderColor) const;
CollapsedBorderValue collapsedBeforeBorder(IncludeBorderColorOrNot = IncludeBorderColor) const;
Modified: trunk/Source/WebCore/rendering/RenderTableRow.cpp (133438 => 133439)
--- trunk/Source/WebCore/rendering/RenderTableRow.cpp 2012-11-05 07:52:52 UTC (rev 133438)
+++ trunk/Source/WebCore/rendering/RenderTableRow.cpp 2012-11-05 07:55:28 UTC (rev 133439)
@@ -84,14 +84,14 @@
const BorderValue& RenderTableRow::borderAdjoiningStartCell(const RenderTableCell* cell) const
{
- ASSERT_UNUSED(cell, !table()->cellBefore(cell));
+ ASSERT_UNUSED(cell, cell->isFirstOrLastCellInRow());
// FIXME: https://webkit.org/b/79272 - Add support for mixed directionality at the cell level.
return style()->borderStart();
}
const BorderValue& RenderTableRow::borderAdjoiningEndCell(const RenderTableCell* cell) const
{
- ASSERT_UNUSED(cell, !table()->cellAfter(cell));
+ ASSERT_UNUSED(cell, cell->isFirstOrLastCellInRow());
// FIXME: https://webkit.org/b/79272 - Add support for mixed directionality at the cell level.
return style()->borderEnd();
}
Modified: trunk/Source/WebCore/rendering/RenderTableSection.cpp (133438 => 133439)
--- trunk/Source/WebCore/rendering/RenderTableSection.cpp 2012-11-05 07:52:52 UTC (rev 133438)
+++ trunk/Source/WebCore/rendering/RenderTableSection.cpp 2012-11-05 07:55:28 UTC (rev 133439)
@@ -987,6 +987,7 @@
tableAlignedRect = tableAlignedRect.transposedRect();
const Vector<int>& columnPos = table()->columnPositions();
+ // FIXME: The table's direction should determine our row's direction, not the section's (see bug 96691).
if (!style()->isLeftToRightDirection())
tableAlignedRect.setX(columnPos[columnPos.size() - 1] - tableAlignedRect.maxX());
@@ -1266,16 +1267,14 @@
const BorderValue& RenderTableSection::borderAdjoiningStartCell(const RenderTableCell* cell) const
{
- ASSERT_UNUSED(cell, !table()->cellBefore(cell));
- // FIXME: https://webkit.org/b/79272 - Add support for mixed directionality at the cell level.
- return style()->borderStart();
+ ASSERT(cell->isFirstOrLastCellInRow());
+ return hasSameDirectionAs(cell) ? style()->borderStart() : style()->borderEnd();
}
const BorderValue& RenderTableSection::borderAdjoiningEndCell(const RenderTableCell* cell) const
{
- ASSERT_UNUSED(cell, !table()->cellAfter(cell));
- // FIXME: https://webkit.org/b/79272 - Add support for mixed directionality at the cell level.
- return style()->borderEnd();
+ ASSERT(cell->isFirstOrLastCellInRow());
+ return hasSameDirectionAs(cell) ? style()->borderEnd() : style()->borderStart();
}
const RenderTableCell* RenderTableSection::firstRowCellAdjoiningTableStart() const
@@ -1429,7 +1428,8 @@
LayoutPoint cellLocation(0, m_rowPos[cell->rowIndex()]);
int horizontalBorderSpacing = table()->hBorderSpacing();
- if (!cell->styleForCellFlow()->isLeftToRightDirection())
+ // FIXME: The table's direction should determine our row's direction, not the section's (see bug 96691).
+ if (!style()->isLeftToRightDirection())
cellLocation.setX(table()->columnPositions()[table()->numEffCols()] - table()->columnPositions()[table()->colToEffCol(cell->col() + cell->colSpan())] + horizontalBorderSpacing);
else
cellLocation.setX(table()->columnPositions()[effectiveColumn] + horizontalBorderSpacing);
Modified: trunk/Source/WebCore/rendering/style/CollapsedBorderValue.h (133438 => 133439)
--- trunk/Source/WebCore/rendering/style/CollapsedBorderValue.h 2012-11-05 07:52:52 UTC (rev 133438)
+++ trunk/Source/WebCore/rendering/style/CollapsedBorderValue.h 2012-11-05 07:55:28 UTC (rev 133439)
@@ -51,7 +51,7 @@
{
}
- unsigned width() const { return m_width; }
+ unsigned width() const { return m_style > BHIDDEN ? m_width : 0; }
EBorderStyle style() const { return static_cast<EBorderStyle>(m_style); }
bool exists() const { return m_precedence != BOFF; }
Color color() const { return Color(m_color, m_colorIsValid); }
