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;
 }
 
+// Arrange row spanning cell in the oder in which we need to process first.

[Julien - ping for review, 2013/07/29 16:52:55]

This comment should be embedded into the method name:
sortRowSpanCellsInProcessingOrder (or something similar, we could probably find
a better naming based on how we sort but not sure if it would make it more
readable).

Outlining how you sort (remember the 'why' comments) would be a very good
comment to add at the top of the function.

[a.suchit, 2013/07/30 08:42:20]

Done.

+static inline void rearrangeRowSpanCells(RenderTableSection::SpanningRenderTableCells& rowSpanCells)
+{
+    unsigned totalCells = rowSpanCells.size();
+
+    // 'rowSpanCelss' list is already sorted based on the cells rowIndex in ascending order

[Julien - ping for review, 2013/07/29 16:52:55]

typo: rowSpanCelss

[a.suchit, 2013/07/30 08:42:20]

Done.

+
+    for (unsigned i = 0; i < totalCells; i++) {
+        bool sortCompleted = true;
+
+        for (unsigned j = totalCells - 1; j > i; j--) {

[Julien - ping for review, 2013/07/29 16:52:55]

As mentioned on IRC, reusing std::sort with a custom sorting function is almost
always a better solution to rolling your own sorting function. Especially here,
the implementation would yield to O(n^2) worst case complexity which would be
easily alleviate by std::sort.

[a.suchit, 2013/07/30 08:42:20]

Done.

+            unsigned cellRowIndex = rowSpanCells[j]->rowIndex();
+            unsigned cellRowSpan = rowSpanCells[j]->rowSpan();
+            unsigned prevCellRowIndex = rowSpanCells[j - 1]->rowIndex();
+            unsigned prevCellRowSpan = rowSpanCells[j - 1]->rowSpan();
+            bool swap = false;
+
+            if (cellRowIndex == prevCellRowIndex) {
+                if (cellRowSpan == prevCellRowSpan) {
+                    // if 2 or more row spanning cells start and end at same rows then we need to process only
+                    // one row spanning cell which have highest height. So these types of cells are sorted in
+                    // descending order based on their heights.
+                    if (rowSpanCells[j]->logicalHeightForRowSizing() > rowSpanCells[j - 1]->logicalHeightForRowSizing())
+                        swap = true;
+                } else if (cellRowSpan < prevCellRowSpan) {
+                    // if 2 or more row spanning cells start at same row but end at diffrent rows then smaller
+                    // row spanning cell should get process first. So these types of cells are sorted in ascending
+                    // order based on their sizes.
+                    swap = true;
+                }
+            } else if (cellRowIndex > prevCellRowIndex) {
+                // if the row spanning cell comes under the boundries of another row spanning cell then inner row
+                // spanning cell should get process first. So these types of cells are sorted in ascending order
+                // based on their sizes.
+                if ((cellRowIndex + cellRowSpan) <= (prevCellRowIndex + prevCellRowSpan))
+                    swap = true;
+            }
+
+            if (swap) {
+                RenderTableCell* cell = rowSpanCells[j];
+                rowSpanCells[j] = rowSpanCells[j - 1];
+                rowSpanCells[j - 1] = cell;
+                sortCompleted = false;
+            }
+        }
+
+        if (sortCompleted)
+            break;
+    }
+}
+
 // 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());
 
+    rearrangeRowSpanCells(rowSpanCells);
+
     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)
-            continue;
-
         unsigned rowSpan = cell->rowSpan();
         int originalBeforePosition = m_rowPos[rowIndex + rowSpan];
 
+        if (rowIndex == lastRowIndex && rowSpan == lastRowSpan)
+            continue;
+
         if (extraHeightToPropagate) {
             for (unsigned row = lastRowIndex + lastRowSpan + 1; row <= rowIndex + rowSpan; row++)
                 m_rowPos[row] += extraHeightToPropagate;
         }
 
         lastRowIndex = rowIndex;
         lastRowSpan = rowSpan;
 
         struct SpanningRowsHeight spanningRowsHeight;
 
@@ skipping 1247 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