Spanning logical height is not added properly in all spanning rows.

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 d8f632d00e89217501a10365948b4e98496c3d98..54468a596e9291270f29bc57d2f689bb1d3e953f 100644
--- a/Source/core/rendering/RenderTableSection.cpp
+++ b/Source/core/rendering/RenderTableSection.cpp
@@ -375,50 +375,67 @@ void RenderTableSection::distributeRowSpanHeightToRows(SpanningRenderTableCells&
 {
     ASSERT(rowSpanCells.size());
 
-    // FIXME: For now, we handle the first rowspan cell in the table but this is wrong.
-    RenderTableCell* cell = rowSpanCells[0];
+    unsigned changedHeight = 0;

[Julien - ping for review, 2013/07/23 00:13:43]

Trying to find better naming for this one:
- extraHeightToPropagate
- heightToPropagate
- heightDifference
- heightDelta

(changedHeight sounds like the new height where it's actually a difference)

Slight preference for the first name but feel free to find a better one.

[a.suchit, 2013/07/23 12:19:56]

Done.

+    unsigned lastRowIndex = 0;
+    unsigned lastRowSpan = 0;
 
-    unsigned rowSpan = cell->rowSpan();
+    for (unsigned i = 0; i < rowSpanCells.size(); i++) {
+        RenderTableCell* cell = rowSpanCells[i];
 
-    struct SpanningRowsHeight spanningRowsHeight;
+        unsigned rowIndex = cell->rowIndex();
 
-    populateSpanningRowsHeightFromCell(cell, spanningRowsHeight);
+        // FIXME: For now, we are handling only rowspan cells those are not overlapping with other
+        // rowspan cells but this is wrong.
+        if (rowIndex < lastRowIndex + lastRowSpan)
+            continue;
 
-    if (!spanningRowsHeight.totalRowsHeight || spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing <= spanningRowsHeight.totalRowsHeight)
-        return;
+        unsigned rowSpan = cell->rowSpan();
+        int initialPos = m_rowPos[rowIndex + rowSpan];

[Julien - ping for review, 2013/07/23 00:13:43]

Let's not abbreviate variables: |initialPosition|

Maybe better: originalBeforePosition

[a.suchit, 2013/07/23 12:19:56]

Done.

 
-    unsigned rowIndex = cell->rowIndex();
-    int totalPercent = 0;
-    int totalAutoRowsHeight = 0;
-    int totalRemainingRowsHeight = spanningRowsHeight.totalRowsHeight;
-
-    // Calculate total percentage, total auto rows height and total rows height except percent rows.
-    for (unsigned row = rowIndex; row < (rowIndex + rowSpan); row++) {
-        if (m_grid[row].logicalHeight.isPercent()) {
-            totalPercent += m_grid[row].logicalHeight.percent();
-            totalRemainingRowsHeight -= spanningRowsHeight.rowHeight[row - rowIndex];
-        } else if (m_grid[row].logicalHeight.isAuto()) {
-            totalAutoRowsHeight += spanningRowsHeight.rowHeight[row - rowIndex];
+        if (changedHeight) {
+            for (unsigned row = lastRowIndex + lastRowSpan + 1; row <= rowIndex + rowSpan; row++)
+                m_rowPos[row] += changedHeight;
         }
-    }
 
-    int initialPos = m_rowPos[rowIndex + rowSpan];
-    int extraRowSpanningHeight = spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing - spanningRowsHeight.totalRowsHeight;
+        lastRowIndex = rowIndex;
+        lastRowSpan = rowSpan;
 
-    distributeExtraRowSpanHeightToPercentRows(cell, totalPercent, extraRowSpanningHeight, spanningRowsHeight.rowHeight);
-    distributeExtraRowSpanHeightToAutoRows(cell, totalAutoRowsHeight, extraRowSpanningHeight, spanningRowsHeight.rowHeight);
-    distributeExtraRowSpanHeightToRemainingRows(cell, totalRemainingRowsHeight, extraRowSpanningHeight, spanningRowsHeight.rowHeight);
+        struct SpanningRowsHeight spanningRowsHeight;
 
-    ASSERT(!extraRowSpanningHeight);
+        populateSpanningRowsHeightFromCell(cell, spanningRowsHeight);
 
-    // Getting total changed height in the table
-    unsigned changedHeight = m_rowPos[rowIndex + rowSpan] - initialPos;
+        if (spanningRowsHeight.totalRowsHeight && spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing > spanningRowsHeight.totalRowsHeight) {
 
-    if (changedHeight) {
-        unsigned totalRows = m_grid.size();
+            int totalPercent = 0;
+            int totalAutoRowsHeight = 0;
+            int totalRemainingRowsHeight = spanningRowsHeight.totalRowsHeight;
 
+            // Calculate total percentage, total auto rows height and total rows height except percent rows.
+            for (unsigned row = rowIndex; row < (rowIndex + rowSpan); row++) {
+                if (m_grid[row].logicalHeight.isPercent()) {
+                    totalPercent += m_grid[row].logicalHeight.percent();
+                    totalRemainingRowsHeight -= spanningRowsHeight.rowHeight[row - rowIndex];
+                } else if (m_grid[row].logicalHeight.isAuto()) {
+                    totalAutoRowsHeight += spanningRowsHeight.rowHeight[row - rowIndex];
+                }
+            }
+
+            int extraRowSpanningHeight = spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing - spanningRowsHeight.totalRowsHeight;
+
+            distributeExtraRowSpanHeightToPercentRows(cell, totalPercent, extraRowSpanningHeight, spanningRowsHeight.rowHeight);
+            distributeExtraRowSpanHeightToAutoRows(cell, totalAutoRowsHeight, extraRowSpanningHeight, spanningRowsHeight.rowHeight);
+            distributeExtraRowSpanHeightToRemainingRows(cell, totalRemainingRowsHeight, extraRowSpanningHeight, spanningRowsHeight.rowHeight);
+
+            ASSERT(!extraRowSpanningHeight);
+
+            // Getting total changed height in the table
+            changedHeight = m_rowPos[rowIndex + rowSpan] - initialPos;
+        }
+    }
+
+    if (changedHeight) {
         // Apply changed height by rowSpan cells to rows present at the end of the table
-        for (unsigned row = rowIndex + rowSpan + 1; row <= totalRows; row++)
+        for (unsigned row = lastRowIndex + lastRowSpan + 1; row <= m_grid.size(); row++)
             m_rowPos[row] += changedHeight;
     }
 }