Table rows are incorrectly collapsed in case of hidden cells and rowspans.

Source/core/rendering/RenderTableSection.cpp

Index: Source/core/rendering/RenderTableSection.cpp
diff --git a/Source/core/rendering/RenderTableSection.cpp b/Source/core/rendering/RenderTableSection.cpp
index 56978bd95ec32b3a6b3063de01f52bb41422bbb5..9ba33f70545ea672d10d58d2d73381f9b53fd0eb 100644
--- a/Source/core/rendering/RenderTableSection.cpp
+++ b/Source/core/rendering/RenderTableSection.cpp
@@ -546,7 +546,36 @@ void RenderTableSection::distributeRowSpanHeightToRows(SpanningRenderTableCells&
         if (spanningRowsHeight.rowWithOnlySpanningCells)
             updateRowsHeightHavingOnlySpanningCells(cell, spanningRowsHeight);
 
-        if (!spanningRowsHeight.totalRowsHeight || spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing <= spanningRowsHeight.totalRowsHeight) {
+        // Row would be valid or we can say, row would be visible only when at least one cell should start from that row
+        // and cell have rowspan=1.

[Julien - ping for review, 2013/11/21 02:46:23]

I have a hard time putting this line in context.

Based on your following example, it seems that you should just say that "we only
take into account non-empty rows, which are rows with at least a visible
(display != none) cell.

[suchit.agrawal, 2013/11/22 11:49:41]

Thanks, I am tring to explain same thing here.

+        // But there is some exception like, cell with property display:none, rowspan=1 and this is only cell present
+        // in a row than that row will not visible.
+        // Just check an example here :
+        // +---+---+                                           +---+---+
+        // | A | B |    If D cell set as 'display:none' then   | A | B |   D row height does not matter here.
+        // +---+---+                                           +---+---+
+        // |   | D |                                           | C | E |
+        // + C +---+                                           +---+---+
+        // |   | E |
+        // +---+---+
+        // If D and E both cells set as 'display:none' then here is matter for D or/and E cells height becuase C cell should
+        // display.

[Julien - ping for review, 2013/11/21 02:46:23]

I don't think I understand how this relates to the algorithm. If a cell has
'display: none' then it doesn't have a renderer so there is no height to spread
anyway.

[suchit.agrawal, 2013/11/22 11:49:41]

Here you have explained my 1 above and 2 below lines in small sentence.
D and E are not have any height to spread in C. So C height remains 0.
So here we are applying C logical height in last row of rowspan cell. 

+        // Our Algo works only when, (1.)at least one non-rowspan cell presents with rowspan cell OR (2.)all cells in the row
+        // are rowspan cells.
+        // So in this case, D and E cells are invisible so D and E rows are 0 and D and E rows are not based on any of above
+        // 2 conditions. So C rowspan cell is getting 0 height.

[Julien - ping for review, 2013/11/21 02:46:23]

Could you explain what the rowspan cell is? Is this the empty row that is
spanned?

+        // So C cell height assigned to last row (E row).
+        // FIXME : If combination of rowspan cells are present here then in some scenarios outer cells may get more height than
+        // expected.

[Julien - ping for review, 2013/11/21 02:46:23]

FIXMEs are great but they are better when they are _actionable_. Here I have no
clue what a scenario where we would be wrong is.

[suchit.agrawal, 2013/11/22 11:49:41]

x---x---x---x---x
| A | B | C | D |  row1
x---x---x---x---x
|   |   | G |      row2
|   | F X---x
|   |   |   |      row3
| E x---x H x
|   |   |   |      row4
|   | I x---x
|   |   | J |      row5
x---x---x---x

I was thinking for like this scenarios if G and J are hidden (display:none).
Here, rowspan cell's logical height goes in the last row of rowspan cell.
Assume
F, H and I heights are 10 each.
H should be part of row3 and row4 but in our case it would be fully part of only
row4.
F should be part of row2 and row3 but in our case it would be part of only row3.
I should be part of row4 and row5 but in our case it would be part of only row4.
Then display will come like below :-
x---x---x---x---x
| A | B | C | D |  row1
x---x---x---x---x                  row2 height would be 0.
|   |   |
|   | F X          row3
|   |   |
| E x---x---x
|   |   |   |
|   | I x H x      row4
|   |   |   |
x---x---x---x                      row5 height would be 0.

But it should look like :-

x---x---x---x---x
| A | B | C | D |  row1
x---x---x---x---x                  row2 height would be 0.
|   |   |   |
|   | F |   |      row3
|   |   |   | 
| E x---x H x
|   |   |   |
|   | I |   |      row4
|   |   |   |
x---x---x---x                      row5 height would be 0.

+        if (!spanningRowsHeight.totalRowsHeight) {
+            if (spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing)
+                m_rowPos[rowIndex + rowSpan] += spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing + borderSpacingForRow(rowIndex + rowSpan - 1);
+
+            extraHeightToPropagate = m_rowPos[rowIndex + rowSpan] - originalBeforePosition;
+            continue;
+        }
+
+        if (spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing <= spanningRowsHeight.totalRowsHeight) {
             extraHeightToPropagate = m_rowPos[rowIndex + rowSpan] - originalBeforePosition;
             continue;
         }