Bug 1503420 Part 2 - Fix appending a subtree with column-span descendants under the ::moz-column-content. r=emilio
authorTing-Yu Lin <tlin@mozilla.com>
Tue, 15 Jan 2019 03:48:02 +0000
changeset 453963 3eafb595be4fce04627edba4b9686ac44baed45f
parent 453962 918042a8eb34fe65335f96b5ae1226409361d4f6
child 453964 83f6feddbba0eb52115774fef93c4ed4274161aa
push id35382
push userdvarga@mozilla.com
push dateWed, 16 Jan 2019 04:47:18 +0000
treeherdermozilla-central@b46d5d689c8d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1503420, 1504053
milestone66.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1503420 Part 2 - Fix appending a subtree with column-span descendants under the ::moz-column-content. r=emilio The elements under <body> are treat as content append, so their frames will be construct by nsCSSFrameConstructor::ContentAppended. This patch fixed only the simple "append" case which is appending to the last continuation of ::moz-column-content. For other more complex appending or inserting cases, we might need to reframe (bug 1504053). Differential Revision: https://phabricator.services.mozilla.com/D16076
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsCSSFrameConstructor.h
layout/base/tests/bug1503420.html
layout/base/tests/mochitest.ini
layout/base/tests/test_frame_reconstruction_for_column_span.html
testing/web-platform/meta/css/css-multicol/multicol-span-all-007.html.ini
testing/web-platform/meta/css/css-multicol/multicol-span-all-dynamic-add-010.html.ini
testing/web-platform/tests/css/css-multicol/multicol-span-all-007-ref.html
testing/web-platform/tests/css/css-multicol/multicol-span-all-007.html
testing/web-platform/tests/css/css-multicol/multicol-span-all-dynamic-add-010-ref.html
testing/web-platform/tests/css/css-multicol/multicol-span-all-dynamic-add-010.html
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -372,16 +372,22 @@ static inline bool IsDisplayContents(con
  * True if aFrame is an instance of an SVG frame class or is an inline/block
  * frame being used for SVG text.
  */
 static bool IsFrameForSVG(const nsIFrame* aFrame) {
   return aFrame->IsFrameOfType(nsIFrame::eSVG) ||
          nsSVGUtils::IsInSVGTextSubtree(aFrame);
 }
 
+static bool IsLastContinuationForColumnContent(const nsIFrame* aFrame) {
+  MOZ_ASSERT(aFrame);
+  return aFrame->Style()->GetPseudo() == nsCSSAnonBoxes::columnContent() &&
+         !aFrame->GetNextContinuation();
+}
+
 /**
  * Returns true iff aFrame explicitly prevents its descendants from floating
  * (at least, down to the level of descendants which themselves are
  * float-containing blocks -- those will manage the floating status of any
  * lower-level descendents inside them, of course).
  */
 static bool ShouldSuppressFloatingOfDescendants(nsIFrame* aFrame) {
   return aFrame->IsFlexOrGridContainer() || aFrame->IsXULBoxFrame() ||
@@ -5845,30 +5851,30 @@ static nsIFrame* GetInsertNextSibling(ns
                                       nsIFrame* aPrevSibling) {
   if (aPrevSibling) {
     return aPrevSibling->GetNextSibling();
   }
 
   return aParentFrame->PrincipalChildList().FirstChild();
 }
 
-/**
- * This function is called by ContentAppended() and ContentInserted() when
- * appending flowed frames to a parent's principal child list. It handles the
- * case where the parent is the trailing inline of an {ib} split.
- */
 void nsCSSFrameConstructor::AppendFramesToParent(
     nsFrameConstructorState& aState, nsContainerFrame* aParentFrame,
     nsFrameItems& aFrameList, nsIFrame* aPrevSibling, bool aIsRecursiveCall) {
   MOZ_ASSERT(
       !IsFramePartOfIBSplit(aParentFrame) || !GetIBSplitSibling(aParentFrame) ||
           !GetIBSplitSibling(aParentFrame)->PrincipalChildList().FirstChild(),
       "aParentFrame has a ib-split sibling with kids?");
   MOZ_ASSERT(!aPrevSibling || aPrevSibling->GetParent() == aParentFrame,
              "Parent and prevsibling don't match");
+  MOZ_ASSERT(
+      !aParentFrame->HasAnyStateBits(NS_FRAME_HAS_MULTI_COLUMN_ANCESTOR) ||
+          !IsFramePartOfIBSplit(aParentFrame),
+      "We should have wiped aParentFrame in WipeContainingBlock() "
+      "if it's part of an IB split!");
 
   nsIFrame* nextSibling = ::GetInsertNextSibling(aParentFrame, aPrevSibling);
 
   NS_ASSERTION(nextSibling || !aParentFrame->GetNextContinuation() ||
                    !aParentFrame->GetNextContinuation()
                         ->PrincipalChildList()
                         .FirstChild() ||
                    aIsRecursiveCall,
@@ -5933,16 +5939,42 @@ void nsCSSFrameConstructor::AppendFrames
       // Recurse so we create new ib siblings as needed for aParentFrame's
       // parent
       return AppendFramesToParent(aState, aParentFrame->GetParent(), ibSiblings,
                                   aParentFrame, true);
     }
     return;
   }
 
+  // If we're appending a list of frames at the last continuations of a
+  // ::-moz-column-content, we may need to create column-span siblings for them.
+  if (!nextSibling && IsLastContinuationForColumnContent(aParentFrame)) {
+    // Extract any initial non-column-span kids, and append them to
+    // ::-moz-column-content's child list.
+    nsFrameList initialNonColumnSpanKids =
+        aFrameList.Split([](nsIFrame* f) { return f->IsColumnSpan(); });
+    AppendFrames(aParentFrame, kPrincipalList, initialNonColumnSpanKids);
+
+    if (aFrameList.IsEmpty()) {
+      // No more kids to process (there weren't any column-span kids).
+      return;
+    }
+
+    nsFrameList columnSpanSiblings = CreateColumnSpanSiblings(
+        aState, aParentFrame, aFrameList,
+        aParentFrame->IsAbsPosContainingBlock() ? aParentFrame : nullptr);
+    FinishBuildingColumns(aState,
+                          GetMultiColumnContainingBlockFor(aParentFrame),
+                          aParentFrame, columnSpanSiblings);
+
+    MOZ_ASSERT(columnSpanSiblings.IsEmpty(),
+               "The column-span siblings should be moved to the proper place!");
+    return;
+  }
+
   // Insert the frames after our aPrevSibling
   InsertFrames(aParentFrame, kPrincipalList, aPrevSibling, aFrameList);
 }
 
 // This gets called to see if the frames corresponding to aSibling and aContent
 // should be siblings in the frame tree. Although (1) rows and cols, (2) row
 // groups and col groups, (3) row groups and captions, (4) legends and content
 // inside fieldsets, (5) popups and other kids of the menu are siblings from a
@@ -10588,17 +10620,17 @@ void nsCSSFrameConstructor::ConstructBlo
     // Create the outside bullet on ColumnSetWrapper so that the position of
     // the bullet is correct.
     blockFrameToCreateBullet = static_cast<nsBlockFrame*>(*aNewFrame);
   }
 
   CreateBulletFrameForListItemIfNeeded(blockFrameToCreateBullet);
 
   if (childItems.IsEmpty()) {
-    // No more column-span kids need to be processed.
+    // No more kids to process (there weren't any column-span kids).
     return;
   }
 
   nsFrameList columnSpanSiblings = CreateColumnSpanSiblings(
       aState, blockFrame, childItems, aPositionedFrameForAbsPosContainer);
 
   if (needsColumn) {
     // We're constructing a column container; need to finish building it.
--- a/layout/base/nsCSSFrameConstructor.h
+++ b/layout/base/nsCSSFrameConstructor.h
@@ -445,21 +445,29 @@ class nsCSSFrameConstructor final : publ
 
   // aParentFrame may be null; this method doesn't use it directly in any case.
   void CreateGeneratedContentItem(nsFrameConstructorState& aState,
                                   nsContainerFrame* aParentFrame,
                                   Element& aOriginatingElement, ComputedStyle&,
                                   CSSPseudoElementType aPseudoElement,
                                   FrameConstructionItemList& aItems);
 
-  // This method can change aFrameList: it can chop off the beginning and put
-  // it in aParentFrame while putting the remainder into a ib-split sibling of
-  // aParentFrame.  aPrevSibling must be the frame after which aFrameList is to
-  // be placed on aParentFrame's principal child list.  It may be null if
-  // aFrameList is being added at the beginning of the child list.
+  // This method is called by ContentAppended() and ContentRangeInserted() when
+  // appending flowed frames to a parent's principal child list. It handles the
+  // case where the parent is the trailing inline of an ib-split or is the last
+  // continuation of a ::-moz-column-content in an nsColumnSetFrame.
+  //
+  // This method can change aFrameList: it can chop off the beginning and put it
+  // in aParentFrame while either putting the remainder into an ib-split sibling
+  // of aParentFrame or creating aParentFrame's column-span siblings for the
+  // remainder.
+  //
+  // aPrevSibling must be the frame after which aFrameList is to be placed on
+  // aParentFrame's principal child list. It may be null if aFrameList is being
+  // added at the beginning of the child list.
   void AppendFramesToParent(nsFrameConstructorState& aState,
                             nsContainerFrame* aParentFrame,
                             nsFrameItems& aFrameList, nsIFrame* aPrevSibling,
                             bool aIsRecursiveCall = false);
 
   // BEGIN TABLE SECTION
   /**
    * Construct a table wrapper frame. This is the FrameConstructionData
new file mode 100644
--- /dev/null
+++ b/layout/base/tests/bug1503420.html
@@ -0,0 +1,77 @@
+<!DOCTYPE html>
+<html>
+  <meta charset="utf-8">
+  <title>
+    Test for Bug 1503420: Test we don't reframe multi-column containing block
+    when appending a block containing a spanner kid.
+  </title>
+  <link rel="author" title="Ting-Yu Lin" href="tlin@mozilla.com">
+  <link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+  <script src="/tests/SimpleTest/SimpleTest.js"></script>
+
+  <script>
+  const utils = SpecialPowers.getDOMWindowUtils(window);
+
+  function appendBlock() {
+    // Create a subtree like the following, and append it to columns.
+    // <div>
+    //   <h3>spanner</h3>
+    //   block2
+    // </div>
+    var spanner = document.createElement("h3");
+    var spannerText = document.createTextNode("spanner");
+    spanner.appendChild(spannerText);
+
+    var block2 = document.createElement("div");
+    var block2Text = document.createTextNode("block2");
+    block2.appendChild(spanner);
+    block2.appendChild(block2Text)
+
+    var column = document.getElementById("column");
+    column.appendChild(block2);
+  }
+
+  function runTest() {
+    document.documentElement.offsetTop;
+    // We expected to construct 6 more frames.
+    // 1) Block frame for <div>
+    // 2) Block frame for <h3>
+    // 3) Text frame for "spanner"
+    // 4) Text frame for "block2"
+    // 5) Column-span wrapper for <h3>, which is a sibling of <div>
+    // 6) Column-span wrapper for 5), which is a sibling of <article>
+    // Note: creating a continuation frame doesn't increase the count.
+    const expectedFrameConstructCount = utils.framesConstructed + 6;
+
+    appendBlock();
+    document.documentElement.offsetTop;
+
+    window.parent.postMessage({
+      "actualResult": utils.framesConstructed,
+      "expectedResult": expectedFrameConstructCount,
+      "message": "We shouldn't construct unexpected frames.",
+    }, "*");
+
+    window.parent.postMessage({"done": true}, "*");
+  }
+  </script>
+
+  <style>
+  #column {
+    column-count: 3;
+    column-rule: 6px solid;
+    width: 400px;
+    outline: 1px solid black;
+  }
+  h3 {
+    column-span: all;
+    outline: 1px solid blue;
+  }
+  </style>
+
+  <body onload="runTest();">
+    <article id="column">
+      <div>block1</div>
+    </article>
+  </body>
+</html>
--- a/layout/base/tests/mochitest.ini
+++ b/layout/base/tests/mochitest.ini
@@ -148,16 +148,19 @@ support-files = bug1226904.html
 [test_emulateMedium.html]
 [test_event_target_iframe_oop.html]
 skip-if = e10s # bug 1020135, nested oop iframes not supported
 support-files = bug921928_event_target_iframe_apps_oop.html
 [test_event_target_radius.html]
 skip-if = toolkit == 'android' # Bug 1355836
 [test_flush_on_paint.html]
 skip-if = true # Bug 688128
+[test_frame_reconstruction_for_column_span.html]
+support-files =
+  bug1503420.html
 [test_frame_reconstruction_for_pseudo_elements.html]
 [test_frame_reconstruction_for_svg_transforms.html]
 [test_frame_reconstruction_scroll_restore.html]
 [test_getBoxQuads_convertPointRectQuad.html]
 support-files =
   file_getBoxQuads_convertPointRectQuad_frame1.html
   file_getBoxQuads_convertPointRectQuad_frame2.html
 [test_getClientRects_emptytext.html]
new file mode 100644
--- /dev/null
+++ b/layout/base/tests/test_frame_reconstruction_for_column_span.html
@@ -0,0 +1,45 @@
+<!DOCTYPE html>
+<html>
+  <meta charset="utf-8">
+  <title>
+    Loader to test frame construction for column span.
+  </title>
+  <link rel="author" title="Ting-Yu Lin" href="tlin@mozilla.com">
+  <link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+  <script src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+
+  <script>
+  SimpleTest.waitForExplicitFinish();
+
+  function receiveMessage(event)
+  {
+    console.log(event);
+    if (event.data.done) {
+      window.removeEventListener("message", receiveMessage);
+      SimpleTest.finish();
+      return;
+    }
+
+    is(event.data.actualResult, event.data.expectedResult,
+       event.data.message);
+  }
+
+  function prepareTest() {
+    window.addEventListener("message", receiveMessage);
+
+    SpecialPowers.pushPrefEnv({
+      "set": [
+        ["layout.css.column-span.enabled", true]
+      ]
+    }, function () {
+      let iframe = document.getElementById("testFrame");
+      iframe.src = "bug1503420.html";
+    });
+  }
+  </script>
+
+  <body onload="prepareTest()">
+    <iframe id="testFrame" height="500" width="500"></iframe>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/css/css-multicol/multicol-span-all-007.html.ini
@@ -0,0 +1,2 @@
+[multicol-span-all-007.html]
+  prefs: [layout.css.column-span.enabled:true]
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/css/css-multicol/multicol-span-all-dynamic-add-010.html.ini
@@ -0,0 +1,2 @@
+[multicol-span-all-dynamic-add-010.html]
+  prefs: [layout.css.column-span.enabled:true]
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-multicol/multicol-span-all-007-ref.html
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<html>
+  <meta charset="utf-8">
+  <title>CSS Multi-column Layout Test Reference: Test column-span:all when the body tag is the multi-column container</title>
+  <link rel="author" title="Ting-Yu Lin" href="tlin@mozilla.com">
+  <link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+
+  <style>
+  body {
+    column-count: 1;
+    column-rule: 6px solid;
+    width: 400px;
+    outline: 1px solid black;
+  }
+  h3 {
+    /* column-count: 1 makes this behave like a real spanner. */
+    outline: 1px solid blue;
+  }
+  </style>
+
+  <body>
+    <div>block1</div>
+    <div>
+      <h3>spanner</h3>
+    </div>
+    <div>block2</div>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-multicol/multicol-span-all-007.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<html>
+  <meta charset="utf-8">
+  <title>CSS Multi-column Layout Test: Test column-span:all when the body tag is the multi-column container</title>
+  <link rel="author" title="Ting-Yu Lin" href="tlin@mozilla.com">
+  <link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+  <link rel="help" href="https://drafts.csswg.org/css-multicol-1/#column-span">
+  <link rel="match" href="multicol-span-all-007-ref.html">
+  <meta name="assert" content="This test checks a column-span:all element is working if the body tag is the multi-column container.">
+
+  <style>
+  body {
+    column-count: 3;
+    column-rule: 6px solid;
+    width: 400px;
+    outline: 1px solid black;
+  }
+  h3 {
+    column-span: all;
+    outline: 1px solid blue;
+  }
+  </style>
+
+  <body>
+    <div>block1</div>
+    <div>
+      <h3>spanner</h3> <!-- Put spanner in a subtree deliberately -->
+    </div>
+    <div>block2</div>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-multicol/multicol-span-all-dynamic-add-010-ref.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+  <meta charset="utf-8">
+  <title>CSS Multi-column Layout Test Reference: Append a block containing a spanner kid. The spanner kid should correctly span across all columns.</title>
+  <link rel="author" title="Ting-Yu Lin" href="tlin@mozilla.com">
+  <link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+
+  <style>
+  #column {
+    column-count: 3;
+    column-rule: 6px solid;
+    width: 400px;
+    outline: 1px solid black;
+  }
+  h3 {
+    column-span: all;
+    outline: 1px solid blue;
+  }
+  </style>
+
+  <body>
+    <article id="column">
+      <div>block1</div>
+      <div><h3>spanner</h3>block2</div>
+    </article>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-multicol/multicol-span-all-dynamic-add-010.html
@@ -0,0 +1,54 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+  <meta charset="utf-8">
+  <title>CSS Multi-column Layout Test: Append a block containing a spanner kid. The spanner kid should correctly span across all columns.</title>
+  <link rel="author" title="Ting-Yu Lin" href="tlin@mozilla.com">
+  <link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+  <link rel="help" href="https://drafts.csswg.org/css-multicol-1/#column-span">
+  <link rel="match" href="multicol-span-all-dynamic-add-010-ref.html">
+  <meta name="assert" content="This test checks appending a block containing 'column-span' element should be rendered correctly.">
+
+  <script>
+  function runTest() {
+    document.body.offsetHeight;
+
+    // Create a subtree like the following, and append it to columns.
+    // <div>
+    //   <h3>spanner</h3>
+    //   block2
+    // </div>
+    var spanner = document.createElement("h3");
+    var spannerText = document.createTextNode("spanner");
+    spanner.appendChild(spannerText);
+
+    var block2 = document.createElement("div");
+    var block2Text = document.createTextNode("block2");
+    block2.appendChild(spanner);
+    block2.appendChild(block2Text)
+
+    var column = document.getElementById("column");
+    column.appendChild(block2);
+
+    document.documentElement.removeAttribute("class");
+  }
+  </script>
+
+  <style>
+  #column {
+    column-count: 3;
+    column-rule: 6px solid;
+    width: 400px;
+    outline: 1px solid black;
+  }
+  h3 {
+    column-span: all;
+    outline: 1px solid blue;
+  }
+  </style>
+
+  <body onload="runTest();">
+    <article id="column">
+      <div>block1</div>
+    </article>
+  </body>
+</html>