Bug 409084: When determining if a row or rowgroup isTopOfPage, check if the previous row has positive YMost, rather than just checking if there *is* a previous row. r=bernd sr=dbaron a1.9=beltzner
authordholbert@cs.stanford.edu
Thu, 17 Apr 2008 11:18:41 -0700
changeset 14430 b7ae82c4c29af460b7b85c428a8cf95dfa86d32d
parent 14429 ae6208575ed4e07608107fc99fab4096605e4588
child 14431 c454d6085318ff46f9cb40a66addb5fc36fb8c9f
push id1
push userroot
push dateTue, 26 Apr 2011 22:38:44 +0000
treeherdermozilla-beta@bfdb6e623a36 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbernd, dbaron
bugs409084
milestone1.9pre
Bug 409084: When determining if a row or rowgroup isTopOfPage, check if the previous row has positive YMost, rather than just checking if there *is* a previous row. r=bernd sr=dbaron a1.9=beltzner
layout/reftests/bugs/409084-1-ref.html
layout/reftests/bugs/409084-1a.html
layout/reftests/bugs/409084-1b.html
layout/reftests/bugs/reftest.list
layout/tables/nsTableFrame.cpp
layout/tables/nsTableRowGroupFrame.cpp
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/409084-1-ref.html
@@ -0,0 +1,15 @@
+<html class="reftest-print">
+<head><style>
+    td { vertical-align: top; }
+    .break { page-break-before: always; height: 1in }
+</style></head>
+<body>
+  <table cellspacing="0" cellpadding="0">
+    <tbody>
+      <tr><td rowspan="2">line 1<br/>line 2<br/>line 3</td></tr>
+      <tr><td><div class="tall"></iframe></td></tr>
+    </tbody>
+  </table>
+  <div class="break"></div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/409084-1a.html
@@ -0,0 +1,19 @@
+<html class="reftest-print">
+<head><style>
+    td { vertical-align: top; }
+    .tall {
+      height: 4in; /* just longer than a reftest-print page. */
+      width:  1in;
+      float:  left;
+      border: 0px;
+    }
+</style></head>
+<body>
+  <table cellspacing="0" cellpadding="0">
+    <tbody>
+      <tr><td rowspan="2">line 1<br/>line 2<br/>line 3</td></tr>
+      <tr><td><iframe class="tall"></iframe></td></tr>
+    </tbody>
+  </table>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/bugs/409084-1b.html
@@ -0,0 +1,20 @@
+<html class="reftest-print">
+<head><style>
+    td { vertical-align: top; }
+    .tall {
+      height: 4in; /* just longer than a reftest-print page. */
+      width:  1in;
+      float:  left;
+      border: 0px;
+    }
+</style></head>
+<body>
+  <table cellspacing="0" cellpadding="0">
+    <tbody></tbody>
+    <tfoot>
+      <tr><td rowspan="2">line 1<br/>line 2<br/>line 3</td></tr>
+      <tr><td><iframe class="tall"></iframe></td></tr>
+    </tfoot>
+  </table>
+</body>
+</html>
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -717,16 +717,18 @@ random == 403134-1.html 403134-1-ref.htm
 == 408493-2.html 408493-2-ref.html
 == 408656-1a.html 408656-1-ref.html 
 == 408656-1b.html 408656-1-ref.html
 == 408656-1c.html 408656-1-ref.html
 == 408782-1a.html 408782-1-ref.html
 == 408782-1b.html 408782-1-ref.html
 == 408782-2a.html 408782-2-ref.html
 == 408782-2b.html 408782-2-ref.html
+== 409084-1a.html 409084-1-ref.html
+== 409084-1b.html 409084-1-ref.html
 == 409659-1a.html 409659-1-ref.html
 != 409659-1b.html 409659-1-ref.html
 != 409659-1c.html 409659-1-ref.html
 == 409659-1d.html 409659-1-ref.html
 == 411334-1.xml 411334-1-ref.xml
 == 411585-1.html 411585-1-ref.html
 == 411585-2.html 411585-2-ref.html
 fails == 411585-3.html 411585-3-ref.html # bug 426909
--- a/layout/tables/nsTableFrame.cpp
+++ b/layout/tables/nsTableFrame.cpp
@@ -2964,21 +2964,23 @@ nsTableFrame::ReflowChildren(nsTableRefl
       desiredSize.width = desiredSize.height = 0;
   
       // Reflow the child into the available space
       nsHTMLReflowState kidReflowState(presContext, aReflowState.reflowState,
                                        kidFrame, kidAvailSize,
                                        -1, -1, PR_FALSE);
       InitChildReflowState(kidReflowState);
 
-      // If this isn't the first row group, then we can't be at the top of the page
-      // When a new page starts, a head row group may be added automatically.
-      // We also consider the row groups just after the head as the top of the page.
-      // That is to prevent the infinite loop in some circumstance. See bug 344883.
-      if (childX > (thead ? 1 : 0)) {
+      // If this isn't the first row group, and the previous row group has a
+      // nonzero YMost, then we can't be at the top of the page.
+      // We ignore the head row group in this check, because a head row group
+      // may be automatically added at the top of *every* page.  This prevents
+      // infinite loops in some circumstances - see bug 344883.
+      if (childX > (thead ? 1 : 0) &&
+          (rowGroups[childX - 1]->GetRect().YMost() > 0)) {
         kidReflowState.mFlags.mIsTopOfPage = PR_FALSE;
       }
       aReflowState.y += cellSpacingY;
       if (NS_UNCONSTRAINEDSIZE != aReflowState.availSize.height) {
         aReflowState.availSize.height -= cellSpacingY;
       }
       // record the presence of a next in flow, it might get destroyed so we
       // need to reorder the row group array
--- a/layout/tables/nsTableRowGroupFrame.cpp
+++ b/layout/tables/nsTableRowGroupFrame.cpp
@@ -399,18 +399,19 @@ nsTableRowGroupFrame::ReflowChildren(nsP
   // (Think about multi-column layout!)
   PRBool isPaginated = aPresContext->IsPaginated();
 
   PRBool haveRow = PR_FALSE;
   PRBool reflowAllKids = aReflowState.reflowState.ShouldReflowAllKids() ||
                          tableFrame->IsGeometryDirty();
   PRBool needToCalcRowHeights = reflowAllKids;
 
-  for (nsIFrame* kidFrame = mFrames.FirstChild(); kidFrame;
-       kidFrame = kidFrame->GetNextSibling()) {
+  nsIFrame *prevKidFrame = nsnull;
+  for (nsIFrame* kidFrame = GetFirstFrame(); kidFrame;
+       prevKidFrame = kidFrame, kidFrame = kidFrame->GetNextSibling()) {
     if (kidFrame->GetType() != nsGkAtoms::tableRowFrame) {
       // XXXldb nsCSSFrameConstructor needs to enforce this!
       NS_NOTREACHED("yikes, a non-row child");
       continue;
     }
 
     haveRow = PR_TRUE;
 
@@ -436,18 +437,21 @@ nsTableRowGroupFrame::ReflowChildren(nsP
                                        kidFrame, kidAvailSize,
                                        -1, -1, PR_FALSE);
       InitChildReflowState(*aPresContext, borderCollapse, kidReflowState);
 
       // This can indicate that columns were resized.
       if (aReflowState.reflowState.mFlags.mHResize)
         kidReflowState.mFlags.mHResize = PR_TRUE;
      
-      // If this isn't the first row, then we can't be at the top of the page
-      if (kidFrame != GetFirstFrame()) {
+      NS_ASSERTION(kidFrame == GetFirstFrame() || prevKidFrame, 
+                   "If we're not on the first frame, we should have a "
+                   "previous sibling...");
+      // If prev row has nonzero YMost, then we can't be at the top of the page
+      if (prevKidFrame && prevKidFrame->GetRect().YMost() > 0) {
         kidReflowState.mFlags.mIsTopOfPage = PR_FALSE;
       }
 
       rv = ReflowChild(kidFrame, aPresContext, desiredSize, kidReflowState,
                        0, aReflowState.y, NS_FRAME_INVALIDATE_ON_MOVE,
                        aStatus);
 
       // Place the child
@@ -1303,17 +1307,19 @@ nsTableRowGroupFrame::SplitRowGroup(nsPr
       // see if there is a page break after the row
       nsTableRowFrame* nextRow = rowFrame->GetNextRow();
       if (nextRow && nsTableFrame::PageBreakAfter(*rowFrame, nextRow)) {
         PushChildren(aPresContext, nextRow, rowFrame);
         aStatus = NS_FRAME_NOT_COMPLETE;
         break;
       }
     }
-    isTopOfPage = PR_FALSE; // after the 1st row, we can't be on top of the page any more.
+    // after the 1st row that has a height, we can't be on top
+    // of the page anymore.
+    isTopOfPage = isTopOfPage && rowRect.YMost() == 0;
   }
   return NS_OK;
 }
 
 /** Layout the entire row group.
   * This method stacks rows vertically according to HTML 4.0 rules.
   * Rows are responsible for layout of their children.
   */