Diff
Modified: trunk/LayoutTests/ChangeLog (103326 => 103327)
--- trunk/LayoutTests/ChangeLog 2011-12-20 14:04:25 UTC (rev 103326)
+++ trunk/LayoutTests/ChangeLog 2011-12-20 14:08:52 UTC (rev 103327)
@@ -1,3 +1,19 @@
+2011-12-20 Julien Chaffraix <jchaffr...@webkit.org>
+
+ Regression(99212): table rows get incorrect height after changing some cells' height
+ https://bugs.webkit.org/show_bug.cgi?id=74303
+
+ Reviewed by Darin Adler.
+
+ Added some tests that resize a cell or a row and check that we properly recompute the height.
+
+ * fast/table/resize-table-binding-cell-expected.txt: Added.
+ * fast/table/resize-table-binding-cell.html: Added.
+ * fast/table/resize-table-cell-expected.txt: Added.
+ * fast/table/resize-table-cell.html: Added.
+ * fast/table/resize-table-row-expected.txt: Added.
+ * fast/table/resize-table-row.html: Added.
+
2011-12-20 Csaba Osztrogonác <o...@webkit.org>
[Qt] Unreviewed gardening.
Added: trunk/LayoutTests/fast/table/resize-table-binding-cell-expected.txt (0 => 103327)
--- trunk/LayoutTests/fast/table/resize-table-binding-cell-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/table/resize-table-binding-cell-expected.txt 2011-12-20 14:08:52 UTC (rev 103327)
@@ -0,0 +1,13 @@
+Regression(99212): table rows get incorrect height after changing some cells' height
+https://bugs.webkit.org/show_bug.cgi?id=74303
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS getComputedStyle(firstRow, '').getPropertyValue('height') is '44px'
+PASS getComputedStyle(firstRow, '').getPropertyValue('height') is '84px'
+PASS getComputedStyle(firstRow, '').getPropertyValue('height') is '44px'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/table/resize-table-binding-cell.html (0 => 103327)
--- trunk/LayoutTests/fast/table/resize-table-binding-cell.html (rev 0)
+++ trunk/LayoutTests/fast/table/resize-table-binding-cell.html 2011-12-20 14:08:52 UTC (rev 103327)
@@ -0,0 +1,60 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ td
+ {
+ width: 50px;
+ height: 40px;
+ }
+ table.big td.binding
+ {
+ height: 80px;
+ }
+
+ table td.filler
+ {
+ height: auto;
+ }
+ </style>
+ <script src=""
+</head>
+<body>
+ <div style="height: 300px">
+ <table border="1" style="height: 100%">
+ <tr id="firstRow">
+ <td class="binding"></td>
+ <td></td>
+ </tr>
+ <tr>
+ <td class="filler" ></td>
+ <td class="filler" ></td>
+ </tr>
+ </table>
+ </div>
+ <script>
+ var bigRows = false;
+
+ function toggleBigRows()
+ {
+ var table = document.querySelector("table")
+ bigRows = !bigRows;
+ if (bigRows)
+ table.classList.add("big");
+ else
+ table.classList.remove("big");
+ }
+
+ description("Regression(99212): table rows get incorrect height after changing some cells' height<br>https://bugs.webkit.org/show_bug.cgi?id=74303");
+ firstRow = document.getElementById("firstRow");
+ // Original value.
+ shouldBe("getComputedStyle(firstRow, '').getPropertyValue('height')", "'44px'");
+
+ toggleBigRows();
+ shouldBe("getComputedStyle(firstRow, '').getPropertyValue('height')", "'84px'");
+ toggleBigRows();
+ shouldBe("getComputedStyle(firstRow, '').getPropertyValue('height')", "'44px'");
+ </script>
+ <script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/table/resize-table-cell-expected.txt (0 => 103327)
--- trunk/LayoutTests/fast/table/resize-table-cell-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/table/resize-table-cell-expected.txt 2011-12-20 14:08:52 UTC (rev 103327)
@@ -0,0 +1,13 @@
+Regression(99212): table rows get incorrect height after changing some cells' height
+https://bugs.webkit.org/show_bug.cgi?id=74303
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS getComputedStyle(firstRow, '').getPropertyValue('height') is '44px'
+PASS getComputedStyle(firstRow, '').getPropertyValue('height') is '24px'
+PASS getComputedStyle(firstRow, '').getPropertyValue('height') is '44px'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/table/resize-table-cell.html (0 => 103327)
--- trunk/LayoutTests/fast/table/resize-table-cell.html (rev 0)
+++ trunk/LayoutTests/fast/table/resize-table-cell.html 2011-12-20 14:08:52 UTC (rev 103327)
@@ -0,0 +1,61 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ td
+ {
+ width: 50px;
+ height: 40px;
+ }
+ table.small td
+ {
+ height: 20px;
+ }
+
+ table td.filler
+ {
+ height: auto;
+ }
+ </style>
+ <script src=""
+</head>
+<body>
+ <div style="height: 300px">
+ <table border="1" style="height: 100%">
+ <tr id="firstRow">
+ <td></td>
+ <td></td>
+ </tr>
+ <tr>
+ <td class="filler" ></td>
+ <td class="filler" ></td>
+ </tr>
+ </table>
+ </div>
+ <script>
+ var smallRows = false;
+
+ function toggleSmallRows()
+ {
+ var table = document.querySelector("table")
+ smallRows = !smallRows;
+ if (smallRows)
+ table.classList.add("small");
+ else
+ table.classList.remove("small");
+ }
+
+ description("Regression(99212): table rows get incorrect height after changing some cells' height<br>https://bugs.webkit.org/show_bug.cgi?id=74303");
+
+ firstRow = document.getElementById("firstRow");
+ // Original value.
+ shouldBe("getComputedStyle(firstRow, '').getPropertyValue('height')", "'44px'");
+
+ toggleSmallRows();
+ shouldBe("getComputedStyle(firstRow, '').getPropertyValue('height')", "'24px'");
+ toggleSmallRows();
+ shouldBe("getComputedStyle(firstRow, '').getPropertyValue('height')", "'44px'");
+ </script>
+ <script src=""
+</body>
+</html>
Added: trunk/LayoutTests/fast/table/resize-table-row-expected.txt (0 => 103327)
--- trunk/LayoutTests/fast/table/resize-table-row-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/table/resize-table-row-expected.txt 2011-12-20 14:08:52 UTC (rev 103327)
@@ -0,0 +1,13 @@
+Regression(99212): table rows get incorrect height after changing some cells' height
+https://bugs.webkit.org/show_bug.cgi?id=74303
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS getComputedStyle(firstRow, '').getPropertyValue('height') is '40px'
+PASS getComputedStyle(firstRow, '').getPropertyValue('height') is '20px'
+PASS getComputedStyle(firstRow, '').getPropertyValue('height') is '40px'
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/table/resize-table-row.html (0 => 103327)
--- trunk/LayoutTests/fast/table/resize-table-row.html (rev 0)
+++ trunk/LayoutTests/fast/table/resize-table-row.html 2011-12-20 14:08:52 UTC (rev 103327)
@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ td
+ {
+ width: 50px;
+ }
+ tr
+ {
+ height: 40px;
+ }
+ table.small tr
+ {
+ height: 20px;
+ }
+
+ table tr.filler
+ {
+ height: auto;
+ }
+ </style>
+ <script src=""
+</head>
+<body>
+ <div style="height: 300px">
+ <table border="1" style="height: 100%">
+ <tr id="firstRow"><td></td></tr>
+ <tr class="filler"><td></td></tr>
+ </table>
+ </div>
+ <script>
+ var smallRows = false;
+
+ function toggleSmallRows()
+ {
+ var table = document.querySelector("table")
+ smallRows = !smallRows;
+ if (smallRows)
+ table.classList.add("small");
+ else
+ table.classList.remove("small");
+ }
+
+ description("Regression(99212): table rows get incorrect height after changing some cells' height<br>https://bugs.webkit.org/show_bug.cgi?id=74303");
+
+ firstRow = document.getElementById("firstRow");
+ // Original value.
+ shouldBe("getComputedStyle(firstRow, '').getPropertyValue('height')", "'40px'");
+
+ toggleSmallRows();
+ shouldBe("getComputedStyle(firstRow, '').getPropertyValue('height')", "'20px'");
+ toggleSmallRows();
+ shouldBe("getComputedStyle(firstRow, '').getPropertyValue('height')", "'40px'");
+ </script>
+ <script src=""
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (103326 => 103327)
--- trunk/Source/WebCore/ChangeLog 2011-12-20 14:04:25 UTC (rev 103326)
+++ trunk/Source/WebCore/ChangeLog 2011-12-20 14:08:52 UTC (rev 103327)
@@ -1,3 +1,32 @@
+2011-12-20 Julien Chaffraix <jchaffr...@webkit.org>
+
+ Regression(99212): table rows get incorrect height after changing some cells' height
+ https://bugs.webkit.org/show_bug.cgi?id=74303
+
+ Reviewed by Darin Adler.
+
+ Tests: fast/table/resize-table-binding-cell.html
+ fast/table/resize-table-cell.html
+ fast/table/resize-table-row.html
+
+ r99212 wrongly implemented the row's logicalHeight recalculation.
+ The original code would use recalcCells which would properly recalculate a
+ row logicalHeight by iterating over the table's cells but throwing out the
+ existing result.
+
+ Our approach is just to recompute our row's logicalHeight and leave the
+ rest of the section untouched.
+
+ * rendering/RenderTableSection.cpp:
+ (WebCore::updateLogicalHeightForCell):
+ Added this new helper function to update the RowStruct logicalHeight during
+ |addCell| and |rowLogicalHeightChanged|.
+
+ (WebCore::RenderTableSection::addCell):
+ Replaced the old code with a call to updateLogicalHeightForCell.
+ (WebCore::RenderTableSection::rowLogicalHeightChanged):
+ Added a call to updateLogicalHeightForCell for each cells.
+
2011-12-20 Sheriff Bot <webkit.review....@gmail.com>
Unreviewed, rolling out r103322.
Modified: trunk/Source/WebCore/rendering/RenderTableSection.cpp (103326 => 103327)
--- trunk/Source/WebCore/rendering/RenderTableSection.cpp 2011-12-20 14:04:25 UTC (rev 103326)
+++ trunk/Source/WebCore/rendering/RenderTableSection.cpp 2011-12-20 14:08:52 UTC (rev 103327)
@@ -56,6 +56,34 @@
row.logicalHeight = Length();
}
+static inline void updateLogicalHeightForCell(RenderTableSection::RowStruct& row, const RenderTableCell* cell)
+{
+ // We ignore height settings on rowspan cells.
+ if (cell->rowSpan() != 1)
+ return;
+
+ Length logicalHeight = cell->style()->logicalHeight();
+ if (logicalHeight.isPositive() || (logicalHeight.isRelative() && logicalHeight.value() >= 0)) {
+ Length cRowLogicalHeight = row.logicalHeight;
+ switch (logicalHeight.type()) {
+ case Percent:
+ if (!(cRowLogicalHeight.isPercent())
+ || (cRowLogicalHeight.isPercent() && cRowLogicalHeight.percent() < logicalHeight.percent()))
+ row.logicalHeight = logicalHeight;
+ break;
+ case Fixed:
+ if (cRowLogicalHeight.type() < Percent
+ || (cRowLogicalHeight.isFixed() && cRowLogicalHeight.value() < logicalHeight.value()))
+ row.logicalHeight = logicalHeight;
+ break;
+ case Relative:
+ default:
+ break;
+ }
+ }
+}
+
+
RenderTableSection::RenderTableSection(Node* node)
: RenderBox(node)
, m_cCol(0)
@@ -210,28 +238,7 @@
while (m_cCol < nCols && (cellAt(insertionRow, m_cCol).hasCells() || cellAt(insertionRow, m_cCol).inColSpan))
m_cCol++;
- if (rSpan == 1) {
- // we ignore height settings on rowspan cells
- Length logicalHeight = cell->style()->logicalHeight();
- if (logicalHeight.isPositive() || (logicalHeight.isRelative() && logicalHeight.value() >= 0)) {
- Length cRowLogicalHeight = m_grid[insertionRow].logicalHeight;
- switch (logicalHeight.type()) {
- case Percent:
- if (!(cRowLogicalHeight.isPercent()) ||
- (cRowLogicalHeight.isPercent() && cRowLogicalHeight.percent() < logicalHeight.percent()))
- m_grid[insertionRow].logicalHeight = logicalHeight;
- break;
- case Fixed:
- if (cRowLogicalHeight.type() < Percent ||
- (cRowLogicalHeight.isFixed() && cRowLogicalHeight.value() < logicalHeight.value()))
- m_grid[insertionRow].logicalHeight = logicalHeight;
- break;
- case Relative:
- default:
- break;
- }
- }
- }
+ updateLogicalHeightForCell(m_grid[insertionRow], cell);
ensureRows(insertionRow + rSpan);
@@ -1179,9 +1186,17 @@
setNeedsLayout(true);
}
+// FIXME: This function could be made O(1) in certain cases (like for the non-most-constrainive cells' case).
void RenderTableSection::rowLogicalHeightChanged(unsigned rowIndex)
{
setRowLogicalHeightToRowStyleLogicalHeightIfNotRelative(m_grid[rowIndex]);
+
+ for (RenderObject* cell = m_grid[rowIndex].rowRenderer->firstChild(); cell; cell = cell->nextSibling()) {
+ if (!cell->isTableCell())
+ continue;
+
+ updateLogicalHeightForCell(m_grid[rowIndex], toRenderTableCell(cell));
+ }
}
void RenderTableSection::setNeedsCellRecalc()
