Table rows are incorrectly collapsed in case of hidden cells and rowspans.

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 526 matching lines @@
         lastRowIndex = rowIndex;
         lastRowSpan = rowSpan;
 
         struct SpanningRowsHeight spanningRowsHeight;
 
         populateSpanningRowsHeightFromCell(cell, spanningRowsHeight);
 
         if (spanningRowsHeight.rowWithOnlySpanningCells)
             updateRowsHeightHavingOnlySpanningCells(cell, spanningRowsHeight);
 
-        if (!spanningRowsHeight.totalRowsHeight || spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing <= spanningRowsHeight.totalRowsHeight) {
+
+        if (!spanningRowsHeight.totalRowsHeight && spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing) {
+            if (spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing) {

[Julien - ping for review, 2013/10/30 18:12:44]

This 'if' is unneeded: to get into this branch, you have to satisfy the exact
same condition above.

[suchit.agrawal, 2013/10/31 13:43:02]

Previous condition is wrong. Forgot to remove it from there when it is added
inside. Thanks

+                spanningRowsHeight.totalRowsHeight = spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing + borderSpacingForRow(rowIndex + rowSpan -1);

[Julien - ping for review, 2013/10/30 18:12:44]

Please add a comment about what this line is supposed to do. It seems like the
next branch will definitely be impacted by this line (you change
spanningRowsHeight.totalRowsHeight to be non-zero above).

Also why do we do it here and not when populating spanningRowsHeight?

[suchit.agrawal, 2013/10/31 13:43:02]

spanningRowsHeight.totalRowsHeight change is not required because just after
that continue is used.

+                m_rowPos[rowIndex + rowSpan] += spanningRowsHeight.totalRowsHeight;
+            }
             extraHeightToPropagate = m_rowPos[rowIndex + rowSpan] - originalBeforePosition;
             continue;
         }
+
+        if (spanningRowsHeight.spanningCellHeightIgnoringBorderSpacing <= spanningRowsHeight.totalRowsHeight) {
+            extraHeightToPropagate = m_rowPos[rowIndex + rowSpan] - originalBeforePosition;
+            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 < spanningCellEndIndex; row++) {
@@ 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