Row spanning cell content is flowing out of the cell border if this row spanning cell comes under t…

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 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 351 matching lines @@
             }
         }
         m_rowPos[row + 1] += accumulatedPositionIncrease;
     }
 
     ASSERT(!remainder);
 
     extraRowSpanningHeight -= accumulatedPositionIncrease;
 }
 
+// To avoid unneeded extra height distributions, we apply the following sorting algorithm:
+// 1. We sort the cells by increasing starting row, then increasing last row (ie the top-most, longest cells first)

[Julien - ping for review, 2013/07/31 18:21:10]

Please don't just copy and paste what I said without double-checking, this
comment is wrong: We sort by increasing start row but *decreasing* last row (ie
top-most, *shortest* cells first).

+// 2. For cells spanning the same rows, we sort by intrinsic size.
+static bool compareRowSpanCellsInHeightDistributionOrder(const RenderTableCell* cell2, const RenderTableCell* cell1)
+{
+    unsigned cellRowIndex1 = cell1->rowIndex();
+    unsigned cellRowSpan1 = cell1->rowSpan();
+    unsigned cellRowIndex2 = cell2->rowIndex();
+    unsigned cellRowSpan2 = cell2->rowSpan();
+
+    if (cellRowIndex1 == cellRowIndex2 && cellRowSpan1 == cellRowSpan2)
+        return (cell2->logicalHeightForRowSizing() > cell1->logicalHeightForRowSizing());
+
+    return (cellRowIndex2 >= cellRowIndex1 && (cellRowIndex2 + cellRowSpan2) <= (cellRowIndex1 + cellRowSpan1));

[Julien - ping for review, 2013/07/31 18:21:10]

By a strict operator I meant an operator that defines a "strict weak ordering"
(per std::sort documentation). The definition of a strict weak ordering:
https://en.wikipedia.org/wiki/Weak_ordering#Strict_weak_orderings

Because you allow equality in the line above, I thought it was possible to
create 3 cells for which the "transitivity" property wouldn't apply (ie x < y
and y < z but not(x < z)). I double-checked the logic on paper and I think you
can't construct such a case.

+}
+
 // 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;
 
     for (unsigned i = 0; i < rowSpanCells.size(); i++) {
         RenderTableCell* cell = rowSpanCells[i];
 
         unsigned rowIndex = cell->rowIndex();
 
-        // 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)
+        unsigned rowSpan = cell->rowSpan();
+
+        // Only heightest spanning cell will distribute it's extra height in row if more then one spanning cells
+        // present at same level.
+        if (rowIndex == lastRowIndex && rowSpan == lastRowSpan)
             continue;
 
-        unsigned rowSpan = cell->rowSpan();
         int originalBeforePosition = m_rowPos[rowIndex + rowSpan];
 
         if (extraHeightToPropagate) {
             for (unsigned row = lastRowIndex + lastRowSpan + 1; row <= rowIndex + rowSpan; row++)
                 m_rowPos[row] += extraHeightToPropagate;
         }
 
         lastRowIndex = rowIndex;
         lastRowSpan = rowSpan;
 
         struct SpanningRowsHeight spanningRowsHeight;
 
         populateSpanningRowsHeightFromCell(cell, spanningRowsHeight);
 
         if (!spanningRowsHeight.totalRowsHeight || spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing <= spanningRowsHeight.totalRowsHeight)
             continue;
 
         int totalPercent = 0;
         int totalAutoRowsHeight = 0;
         int totalRemainingRowsHeight = spanningRowsHeight.totalRowsHeight;
 
+        // FIXME: Inner spanning cell height should not change if it have fixed height when it's parent spanning cell
+        // is distributing it's extra height in rows.
+
         // 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];
             }
         }
 
@@ skipping 1228 matching lines @@
     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);
     view()->addLayoutDelta(oldCellLocation - cell->location());
 }
 
 } // namespace WebCore