Bug 488725: Don't position floats in a nowrap context until hitting a break opportunity. r=dbaron

Differential Revision: https://phabricator.services.mozilla.com/D3900

layout/generic/BlockReflowInput.h

@@ -382,6 +382,10 @@ public:
   // being N^2.
   nsFloatCacheFreeList mBelowCurrentLineFloats;
 
+  // The list of floats that are waiting on a break opportunity in order to be
+  // placed, since we're on a nowrap context.
+  nsTArray<nsIFrame*> mNoWrapFloats;
+
   nscoord mMinLineHeight;
 
   int32_t mLineNumber;

layout/generic/nsBlockFrame.cpp

@@ -3971,6 +3971,7 @@ nsBlockFrame::ReflowInlineFrames(BlockReflowInput& aState,
           aState.FloatManager()->PopState(&floatManagerState);
           // Clear out float lists
           aState.mCurrentLineFloats.DeleteAll();
+          MOZ_ASSERT(aState.mNoWrapFloats.IsEmpty());
           aState.mBelowCurrentLineFloats.DeleteAll();
         }
 
@@ -4596,6 +4597,9 @@ nsBlockFrame::PlaceLine(BlockReflowInput& aState,
                         nscoord& aAvailableSpaceBSize,
                         bool* aKeepReflowGoing)
 {
+  // Try to position the floats in a nowrap context.
+  aLineLayout.FlushNoWrapFloats();
+
   // Trim extra white-space from the line before placing the frames
   aLineLayout.TrimTrailingWhiteSpace();
 

layout/generic/nsLineLayout.cpp

@@ -264,6 +264,7 @@ nsLineLayout::EndLineReflow()
                (!mSpansAllocated && !mSpansFreed && !mSpanFreeList &&
                 !mFramesAllocated && !mFramesFreed && !mFrameFreeList),
                "Allocated frames or spans on non-base line layout?");
+  MOZ_ASSERT(mRootSpan == mCurrentSpan);
 
   UnlinkFrame(mRootSpan->mFrame);
   mCurrentSpan = mRootSpan = nullptr;
@@ -457,6 +458,12 @@ nsLineLayout::EndSpan(nsIFrame* aFrame)
   printf(": EndSpan width=%d\n", mCurrentSpan->mICoord - mCurrentSpan->mIStart);
 #endif
   PerSpanData* psd = mCurrentSpan;
+  MOZ_ASSERT(psd->mParent, "We never call this on the root");
+
+  if (psd->mNoWrap && !psd->mParent->mNoWrap) {
+    FlushNoWrapFloats();
+  }
+
   nscoord iSizeResult = psd->mLastFrame ? (psd->mICoord - psd->mIStart) : 0;
 
   mSpanDepth--;
@@ -954,27 +961,14 @@ nsLineLayout::ReflowFrame(nsIFrame* aFrame,
       pfd->mSkipWhenTrimmingWhitespace = true;
       nsIFrame* outOfFlowFrame = nsLayoutUtils::GetFloatFromPlaceholder(aFrame);
       if (outOfFlowFrame) {
-        // Add mTrimmableISize to the available width since if the line ends
-        // here, the width of the inline content will be reduced by
-        // mTrimmableISize.
-        nscoord availableISize = psd->mIEnd - (psd->mICoord - mTrimmableISize);
-        if (psd->mNoWrap) {
-          // If we place floats after inline content where there's
-          // no break opportunity, we don't know how much additional
-          // width is required for the non-breaking content after the float,
-          // so we can't know whether the float plus that content will fit
-          // on the line. So for now, don't place floats after inline
-          // content where there's no break opportunity. This is incorrect
-          // but hopefully rare. Fixing it will require significant
-          // restructuring of line layout.
-          // We might as well allow zero-width floats to be placed, though.
-          availableISize = 0;
+        if (psd->mNoWrap &&
+            !LineIsEmpty() && // We can always place floats in an empty line.
+            !GetOutermostLineLayout()->mBlockRI->mFlags.mCanHaveTextOverflow) {
+          // We'll do this at the next break opportunity.
+          RecordNoWrapFloat(outOfFlowFrame);
+        } else {
+          placedFloat = TryToPlaceFloat(outOfFlowFrame);
         }
-        placedFloat = GetOutermostLineLayout()->
-          AddFloat(outOfFlowFrame, availableISize);
-        NS_ASSERTION(!(outOfFlowFrame->IsLetterFrame() &&
-                       GetFirstLetterStyleOK()),
-                    "FirstLetterStyle set on line with floating first letter");
       }
     }
     else if (isText) {
@@ -1119,8 +1113,8 @@ nsLineLayout::ReflowFrame(nsIFrame* aFrame,
         VerticalAlignFrames(span);
       }
 
-      if (!continuingTextRun) {
-        if (!psd->mNoWrap && (!LineIsEmpty() || placedFloat)) {
+      if (!continuingTextRun && !psd->mNoWrap) {
+        if (!LineIsEmpty() || placedFloat) {
           // record soft break opportunity after this content that can't be
           // part of a text run. This is not a text frame so we know
           // that offset INT32_MAX means "after the content".
@@ -1505,6 +1499,60 @@ nsLineLayout::DumpPerSpanData(PerSpanData* psd, int32_t aIndent)
 }
 #endif
 
+void
+nsLineLayout::RecordNoWrapFloat(nsIFrame* aFloat)
+{
+  GetOutermostLineLayout()->mBlockRI->mNoWrapFloats.AppendElement(aFloat);
+}
+
+void
+nsLineLayout::FlushNoWrapFloats()
+{
+  auto& noWrapFloats = GetOutermostLineLayout()->mBlockRI->mNoWrapFloats;
+  for (nsIFrame* floatedFrame : noWrapFloats) {
+    TryToPlaceFloat(floatedFrame);
+  }
+  noWrapFloats.Clear();
+}
+
+bool
+nsLineLayout::TryToPlaceFloat(nsIFrame* aFloat)
+{
+  // Add mTrimmableISize to the available width since if the line ends here, the
+  // width of the inline content will be reduced by mTrimmableISize.
+  nscoord availableISize = mCurrentSpan->mIEnd - (mCurrentSpan->mICoord - mTrimmableISize);
+  NS_ASSERTION(!(aFloat->IsLetterFrame() && GetFirstLetterStyleOK()),
+              "FirstLetterStyle set on line with floating first letter");
+  return GetOutermostLineLayout()->AddFloat(aFloat, availableISize);
+}
+
+bool
+nsLineLayout::NotifyOptionalBreakPosition(nsIFrame* aFrame,
+                                          int32_t aOffset,
+                                          bool aFits,
+                                          gfxBreakPriority aPriority)
+{
+  MOZ_ASSERT(!aFits || !mNeedBackup,
+             "Shouldn't be updating the break position with a break that fits "
+             "after we've already flagged an overrun");
+  MOZ_ASSERT(mCurrentSpan, "Should be doing line layout");
+  if (mCurrentSpan->mNoWrap) {
+    FlushNoWrapFloats();
+  }
+
+  // Remember the last break position that fits; if there was no break that fit,
+  // just remember the first break
+  if ((aFits && aPriority >= mLastOptionalBreakPriority) ||
+      !mLastOptionalBreakFrame) {
+    mLastOptionalBreakFrame = aFrame;
+    mLastOptionalBreakFrameOffset = aOffset;
+    mLastOptionalBreakPriority = aPriority;
+  }
+  return aFrame && mForceBreakFrame == aFrame &&
+    mForceBreakFrameOffset == aOffset;
+}
+
+
 #define VALIGN_OTHER  0
 #define VALIGN_TOP    1
 #define VALIGN_BOTTOM 2

layout/generic/nsLineLayout.h

@@ -251,21 +251,21 @@ public:
    * @return true if we are actually reflowing with forced break position and we
    * should break here
    */
-  bool NotifyOptionalBreakPosition(nsIFrame* aFrame, int32_t aOffset,
-                                   bool aFits, gfxBreakPriority aPriority) {
-    NS_ASSERTION(!aFits || !mNeedBackup,
-                  "Shouldn't be updating the break position with a break that fits after we've already flagged an overrun");
-    // Remember the last break position that fits; if there was no break that fit,
-    // just remember the first break
-    if ((aFits && aPriority >= mLastOptionalBreakPriority) ||
-        !mLastOptionalBreakFrame) {
-      mLastOptionalBreakFrame = aFrame;
-      mLastOptionalBreakFrameOffset = aOffset;
-      mLastOptionalBreakPriority = aPriority;
-    }
-    return aFrame && mForceBreakFrame == aFrame &&
-      mForceBreakFrameOffset == aOffset;
-  }
+  bool NotifyOptionalBreakPosition(nsIFrame* aFrame,
+                                   int32_t aOffset,
+                                   bool aFits,
+                                   gfxBreakPriority aPriority);
+
+  // Tries to place a float, and records whether the float actually was placed.
+  bool TryToPlaceFloat(nsIFrame* aFloat);
+
+  // Records a floating frame in a nowrap context for it to be placed on the
+  // next break opportunity.
+  void RecordNoWrapFloat(nsIFrame* aFloat);
+
+  // Tries to place the floats from the nowrap context.
+  void FlushNoWrapFloats();
+
   /**
    * Like NotifyOptionalBreakPosition, but here it's OK for mNeedBackup
    * to be set, because the caller is merely pruning some saved break position(s)

layout/reftests/bugs/reftest.list

@@ -1151,7 +1151,7 @@ fuzzy-if(skiaContent,0-1,0-3280) == 438987-2c.html 438987-2-ref.html
 fuzzy-if(skiaContent,0-1,0-1) == 440112.html 440112-ref.html
 == 440149-1.html 440149-1-ref.html
 == 441259-1.html 441259-1-ref.html
-fails == 441259-2.html 441259-2-ref.html # bug 441400
+== 441259-2.html 441259-2-ref.html
 fuzzy-if(skiaContent,0-1,0-3) == 442542-1.html 442542-1-ref.html
 == 444015-1.html 444015-1-ref.html
 == 444375-1.html 444375-1-ref.html

layout/reftests/css-grid/grid-container-baselines-003-ref.html

@@ -87,7 +87,7 @@ A<div class="ib">
 <div class="ib" style="width:200px;">
   <div class="ib" style="height:120px; width:76px; white-space:nowrap"><span
      class="a f" style="width:30px; margin-top:7px; padding-top:5px; padding-bottom:18px;">A<br>B</span><span
-     class="a f" style="float:right; width:30px; position:relative; left:-4px; padding-bottom:22px; margin-top:-100px">A<br>B</span>
+     class="a f" style="float:right; width:30px; position:relative; left:-4px; padding-bottom:22px;">A<br>B</span>
   </div><span class="i a f" style="width:54px">C<br>D</span><span
      class="a f" style="width:54px; padding-top:20px;">E<br>F</span>
 </div>

layout/reftests/floats/float-nowrap-1-notref.html

@@ -0,0 +1,18 @@
+<!doctype html>
+<style>
+  div {
+    width: 10ch;
+    white-space: nowrap;
+    font-family: monospace;
+  }
+  span {
+    float: right;
+    width: 5ch;
+    height: 5ch;
+    background: blue;
+  }
+</style>
+<div>
+  <span></span>
+  Some text that overflows my parent.
+</div>

layout/reftests/floats/float-nowrap-1.html

@@ -0,0 +1,18 @@
+<!doctype html>
+<style>
+  div {
+    width: 10ch;
+    white-space: nowrap;
+    font-family: monospace;
+  }
+  span {
+    float: right;
+    width: 5ch;
+    height: 5ch;
+    background: blue;
+  }
+</style>
+<div>
+  Some text that overflows my parent.
+  <span></span>
+</div>

layout/reftests/floats/float-nowrap-2.html

@@ -0,0 +1,19 @@
+<!doctype html>
+<style>
+  div {
+    width: 10ch;
+    white-space: nowrap;
+    font-family: monospace;
+  }
+  span {
+    float: right;
+    width: 5ch;
+    height: 5ch;
+    background: blue;
+  }
+</style>
+<div>
+  Some text that
+  <span></span>
+  overflows my parent.
+</div>

layout/reftests/floats/float-nowrap-3-ref.html

@@ -0,0 +1,23 @@
+<!doctype html>
+<style>
+  div {
+    width: 10ch;
+    font-family: monospace;
+  }
+  .float {
+    float: right;
+    width: 5ch;
+    height: 5ch;
+    background: blue;
+  }
+  .nowrap {
+    white-space: nowrap;
+  }
+</style>
+<div>
+  Some
+  <span class="nowrap">
+    text that overflows my parent.
+  </span>
+  <span class="float"></span>
+</div>

layout/reftests/floats/float-nowrap-3.html

@@ -0,0 +1,22 @@
+<!doctype html>
+<style>
+  div {
+    width: 10ch;
+    font-family: monospace;
+  }
+  .float {
+    float: right;
+    width: 5ch;
+    height: 5ch;
+    background: blue;
+  }
+  .nowrap {
+    white-space: nowrap;
+  }
+</style>
+<div>
+  Some
+  <span class="nowrap">
+    text that overflows <span class="float"></span> my parent.
+  </span>
+</div>

layout/reftests/floats/float-nowrap-4-ref.html

@@ -0,0 +1,23 @@
+<!doctype html>
+<style>
+  div {
+    width: 10ch;
+    font-family: monospace;
+  }
+  .float {
+    float: right;
+    width: 5ch;
+    height: 5ch;
+    background: blue;
+  }
+  .nowrap {
+    white-space: nowrap;
+  }
+</style>
+<div>
+  Some
+  <span class="float"></span>
+  <span class="nowrap">
+    text that overflows my parent.
+  </span>
+</div>

layout/reftests/floats/float-nowrap-4.html

@@ -0,0 +1,22 @@
+<!doctype html>
+<style>
+  div {
+    width: 10ch;
+    font-family: monospace;
+  }
+  .float {
+    float: right;
+    width: 5ch;
+    height: 5ch;
+    background: blue;
+  }
+  .nowrap {
+    white-space: nowrap;
+  }
+</style>
+<div>
+  Some
+  <span class="nowrap">
+    <span class="float"></span> text that overflows my parent.
+  </span>
+</div>

layout/reftests/floats/float-nowrap-5-ref.html

@@ -0,0 +1,2 @@
+<!doctype html>
+<div>Hello Kittie

layout/reftests/floats/float-nowrap-5.html

@@ -0,0 +1,10 @@
+<!doctype html>
+<style>
+  div {
+    white-space: nowrap;
+  }
+  span {
+    float: left;
+  }
+</style>
+<div>Kittie<span>Hello&nbsp;</span>

layout/reftests/floats/float-nowrap-6.html

@@ -0,0 +1,10 @@
+<!doctype html>
+<style>
+  div {
+    white-space: nowrap;
+  }
+  span {
+    float: left;
+  }
+</style>
+<div><span>Hello&nbsp;</span>Kittie

layout/reftests/floats/float-nowrap-7.html

@@ -0,0 +1,20 @@
+<!doctype html>
+<style>
+  div {
+    width: 10ch;
+    white-space: nowrap;
+    font-family: monospace;
+  }
+  .nowrap {
+    white-space: nowrap;
+  }
+  .float {
+    float: right;
+    width: 5ch;
+    height: 5ch;
+    background: blue;
+  }
+</style>
+<div>
+  <span class="nowrap">S<div class="float"></div><span>ome</span> text that overflows my parent.</span>
+</div>

layout/reftests/floats/float-nowrap-8.html

@@ -0,0 +1,18 @@
+<!doctype html>
+<style>
+  div {
+    width: 10ch;
+    white-space: nowrap;
+    font-family: monospace;
+  }
+  span {
+    float: right;
+    width: 5ch;
+    height: 5ch;
+    background: blue;
+  }
+</style>
+<div>
+  Some <span></span>
+  text that overflows my parent.
+</div>

layout/reftests/floats/float-nowrap-9.html

@@ -0,0 +1,22 @@
+<!doctype html>
+<style>
+  div {
+    width: 10ch;
+    font-family: monospace;
+  }
+  .float {
+    float: right;
+    width: 5ch;
+    height: 5ch;
+    background: blue;
+  }
+  .nowrap {
+    white-space: nowrap;
+  }
+</style>
+<div>
+  Some
+  <span class="nowrap">
+    text <span class="float"></span> that overflows my parent.
+  </span>
+</div>

layout/reftests/floats/reftest.list

@@ -117,3 +117,13 @@ random-if(/^Windows\x20NT\x206\.1/.test(http.oscpu)) == float-in-rtl-slr-2d.html
 == float-in-rtl-slr-4b.html float-in-rtl-slr-4-ref.html
 == float-in-rtl-slr-4c.html float-in-rtl-slr-4-ref.html
 == float-in-rtl-slr-4d.html float-in-rtl-slr-4-ref.html
+!= float-nowrap-1.html float-nowrap-1-notref.html
+== float-nowrap-2.html float-nowrap-1.html
+== float-nowrap-3.html float-nowrap-3-ref.html
+!= float-nowrap-3.html float-nowrap-4.html
+== float-nowrap-4.html float-nowrap-4-ref.html
+== float-nowrap-5.html float-nowrap-5-ref.html
+== float-nowrap-6.html float-nowrap-5-ref.html
+== float-nowrap-7.html float-nowrap-1.html
+== float-nowrap-8.html float-nowrap-1.html
+== float-nowrap-9.html float-nowrap-3-ref.html

testing/web-platform/meta/css/css-text/line-breaking/line-breaking-012.html.ini

@@ -1,2 +0,0 @@
-[line-breaking-012.html]
-  expected: FAIL