Tables with specific merge cell configuration render with extra height to tr elements.

Source/core/rendering/RenderTableSection.cpp

 /*
  * Copyright (C) 1997 Martin Jones (mjones@kde.org)
  *           (C) 1997 Torben Weis (weis@kde.org)
  *           (C) 1998 Waldo Bastian (bastian@kde.org)
  *           (C) 1999 Lars Knoll (knoll@kde.org)
  *           (C) 1999 Antti Koivisto (koivisto@kde.org)
  * Copyright (C) 2003, 2004, 2005, 2006, 2008, 2009, 2010, 2013 Apple Inc. All rights reserved.
  * Copyright (C) 2006 Alexey Proskuryakov (ap@nypop.com)
  *
  * This library is free software; you can redistribute it and/or
@@ skipping 466 matching lines @@
     if (cellIsFullyIncludedInOtherCell(cell1, cell2))
         return true;
     // Sorting lower row index first because first we need to apply the extra height of spanning cell which
     // comes first in the table so lower rows's position would increment in sequence.
     if (!cellIsFullyIncludedInOtherCell(cell2, cell1))
         return (cell1->rowIndex() < cell2->rowIndex());
 
     return false;
 }
 
-bool RenderTableSection::isHeightNeededForRowHavingOnlySpanningCells(unsigned row)
-{
-    unsigned totalCols = m_grid[row].row.size();
-
-    if (!totalCols)
-        return false;
-
-    for (unsigned col = 0; col < totalCols; col++) {
-        const CellStruct& rowSpanCell = cellAt(row, col);
-
-        if (rowSpanCell.cells.size()) {
-            RenderTableCell* cell = rowSpanCell.cells[0];
-            const unsigned rowIndex = cell->rowIndex();
-            const unsigned rowSpan = cell->rowSpan();
-            int totalRowSpanCellHeight = 0;
-
-            for (unsigned row = 0; row < rowSpan; row++) {
-                unsigned actualRow = row + rowIndex;
-                totalRowSpanCellHeight += m_rowPos[actualRow + 1] - m_rowPos[actualRow];
-            }
-            totalRowSpanCellHeight -= borderSpacingForRow(rowIndex + rowSpan - 1);
-
-            if (totalRowSpanCellHeight < cell->logicalHeightForRowSizing())
-                return true;
-        }
-    }
-
-    return false;
-}
-
-unsigned RenderTableSection::calcRowHeightHavingOnlySpanningCells(unsigned row)
+unsigned RenderTableSection::calcRowHeightHavingOnlySpanningCells(unsigned row, int& accumulatedCellPositionIncrease, unsigned rowToApplyExtraHeight, unsigned& extraTableHeightToPropgate, Vector<int>& rowsCountWithOnlySpanningCells)
 {
     ASSERT(rowHasOnlySpanningCells(row));
 
     unsigned totalCols = m_grid[row].row.size();
 
     if (!totalCols)
         return 0;
 
     unsigned rowHeight = 0;
 
     for (unsigned col = 0; col < totalCols; col++) {
         const CellStruct& rowSpanCell = cellAt(row, col);
-        if (rowSpanCell.cells.size() && rowSpanCell.cells[0]->rowSpan() > 1)
-            rowHeight = std::max(rowHeight, rowSpanCell.cells[0]->logicalHeightForRowSizing() / rowSpanCell.cells[0]->rowSpan());
+
+        if (!rowSpanCell.cells.size())
+            continue;
+
+        RenderTableCell* cell = rowSpanCell.cells[0];
+
+        if (cell->rowSpan() <= 1)

[Julien - ping for review, 2014/12/16 23:00:27]

rowSpan is at least 1 so we shouldn't put use <= as it obfuscate the intent.

[a.suchit, 2014/12/17 13:50:37]

Done.

+            continue;
+
+        const unsigned cellRowIndex = cell->rowIndex();
+        const unsigned cellRowSpan = cell->rowSpan();

[Julien - ping for review, 2014/12/16 23:00:27]

Note that the Chromium coding style bans the use of any integer type except for
'int' and special types like size_t
(http://google-styleguide.googlecode.com/svn/trunk/cppguide.html#Integer_Types).

This means that we should produce new code using size_t.

[a.suchit, 2014/12/17 13:50:37]

In that case, we need to change it in whole table module.
I can work as another patch for this changes. Because it is required many
changes.

+
+        // As we are going from the top of the table to the bottom to calculate the row
+        // heights for rows that only contain spanning cells and all previous rows are
+        // processed we only need to find the number of rows with spanning cells from the
+        // current cell to the end of the current cells spanning height.
+        unsigned startRowForSpanningCellCount = max(cellRowIndex, row);

[Julien - ping for review, 2014/12/16 23:00:27]

Coding style: std::max should be used not max.

[a.suchit, 2014/12/17 13:50:37]

Done.

+        unsigned endRow = cellRowIndex + cellRowSpan;
+        unsigned spanningCellsRowsCountHavingZeroHeight = rowsCountWithOnlySpanningCells[endRow - 1];
+
+        if (startRowForSpanningCellCount)
+            spanningCellsRowsCountHavingZeroHeight -= rowsCountWithOnlySpanningCells[startRowForSpanningCellCount - 1];
+
+        int totalRowspanCellHeight = (m_rowPos[endRow] - m_rowPos[cellRowIndex]) - borderSpacingForRow(endRow - 1);
+
+        totalRowspanCellHeight += accumulatedCellPositionIncrease;
+        if (rowToApplyExtraHeight >= cellRowIndex && rowToApplyExtraHeight < endRow)
+            totalRowspanCellHeight += extraTableHeightToPropgate;
+
+        if (totalRowspanCellHeight < cell->logicalHeightForRowSizing()) {
+            unsigned extraHeightRequired = cell->logicalHeightForRowSizing() - totalRowspanCellHeight;
+
+            rowHeight = std::max(rowHeight, extraHeightRequired / spanningCellsRowsCountHavingZeroHeight);
+        }
     }
 
     return rowHeight;
 }
 
-void RenderTableSection::updateRowsHeightHavingOnlySpanningCells(RenderTableCell* cell, struct SpanningRowsHeight& spanningRowsHeight)
+void RenderTableSection::updateRowsHeightHavingOnlySpanningCells(RenderTableCell* cell, struct SpanningRowsHeight& spanningRowsHeight, unsigned& extraHeightToPropagate, Vector<int>& rowsCountWithOnlySpanningCells)
 {
     ASSERT(spanningRowsHeight.rowHeight.size());
 
     int accumulatedPositionIncrease = 0;
     const unsigned rowSpan = cell->rowSpan();
     const unsigned rowIndex = cell->rowIndex();
 
     ASSERT_UNUSED(rowSpan, rowSpan == spanningRowsHeight.rowHeight.size());
 
     for (unsigned row = 0; row < spanningRowsHeight.rowHeight.size(); row++) {
         unsigned actualRow = row + rowIndex;
-        if (!spanningRowsHeight.rowHeight[row] && rowHasOnlySpanningCells(actualRow) && isHeightNeededForRowHavingOnlySpanningCells(actualRow)) {
-            spanningRowsHeight.rowHeight[row] = calcRowHeightHavingOnlySpanningCells(actualRow);
+        if (!spanningRowsHeight.rowHeight[row] && rowHasOnlySpanningCells(actualRow)) {
+            spanningRowsHeight.rowHeight[row] = calcRowHeightHavingOnlySpanningCells(actualRow, accumulatedPositionIncrease, rowIndex + rowSpan, extraHeightToPropagate, rowsCountWithOnlySpanningCells);
             accumulatedPositionIncrease += spanningRowsHeight.rowHeight[row];
         }
         m_rowPos[actualRow + 1] += accumulatedPositionIncrease;
     }
 
     spanningRowsHeight.totalRowsHeight += accumulatedPositionIncrease;
 }
 
 // Distribute rowSpan cell height in rows those comes in rowSpan cell based on the ratio of row's height if
 // 1. RowSpan cell height is greater then the total height of rows in rowSpan cell
 void RenderTableSection::distributeRowSpanHeightToRows(SpanningRenderTableCells& rowSpanCells)
 {
     ASSERT(rowSpanCells.size());
 
     // 'rowSpanCells' list is already sorted based on the cells rowIndex in ascending order
     // Arrange row spanning cell in the order in which we need to process first.
     std::sort(rowSpanCells.begin(), rowSpanCells.end(), compareRowSpanCellsInHeightDistributionOrder);
 
     unsigned extraHeightToPropagate = 0;
     unsigned lastRowIndex = 0;
     unsigned lastRowSpan = 0;
 
+    Vector<int> rowsCountWithOnlySpanningCells;
+
+    // At this stage, Height of the rows are zero for the one containing only spanning cells.
+    int count = 0;
+    for (unsigned row = 0; row < m_grid.size(); row++) {
+        if (rowHasOnlySpanningCells(row))
+            count++;
+        rowsCountWithOnlySpanningCells.append(count);
+    }
+
     for (unsigned i = 0; i < rowSpanCells.size(); i++) {
         RenderTableCell* cell = rowSpanCells[i];
 
         unsigned rowIndex = cell->rowIndex();
 
         unsigned rowSpan = cell->rowSpan();
 
         unsigned spanningCellEndIndex = rowIndex + rowSpan;
         unsigned lastSpanningCellEndIndex = lastRowIndex + lastRowSpan;
 
@@ skipping 17 matching lines @@
 
         lastRowIndex = rowIndex;
         lastRowSpan = rowSpan;
 
         struct SpanningRowsHeight spanningRowsHeight;
 
         populateSpanningRowsHeightFromCell(cell, spanningRowsHeight);
 
         // Here we are handling only row(s) who have only rowspanning cells and do not have any empty cell.
         if (spanningRowsHeight.isAnyRowWithOnlySpanningCells)
-            updateRowsHeightHavingOnlySpanningCells(cell, spanningRowsHeight);
+            updateRowsHeightHavingOnlySpanningCells(cell, spanningRowsHeight, extraHeightToPropagate, rowsCountWithOnlySpanningCells);
 
         // This code handle row(s) that have rowspanning cell(s) and at least one empty cell.
         // Such rows are not handled below and end up having a height of 0. That would mean
         // content overlapping if one of their cells has any content. To avoid the problem, we
         // add all the remaining spanning cells' height to the last spanned row.
         // This means that we could grow a row past its 'height' or break percentage spreading
         // however this is better than overlapping content.
         // FIXME: Is there a better algorithm?
         if (!spanningRowsHeight.totalRowsHeight) {
             if (spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing)
@@ skipping 994 matching lines @@
     // FIXME: The table's direction should determine our row's direction, not the section's (see bug 96691).
     if (!style()->isLeftToRightDirection())
         cellLocation.setX(table()->columnPositions()[table()->numEffCols()] - table()->columnPositions()[table()->colToEffCol(cell->col() + cell->colSpan())] + horizontalBorderSpacing);
     else
         cellLocation.setX(table()->columnPositions()[effectiveColumn] + horizontalBorderSpacing);
 
     cell->setLogicalLocation(cellLocation);
 }
 
 } // namespace blink