Title: [122516] trunk
- Revision
- 122516
- Author
- [email protected]
- Date
- 2012-07-12 15:33:36 -0700 (Thu, 12 Jul 2012)
Log Message
Row size/position is wrongly calculated when table having overlapping rowspan cell and colspan cell
https://bugs.webkit.org/show_bug.cgi?id=16811
Patch by Pravin D <[email protected]> on 2012-07-12
Reviewed by Julien Chaffraix.
Source/WebCore:
The height of a row is calculated by taking the max height of the cells contained in it. When a row contains
a rowSpan cell and if this row is not the last row of the cell, then its height is max height of other non
rowSpan cells. If the row is the last row of the rowSpan cell, then using the contraint laid by CSS2.1 spec
"For a rowSpan cell, the sum of the row heights involved must be great enough to encompass the cell spanning the rows",
the last remaining height of the rowSpan(cell height minus heights of other involved rows) is taken into consideration
while calculating the height of this row.
Currently when calculating the height of the row we are only using the height of the primary cell at position (row, col).
However when a row has colSpan cell and rowSpan, they might overlap. In such a sitution as only the primary cells height
is considered, the height of the row will be calculated worngly if the other overlapping cell has greater height.
Thus all the overlapping cell at position (row, col) must be considered while calculating the height of a row.
Test: fast/table/last-cell-of-rowspan-overlapping-colspan-cell.html
* rendering/RenderTableSection.cpp:
(WebCore::RenderTableSection::calcRowLogicalHeight):
Fixed function to use all the overlapping cells at position(row, col) to calculate the height/position of rows.
LayoutTests:
* fast/table/last-cell-of-rowspan-overlapping-colspan-cell-expected.html: Added.
* fast/table/last-cell-of-rowspan-overlapping-colspan-cell.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (122515 => 122516)
--- trunk/LayoutTests/ChangeLog 2012-07-12 22:19:10 UTC (rev 122515)
+++ trunk/LayoutTests/ChangeLog 2012-07-12 22:33:36 UTC (rev 122516)
@@ -1,3 +1,13 @@
+2012-07-12 Pravin D <[email protected]>
+
+ Row size/position is wrongly calculated when table having overlapping rowspan cell and colspan cell
+ https://bugs.webkit.org/show_bug.cgi?id=16811
+
+ Reviewed by Julien Chaffraix.
+
+ * fast/table/last-cell-of-rowspan-overlapping-colspan-cell-expected.html: Added.
+ * fast/table/last-cell-of-rowspan-overlapping-colspan-cell.html: Added.
+
2012-07-12 Joshua Bell <[email protected]>
IndexedDB: Enable IDBFactory.deleteDatabase() and webkitGetDatabaseNames() in Workers
Added: trunk/LayoutTests/fast/table/last-cell-of-rowspan-overlapping-colspan-cell-expected.html (0 => 122516)
--- trunk/LayoutTests/fast/table/last-cell-of-rowspan-overlapping-colspan-cell-expected.html (rev 0)
+++ trunk/LayoutTests/fast/table/last-cell-of-rowspan-overlapping-colspan-cell-expected.html 2012-07-12 22:33:36 UTC (rev 122516)
@@ -0,0 +1,25 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p> <h4>Testcase for <a href="" 16811</a>.<br>
+The test case checks if the height and position of rows having overlapping rowSpan and colSpan cells gets properly calculated.</h4>
+If the row height/position is properly calculated then the height of the green rectangle should be same as the yellow rectangle and the green rectangle must be contained
+within the yellow rectangle.
+</p>
+<table style="border:1px solid yellow;">
+ <tr>
+ <td>
+ <img width="20" height="60"/>
+ </td>
+ <td rowspan="2">
+ <img width="20" height="120" style="border:1px solid green;"/>
+ </td>
+ </tr>
+ <tr>
+ <td>
+ <img height="20"/>
+ </td>
+ </tr>
+</table>
+</body>
+</html>
Added: trunk/LayoutTests/fast/table/last-cell-of-rowspan-overlapping-colspan-cell.html (0 => 122516)
--- trunk/LayoutTests/fast/table/last-cell-of-rowspan-overlapping-colspan-cell.html (rev 0)
+++ trunk/LayoutTests/fast/table/last-cell-of-rowspan-overlapping-colspan-cell.html 2012-07-12 22:33:36 UTC (rev 122516)
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<html>
+<body>
+<p> <h4>Testcase for <a href="" 16811</a>.<br>
+The test case checks if the height and position of rows having overlapping rowSpan and colSpan cells gets properly calculated.</h4>
+If the row height/position is properly calculated then the height of the green rectangle should be same as the yellow rectangle and the green rectangle must be contained
+within the yellow rectangle.
+</p>
+<table style="border:1px solid yellow;">
+ <tr>
+ <td>
+ <img width="20" height="60"/>
+ </td>
+ <td rowspan="2">
+ <img width="20" height="120" style="border:1px solid green;"/>
+ </td>
+ </tr>
+ <tr>
+ <td colspan="2">
+ <img height="20"/>
+ </td>
+ </tr>
+</table>
+</body>
+</html>
+
Modified: trunk/Source/WebCore/ChangeLog (122515 => 122516)
--- trunk/Source/WebCore/ChangeLog 2012-07-12 22:19:10 UTC (rev 122515)
+++ trunk/Source/WebCore/ChangeLog 2012-07-12 22:33:36 UTC (rev 122516)
@@ -1,3 +1,27 @@
+2012-07-12 Pravin D <[email protected]>
+
+ Row size/position is wrongly calculated when table having overlapping rowspan cell and colspan cell
+ https://bugs.webkit.org/show_bug.cgi?id=16811
+
+ Reviewed by Julien Chaffraix.
+
+ The height of a row is calculated by taking the max height of the cells contained in it. When a row contains
+ a rowSpan cell and if this row is not the last row of the cell, then its height is max height of other non
+ rowSpan cells. If the row is the last row of the rowSpan cell, then using the contraint laid by CSS2.1 spec
+ "For a rowSpan cell, the sum of the row heights involved must be great enough to encompass the cell spanning the rows",
+ the last remaining height of the rowSpan(cell height minus heights of other involved rows) is taken into consideration
+ while calculating the height of this row.
+ Currently when calculating the height of the row we are only using the height of the primary cell at position (row, col).
+ However when a row has colSpan cell and rowSpan, they might overlap. In such a sitution as only the primary cells height
+ is considered, the height of the row will be calculated worngly if the other overlapping cell has greater height.
+ Thus all the overlapping cell at position (row, col) must be considered while calculating the height of a row.
+
+ Test: fast/table/last-cell-of-rowspan-overlapping-colspan-cell.html
+
+ * rendering/RenderTableSection.cpp:
+ (WebCore::RenderTableSection::calcRowLogicalHeight):
+ Fixed function to use all the overlapping cells at position(row, col) to calculate the height/position of rows.
+
2012-07-12 Joshua Bell <[email protected]>
IndexedDB: Enable IDBFactory.deleteDatabase() and webkitGetDatabaseNames() in Workers
Modified: trunk/Source/WebCore/rendering/RenderTableSection.cpp (122515 => 122516)
--- trunk/Source/WebCore/rendering/RenderTableSection.cpp 2012-07-12 22:19:10 UTC (rev 122515)
+++ trunk/Source/WebCore/rendering/RenderTableSection.cpp 2012-07-12 22:33:36 UTC (rev 122516)
@@ -340,41 +340,42 @@
for (unsigned c = 0; c < totalCols; c++) {
CellStruct& current = cellAt(r, c);
- cell = current.primaryCell();
+ for (unsigned i = 0; i < current.cells.size(); i++) {
+ cell = current.cells[i];
+ if (current.inColSpan && cell->rowSpan() == 1)
+ continue;
- if (!cell || current.inColSpan)
- continue;
+ // FIXME: We are always adding the height of a rowspan to the last rows which doesn't match
+ // other browsers. See webkit.org/b/52185 for example.
+ if ((cell->rowIndex() + cell->rowSpan() - 1) != r)
+ continue;
- // FIXME: We are always adding the height of a rowspan to the last rows which doesn't match
- // other browsers. See webkit.org/b/52185 for example.
- if ((cell->rowIndex() + cell->rowSpan() - 1) != r)
- continue;
+ // For row spanning cells, |r| is the last row in the span.
+ unsigned cellStartRow = cell->rowIndex();
- // For row spanning cells, |r| is the last row in the span.
- unsigned cellStartRow = cell->rowIndex();
-
- if (cell->hasOverrideHeight()) {
- if (!statePusher.didPush()) {
- // Technically, we should also push state for the row, but since
- // rows don't push a coordinate transform, that's not necessary.
- statePusher.push(this, locationOffset());
+ if (cell->hasOverrideHeight()) {
+ if (!statePusher.didPush()) {
+ // Technically, we should also push state for the row, but since
+ // rows don't push a coordinate transform, that's not necessary.
+ statePusher.push(this, locationOffset());
+ }
+ cell->clearIntrinsicPadding();
+ cell->clearOverrideSize();
+ cell->setChildNeedsLayout(true, MarkOnlyThis);
+ cell->layoutIfNeeded();
}
- cell->clearIntrinsicPadding();
- cell->clearOverrideSize();
- cell->setChildNeedsLayout(true, MarkOnlyThis);
- cell->layoutIfNeeded();
- }
- int cellLogicalHeight = cell->logicalHeightForRowSizing();
- m_rowPos[r + 1] = max(m_rowPos[r + 1], m_rowPos[cellStartRow] + cellLogicalHeight);
+ int cellLogicalHeight = cell->logicalHeightForRowSizing();
+ m_rowPos[r + 1] = max(m_rowPos[r + 1], m_rowPos[cellStartRow] + cellLogicalHeight);
- // find out the baseline
- EVerticalAlign va = cell->style()->verticalAlign();
- if (va == BASELINE || va == TEXT_BOTTOM || va == TEXT_TOP || va == SUPER || va == SUB || va == LENGTH) {
- LayoutUnit baselinePosition = cell->cellBaselinePosition();
- if (baselinePosition > cell->borderBefore() + cell->paddingBefore()) {
- m_grid[cellStartRow].baseline = max(m_grid[cellStartRow].baseline, baselinePosition - cell->intrinsicPaddingBefore());
- baselineDescent = max(baselineDescent, m_rowPos[cellStartRow] + cellLogicalHeight - (baselinePosition - cell->intrinsicPaddingBefore()));
+ // find out the baseline
+ EVerticalAlign va = cell->style()->verticalAlign();
+ if (va == BASELINE || va == TEXT_BOTTOM || va == TEXT_TOP || va == SUPER || va == SUB || va == LENGTH) {
+ LayoutUnit baselinePosition = cell->cellBaselinePosition();
+ if (baselinePosition > cell->borderBefore() + cell->paddingBefore()) {
+ m_grid[cellStartRow].baseline = max(m_grid[cellStartRow].baseline, baselinePosition - cell->intrinsicPaddingBefore());
+ baselineDescent = max(baselineDescent, m_rowPos[cellStartRow] + cellLogicalHeight - (baselinePosition - cell->intrinsicPaddingBefore()));
+ }
}
}
}
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes
