Bug 1404179 - Prepend rather than append frames from overflow list of prev-in-flow for ruby frames. r?dholbert draft
authorXidorn Quan <me@upsuper.org>
Tue, 03 Oct 2017 16:19:41 +1100
changeset 674525 3c58dd92e5e4483e2bb4595292a260e0c416dfe4
parent 674052 e267e5fc5b25c5cec3e94a1819a7ada440717ab2
child 734366 b6ea46dde586a013409c99d9b994dc96b38e285a
push id82873
push userxquan@mozilla.com
push dateTue, 03 Oct 2017 22:39:14 +0000
reviewersdholbert
bugs1404179
milestone58.0a1
Bug 1404179 - Prepend rather than append frames from overflow list of prev-in-flow for ruby frames. r?dholbert MozReview-Commit-ID: 1xUEf1S6GEK
layout/generic/nsContainerFrame.cpp
layout/generic/nsContainerFrame.h
layout/generic/nsRubyBaseContainerFrame.cpp
layout/generic/nsRubyFrame.cpp
layout/reftests/css-ruby/line-breaking-3-ref.html
layout/reftests/css-ruby/line-breaking-3.html
layout/reftests/css-ruby/reftest.list
--- a/layout/generic/nsContainerFrame.cpp
+++ b/layout/generic/nsContainerFrame.cpp
@@ -1511,57 +1511,76 @@ nsContainerFrame::PushChildren(nsIFrame*
   }
   else {
     // Add the frames to our overflow list
     SetOverflowFrames(tail);
   }
 }
 
 bool
-nsContainerFrame::MoveOverflowToChildList(nsIFrame* aLineContainer)
+nsContainerFrame::MoveOverflowToChildList()
 {
   bool result = false;
 
   // Check for an overflow list with our prev-in-flow
   nsContainerFrame* prevInFlow = (nsContainerFrame*)GetPrevInFlow();
   if (nullptr != prevInFlow) {
     AutoFrameListPtr prevOverflowFrames(PresContext(),
                                         prevInFlow->StealOverflowFrames());
     if (prevOverflowFrames) {
-      // Overflow frames from prev-in-flow should have been pushed to
-      // this frame directly if we have already existed, so there should
-      // be no frame in mFrames when we steal frames from overflow list
-      // of prev-in-flow. However, there are two special cases:
-      // * tables can have repeated header/footer frames at this point,
-      // * inline frames always push frames into overflow list rather
-      //   than next-in-flow, so that floats reparenting can be handled
-      //   properly below.
-      NS_ASSERTION(mFrames.IsEmpty() || IsTableFrame() || aLineContainer,
-                   "bad overflow list");
-      // If we are on a frame which has line container, we may need to
-      // reparent floats from prev-in-flow to our line container.
-      if (aLineContainer && aLineContainer->GetPrevContinuation()) {
-        ReparentFloatsForInlineChild(aLineContainer,
-                                     prevOverflowFrames->FirstChild(),
-                                     true);
-      }
+      // Tables are special; they can have repeated header/footer
+      // frames on mFrames at this point.
+      NS_ASSERTION(mFrames.IsEmpty() || IsTableFrame(), "bad overflow list");
       // When pushing and pulling frames we need to check for whether any
       // views need to be reparented.
       nsContainerFrame::ReparentFrameViewList(*prevOverflowFrames,
                                               prevInFlow, this);
       mFrames.AppendFrames(this, *prevOverflowFrames);
       result = true;
     }
   }
 
   // It's also possible that we have an overflow list for ourselves.
   return DrainSelfOverflowList() || result;
 }
 
 bool
+nsContainerFrame::MoveInlineOverflowToChildList(nsIFrame* aLineContainer)
+{
+  MOZ_ASSERT(aLineContainer,
+             "Must have line container for moving inline overflows");
+
+  bool result = false;
+
+  // Check for an overflow list with our prev-in-flow
+  if (auto prevInFlow = static_cast<nsContainerFrame*>(GetPrevInFlow())) {
+    AutoFrameListPtr prevOverflowFrames(PresContext(),
+                                        prevInFlow->StealOverflowFrames());
+    if (prevOverflowFrames) {
+      // We may need to reparent floats from prev-in-flow to our line
+      // container if the container has prev continuation.
+      if (aLineContainer->GetPrevContinuation()) {
+        ReparentFloatsForInlineChild(aLineContainer,
+                                     prevOverflowFrames->FirstChild(), true);
+      }
+      // When pushing and pulling frames we need to check for whether
+      // any views need to be reparented.
+      nsContainerFrame::ReparentFrameViewList(*prevOverflowFrames,
+                                              prevInFlow, this);
+      // Prepend overflow frames to the list.
+      mFrames.InsertFrames(this, nullptr, *prevOverflowFrames);
+      result = true;
+    }
+  }
+
+  // It's also possible that we have overflow list for ourselves.
+  return DrainSelfOverflowList() || result;
+}
+
+bool
 nsContainerFrame::DrainSelfOverflowList()
 {
   AutoFrameListPtr overflowFrames(PresContext(), StealOverflowFrames());
   if (overflowFrames) {
     mFrames.AppendFrames(nullptr, *overflowFrames);
     return true;
   }
   return false;
--- a/layout/generic/nsContainerFrame.h
+++ b/layout/generic/nsContainerFrame.h
@@ -624,22 +624,30 @@ protected:
 
   /**
    * Moves any frames on both the prev-in-flow's overflow list and the
    * receiver's overflow to the receiver's child list.
    *
    * Resets the overlist pointers to nullptr, and updates the receiver's child
    * count and content mapping.
    *
-   * @param aLineContainer the line container of the current frame if it
-   *          has one. nullptr if unrelated.
+   * @return true if any frames were moved and false otherwise
+   */
+  bool MoveOverflowToChildList();
+
+  /**
+   * Basically same as MoveOverflowToChildList, except that this is for
+   * handling inline children where children of prev-in-flow can be
+   * pushed to overflow list even if a next-in-flow exists.
+   *
+   * @param aLineContainer the line container of the current frame.
    *
    * @return true if any frames were moved and false otherwise
    */
-  bool MoveOverflowToChildList(nsIFrame* aLineContainer = nullptr);
+  bool MoveInlineOverflowToChildList(nsIFrame* aLineContainer);
 
   /**
    * Push aFromChild and its next siblings to the overflow list.
    *
    * @param aFromChild the first child frame to push. It is disconnected
    *          from aPrevSibling
    * @param aPrevSibling aFrameChild's previous sibling. Must not be null.
    *          It's an error to push a parent's first child frame.
--- a/layout/generic/nsRubyBaseContainerFrame.cpp
+++ b/layout/generic/nsRubyBaseContainerFrame.cpp
@@ -309,22 +309,22 @@ nsRubyBaseContainerFrame::Reflow(nsPresC
       aReflowInput.mLineLayout,
       "No line layout provided to RubyBaseContainerFrame reflow method.");
     return;
   }
 
   mDescendantLeadings.Reset();
 
   nsIFrame* lineContainer = aReflowInput.mLineLayout->LineContainerFrame();
-  MoveOverflowToChildList(lineContainer);
+  MoveInlineOverflowToChildList(lineContainer);
   // Ask text containers to drain overflows
   AutoRubyTextContainerArray textContainers(this);
   const uint32_t rtcCount = textContainers.Length();
   for (uint32_t i = 0; i < rtcCount; i++) {
-    textContainers[i]->MoveOverflowToChildList(lineContainer);
+    textContainers[i]->MoveInlineOverflowToChildList(lineContainer);
   }
 
   WritingMode lineWM = aReflowInput.mLineLayout->GetWritingMode();
   LogicalSize availSize(lineWM, aReflowInput.AvailableISize(),
                         aReflowInput.AvailableBSize());
 
   // We have a reflow state and a line layout for each RTC.
   // They are conceptually the state of the RTCs, but we don't actually
--- a/layout/generic/nsRubyFrame.cpp
+++ b/layout/generic/nsRubyFrame.cpp
@@ -110,17 +110,18 @@ nsRubyFrame::Reflow(nsPresContext* aPres
 
   if (!aReflowInput.mLineLayout) {
     NS_ASSERTION(aReflowInput.mLineLayout,
                  "No line layout provided to RubyFrame reflow method.");
     return;
   }
 
   // Grab overflow frames from prev-in-flow and its own.
-  MoveOverflowToChildList(aReflowInput.mLineLayout->LineContainerFrame());
+  MoveInlineOverflowToChildList(
+    aReflowInput.mLineLayout->LineContainerFrame());
 
   // Clear leadings
   mLeadings.Reset();
 
   // Since the ruby base container is going to reflow not only the ruby
   // base frames, but also the ruby text frames, and then *afterwards*
   // we're going to reflow the ruby text containers (which do not reflow
   // their children), we need to transfer NS_FRAME_IS_DIRTY status from
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-ruby/line-breaking-3-ref.html
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html lang="ja">
+<head>
+  <meta charset="UTF-8">
+  <title>Bug 1404179</title>
+  <link rel="stylesheet" href="common.css">
+  <style>
+    #test {
+      border: 1px solid;
+      text-align: center;
+      width: 2.5em;
+    }
+  </style>
+</head>
+<body>
+  <div id="test">
+    <p>一二三四五六七八</p>
+    <p>一二三四五六七八</p>
+  </div>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/css-ruby/line-breaking-3.html
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<html lang="ja">
+<head>
+  <meta charset="UTF-8">
+  <title>Bug 1404179</title>
+  <link rel="stylesheet" href="common.css">
+  <style>
+    #test {
+      border: 1px solid;
+      text-align: center;
+    }
+  </style>
+</head>
+<body>
+  <div id="test">
+    <p><ruby><rb>一<rb>二<rb>三<rb>四<rb>五<rb>六<rb>七<rb>八</ruby></p>
+    <p><ruby><rbc>一</rbc><rbc>二</rbc><rbc>三</rbc><rbc>四</rbc><rbc>五</rbc><rbc>六</rbc><rbc>七</rbc><rbc>八</rbc></ruby></p>
+  </div>
+  <script>
+    let div = document.getElementById("test");
+    document.body.offsetHeight;
+    test.style.width = "2.5em";
+    document.body.offsetHeight;
+    test.style.width = "4.5em";
+    document.body.offsetHeight;
+    test.style.width = "2.5em";
+  </script>
+</body>
+</html>
--- a/layout/reftests/css-ruby/reftest.list
+++ b/layout/reftests/css-ruby/reftest.list
@@ -22,16 +22,17 @@ test-pref(dom.meta-viewport.enabled,true
 == intra-level-whitespace-3.html intra-level-whitespace-3-ref.html
 == intrinsic-isize-1.html intrinsic-isize-1-ref.html
 == intrinsic-isize-2.html intrinsic-isize-2-ref.html
 == justification-1.html justification-1-ref.html
 == justification-2.html justification-2-ref.html
 fuzzy-if(winWidget,255,792) == lang-specific-style-1.html lang-specific-style-1-ref.html # bug 1134947
 == line-breaking-1.html line-breaking-1-ref.html
 == line-breaking-2.html line-breaking-2-ref.html
+== line-breaking-3.html line-breaking-3-ref.html
 fuzzy-if(/^Windows\x20NT\x2010\.0/.test(http.oscpu),3,2) == line-break-suppression-1.html line-break-suppression-1-ref.html
 fuzzy-if(/^Windows\x20NT\x2010\.0/.test(http.oscpu),3,2) == line-break-suppression-2.html line-break-suppression-2-ref.html
 == line-break-suppression-3.html line-break-suppression-3-ref.html
 == line-break-suppression-4.html line-break-suppression-4-ref.html
 == line-break-suppression-5.html line-break-suppression-5-ref.html
 == line-height-1.html line-height-1-ref.html
 == line-height-2.html line-height-2-ref.html
 == line-height-3.html line-height-3-ref.html