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.
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
suchit.agrawal
2013/11/22 11:49:41
Here you have explained my 1 above and 2 below lin
|
+ // 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 thi
|
+ // 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
suchit.agrawal
2013/11/22 11:49:41
x---x---x---x---x
| A | B | C | D | row1
x---x---
|
+ 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; |
} |