Replace force-fit with setting mIsTopOfPage correctly. (Bug 563584, patch 5) r=roc

layout/generic/nsBlockFrame.cpp

@@ -5527,12 +5527,6 @@ nsBlockFrame::AdjustFloatAvailableSpace(nsBlockReflowState& aState,
     // if they can shrink we may not be constrained to place
     // them in the next line
     availWidth = aFloatAvailableSpace.width;
-    // round down to twips per pixel so that we fit
-    // needed when prev. float has procentage width
-    // (maybe is a table flaw that makes table chose to round up
-    // but I don't want to change that, too risky)
-    nscoord twp = nsPresContext::CSSPixelsToAppUnits(1);
-    availWidth -=  availWidth % twp;
   }
 
   // aState.mY is relative to the border-top, make it relative to the content-top
@@ -5579,6 +5573,7 @@ nsBlockFrame::ReflowFloat(nsBlockReflowState& aState,
                           const nsRect&       aAdjustedAvailableSpace,
                           nsIFrame*           aFloat,
                           nsMargin&           aFloatMargin,
+                          PRBool              aFloatPushedDown,
                           nsReflowStatus&     aReflowStatus)
 {
   NS_PRECONDITION(aFloat->GetStateBits() & NS_FRAME_OUT_OF_FLOW,
@@ -5598,7 +5593,21 @@ nsBlockFrame::ReflowFloat(nsBlockReflowState& aState,
                             nsSize(aAdjustedAvailableSpace.width,
                                    aAdjustedAvailableSpace.height));
 
-  // Setup a block reflow state to reflow the float.
+  // Normally the mIsTopOfPage state is copied from the parent reflow
+  // state.  However, when reflowing a float, if we've placed other
+  // floats that force this float *down* or *narrower*, we should unset
+  // the mIsTopOfPage state.
+  // FIXME: This is somewhat redundant with the |isAdjacentWithTop|
+  // variable below, which has the exact same effect.  Perhaps it should
+  // be merged into that, except that the test for narrowing here is not
+  // about adjacency with the top, so it seems misleading.
+  if (floatRS.mFlags.mIsTopOfPage &&
+      (aFloatPushedDown ||
+       aAdjustedAvailableSpace.width != aState.mContentArea.width)) {
+    floatRS.mFlags.mIsTopOfPage = PR_FALSE;
+  }
+
+  // Setup a block reflow context to reflow the float.
   nsBlockReflowContext brc(aState.mPresContext, aState.mReflowState);
 
   // Reflow the float

layout/generic/nsBlockFrame.h

@@ -588,6 +588,10 @@ protected:
                        const nsRect&       aAdjustedAvailableSpace,
                        nsIFrame*           aFloat,
                        nsMargin&           aFloatMargin,
+                       // Whether the float's position
+                       // (aAdjustedAvailableSpace) has been pushed down
+                       // due to the presence of other floats.
+                       PRBool              aFloatPushedDown,
                        nsReflowStatus&     aReflowStatus);
 
   //----------------------------------------

layout/generic/nsBlockReflowState.cpp

@@ -822,8 +822,9 @@ nsBlockReflowState::FlowAndPlaceFloat(nsIFrame*       aFloat,
   // Reflow the float after computing its vertical position so it knows
   // where to break.
   nsMargin floatMargin; // computed margin
+  PRBool pushedDown = mY != saveY;
   mBlock->ReflowFloat(*this, adjustedAvailableSpace, aFloat,
-                      floatMargin, aReflowStatus);
+                      floatMargin, pushedDown, aReflowStatus);
   if (aFloat->GetPrevInFlow())
     floatMargin.top = 0;
   if (NS_FRAME_IS_NOT_COMPLETE(aReflowStatus))

layout/generic/nsIFrame.h

@@ -415,9 +415,11 @@ typedef PRUint32 nsReflowStatus;
   ((_completionStatus) | NS_INLINE_BREAK | NS_INLINE_BREAK_AFTER |      \
    NS_INLINE_MAKE_BREAK_TYPE(NS_STYLE_CLEAR_LINE))
 
-// The frame (not counting a continuation) did not fit in the available height and 
-// wasn't at the top of a page. If it was at the top of a page, then it is not 
-// possible to reflow it again with more height, so we don't set it in that case.
+// A frame is "truncated" if the part of the frame before the first
+// possible break point was unable to fit in the available vertical
+// space.  Therefore, the entire frame should be moved to the next page.
+// A frame that begins at the top of the page must never be "truncated".
+// Doing so would likely cause an infinite loop.
 #define NS_FRAME_TRUNCATED  0x0010
 #define NS_FRAME_IS_TRUNCATED(status) \
   (0 != ((status) & NS_FRAME_TRUNCATED))

layout/reftests/bugs/563584-8a-ref.html

@@ -0,0 +1,6 @@
+<!DOCTYPE HTML>
+<html class="reftest-print">
+<title>Test for setting and handling of mIsTopOfPage on floats</title>
+<body style="margin: 0">
+<div style="height: 2in; width: 1in; background: aqua"></div>
+<div style="height: 1in; width: 1in; background: yellow; page-break-before: always"></div>

layout/reftests/bugs/563584-8a.html

@@ -0,0 +1,8 @@
+<!DOCTYPE HTML>
+<html class="reftest-print">
+<title>Test for setting and handling of mIsTopOfPage on floats</title>
+<body style="margin: 0">
+<div style="float: left">
+  <div><div style="display:inline-block; vertical-align: top; height: 3in; width: 1in; background: aqua"></div></div>
+  <div><div style="display:inline-block; vertical-align: top; height: 1in; width: 1in; background: yellow"></div></div>
+</div>

layout/reftests/bugs/563584-8b-ref.html

@@ -0,0 +1,13 @@
+<!DOCTYPE HTML>
+<html class="reftest-print">
+<title>Test for setting and handling of mIsTopOfPage on floats</title>
+<body style="margin: 0">
+<div>
+  <div style="display:inline-block; vertical-align: top; height: 1in; width: 1in; background: fuchsia"></div
+  ><div style="display:inline-block; vertical-align: top; height: 2in; width: 1in; background: aqua"></div>
+</div>
+<div style="page-break-before:always">
+  <!-- FIXME: uncomment this when bug 511551 is fixed
+  <div style="display:inline-block; vertical-align: top; height: 1in; width: 1in"></div
+  >--><div style="display:inline-block; vertical-align: top; height: 1in; width: 1in; background: yellow"></div>
+</div>

layout/reftests/bugs/563584-8b.html

@@ -0,0 +1,9 @@
+<!DOCTYPE HTML>
+<html class="reftest-print">
+<title>Test for setting and handling of mIsTopOfPage on floats</title>
+<body style="margin: 0">
+<div style="float: left; height: 1in; width: 1in; background: fuchsia"></div>
+<div style="float: left">
+  <div><div style="display:inline-block; vertical-align: top; height: 3in; width: 1in; background: aqua"></div></div>
+  <div><div style="display:inline-block; vertical-align: top; height: 1in; width: 1in; background: yellow"></div></div>
+</div>

layout/reftests/bugs/563584-8c-ref.html

@@ -0,0 +1,7 @@
+<!DOCTYPE HTML>
+<html class="reftest-print">
+<title>Test for setting and handling of mIsTopOfPage on floats</title>
+<body style="margin: 0">
+<div style="height: 2in"></div>
+<div style="height: 2in; width: 1in; background: aqua; page-break-before: always"></div>
+<div style="height: 1in; width: 1in; background: yellow; page-break-before: always"></div>

layout/reftests/bugs/563584-8c.html

@@ -0,0 +1,9 @@
+<!DOCTYPE HTML>
+<html class="reftest-print">
+<title>Test for setting and handling of mIsTopOfPage on floats</title>
+<body style="margin: 0">
+<div style="height: 1in"></div>
+<div style="float: left">
+  <div><div style="display:inline-block; vertical-align: top; height: 3in; width: 1in; background: aqua"></div></div>
+  <div><div style="display:inline-block; vertical-align: top; height: 1in; width: 1in; background: yellow"></div></div>
+</div>

layout/reftests/bugs/563584-8d-ref.html

@@ -0,0 +1,8 @@
+<!DOCTYPE HTML>
+<html class="reftest-print">
+<title>Test for setting and handling of mIsTopOfPage on floats</title>
+<body style="margin: 0">
+<div style="height: 1in"></div>
+<div style="height: 1in; width: 1in; background: fuchsia"></div>
+<div style="height: 2in; width: 1in; background: aqua; page-break-before: always"></div>
+<div style="height: 1in; width: 1in; background: yellow; page-break-before: always"></div>

layout/reftests/bugs/563584-8d.html

@@ -0,0 +1,10 @@
+<!DOCTYPE HTML>
+<html class="reftest-print">
+<title>Test for setting and handling of mIsTopOfPage on floats</title>
+<body style="margin: 0">
+<div style="height: 1in"></div>
+<div style="float: left; height: 1in; width: 1in; background: fuchsia"></div>
+<div style="float: left">
+  <div><div style="display:inline-block; vertical-align: top; height: 3in; width: 1in; background: aqua"></div></div>
+  <div><div style="display:inline-block; vertical-align: top; height: 1in; width: 1in; background: yellow"></div></div>
+</div>

layout/reftests/bugs/reftest.list

@@ -1445,6 +1445,13 @@ random-if(!haveTestPlugin) == 546071-1.html 546071-1-ref.html
 == 563584-6-columns.html 563584-6-columns-ref.html
 == 563584-6-printing.html 563584-6-printing-ref.html
 == 563584-7.html 563584-7-ref.html
+# FIXME: It would be nice to have variants of these -8 tests for the
+# table narrowing quirk causing a change to mIsTopOfPage (though I'm not
+# entirely sure our behavior is the right one, either).
+== 563584-8a.html 563584-8a-ref.html
+== 563584-8b.html 563584-8b-ref.html
+== 563584-8c.html 563584-8c-ref.html
+== 563584-8d.html 563584-8d-ref.html
 == 564054-1.html 564054-1-ref.html
 == 564991-1.html 564991-1-ref.html
 == 565819-1.html 565819-ref.html