Bug 1506204 - Check the in flow frame to determine when to reframe the multicol container. r=dbaron
authorTing-Yu Lin <tlin@mozilla.com>
Sun, 18 Nov 2018 00:11:55 +0000
changeset 503361 75ca66f490c31648aa80c228e043806bdee327d0
parent 503360 2d8c5d46389336d6c4620e947f757e302d3f224c
child 503362 b2248edb85bb91fb05603e8d0b8626150723051f
push id10290
push userffxbld-merge
push dateMon, 03 Dec 2018 16:23:23 +0000
treeherdermozilla-beta@700bed2445e6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1506204
milestone65.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 1506204 - Check the in flow frame to determine when to reframe the multicol container. r=dbaron When removing an out of flow frame under a multicol subtree, it shouldn't affect the multicol tree structure. We should instead check the in flow frame's relationship with the multicol subtree. Differential Revision: https://phabricator.services.mozilla.com/D11540
layout/base/crashtests/1506204.html
layout/base/crashtests/crashtests.list
layout/base/nsCSSFrameConstructor.cpp
testing/web-platform/meta/css/css-multicol/multicol-span-all-dynamic-remove-007.html.ini
testing/web-platform/tests/css/css-multicol/multicol-span-all-dynamic-remove-007.html
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/1506204.html
@@ -0,0 +1,17 @@
+<head id="a">
+<style>
+* {
+  column-count: 1
+}
+.x {
+  position: fixed;
+  column-span: all;
+}
+</style>
+<script>
+function go() {
+  a.appendChild(b);
+}
+</script>
+<body onload=go()>
+<ol id="b" class="x">A</ol>
--- a/layout/base/crashtests/crashtests.list
+++ b/layout/base/crashtests/crashtests.list
@@ -543,8 +543,9 @@ load 1469354.html
 pref(layout.accessiblecaret.enabled,true) load 1472020.html
 load 1472027.html
 load 1477847.html
 load 1489149.html
 load 1490037.html
 load 1494332.html
 load 1494030.html
 load 1505420.html
+pref(layout.css.column-span.enabled,true) load 1506204.html
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -8734,52 +8734,52 @@ nsCSSFrameConstructor::MaybeRecreateCont
   nsIFrame* inFlowFrame =
     (aFrame->GetStateBits() & NS_FRAME_OUT_OF_FLOW) ?
       aFrame->GetPlaceholderFrame() : aFrame;
   MOZ_ASSERT(inFlowFrame, "How did that happen?");
   MOZ_ASSERT(inFlowFrame == inFlowFrame->FirstContinuation(),
              "placeholder for primary frame has previous continuations?");
   nsIFrame* parent = inFlowFrame->GetParent();
 
-  if (aFrame->HasAnyStateBits(NS_FRAME_HAS_MULTI_COLUMN_ANCESTOR)) {
+  if (inFlowFrame->HasAnyStateBits(NS_FRAME_HAS_MULTI_COLUMN_ANCESTOR)) {
     nsIFrame* grandparent = parent->GetParent();
     MOZ_ASSERT(grandparent);
 
     bool needsReframe =
       // 1. Removing a column-span may lead to an empty
       // ::-moz-column-span-wrapper.
-      aFrame->IsColumnSpan() ||
+      inFlowFrame->IsColumnSpan() ||
       // 2. Removing a frame which has any column-span siblings may also
       // lead to an empty ::-moz-column-span-wrapper subtree. The
       // column-span siblings were the frame's children, but later become
       // the frame's siblings after CreateColumnSpanSiblings().
-      aFrame->GetProperty(nsIFrame::HasColumnSpanSiblings()) ||
+      inFlowFrame->GetProperty(nsIFrame::HasColumnSpanSiblings()) ||
       // 3. Removing the only child of a ::-moz-column-content, whose
       // ColumnSet grandparent has a previous column-span sibling, requires
       // reframing since we might connect the ColumnSet's next column-span
       // sibling (if there's one). Note that this isn't actually needed if
       // the ColumnSet is at the end of ColumnSetWrapper since we create
       // empty ones at the end anyway, but we're not worried about
       // optimizing that case.
       (parent->Style()->GetPseudo() == nsCSSAnonBoxes::columnContent() &&
        // The only child in ::-moz-column-content (might be tall enough to
        // split across columns)
-       !aFrame->GetPrevSibling() && !aFrame->GetNextSibling() &&
+       !inFlowFrame->GetPrevSibling() && !inFlowFrame->GetNextSibling() &&
        // That ::-moz-column-content is the first column.
        !parent->GetPrevInFlow() &&
        // The ColumnSet grandparent has a previous sibling that is a
        // column-span.
        grandparent->GetPrevSibling());
 
     if (needsReframe) {
-      nsIFrame* containingBlock = GetMultiColumnContainingBlockFor(aFrame);
+      nsIFrame* containingBlock = GetMultiColumnContainingBlockFor(inFlowFrame);
 
 #ifdef DEBUG
-      if (IsFramePartOfIBSplit(aFrame)) {
-        nsIFrame* ibContainingBlock = GetIBContainingBlockFor(aFrame);
+      if (IsFramePartOfIBSplit(inFlowFrame)) {
+        nsIFrame* ibContainingBlock = GetIBContainingBlockFor(inFlowFrame);
         MOZ_ASSERT(containingBlock == ibContainingBlock ||
                    nsLayoutUtils::IsProperAncestorFrame(containingBlock,
                                                         ibContainingBlock),
                    "Multi-column containing block should be equal to or be the "
                    "ancestor of the IB containing block!");
       }
 #endif
 
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/css/css-multicol/multicol-span-all-dynamic-remove-007.html.ini
@@ -0,0 +1,2 @@
+[multicol-span-all-dynamic-remove-007.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-dynamic-remove-007.html
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<html class="reftest-wait">
+  <meta charset="utf-8">
+  <title>CSS Multi-column Layout Test: Remove the position:fixed spanner as the first child</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-remove-001-ref.html">
+  <meta name="assert" content="This test checks removing a position:fixed 'column-span:all' element (which doesn't actually span columns) should be rendered correctly.">
+
+  <script>
+  function runTest() {
+    document.body.offsetHeight;
+
+    document.getElementById("spanner").remove();
+
+    document.documentElement.removeAttribute("class");
+  }
+  </script>
+
+  <style>
+  #column {
+    column-count: 3;
+    column-rule: 6px solid;
+    width: 400px;
+    outline: 1px solid black;
+  }
+  h3 {
+    position: fixed;
+    column-span: all;
+    outline: 1px solid blue;
+  }
+  </style>
+
+  <body onload="runTest();">
+    <article id="column">
+      <h3 id="spanner">spanner</h3>
+      <div>block1</div>
+      <div>block2</div>
+    </article>
+  </body>
+</html>