Bug 1541679 - Skip pseudo frames when finding multicol containing block for reframing. r=bzbarsky
authorTing-Yu Lin <tlin@mozilla.com>
Tue, 16 Apr 2019 05:35:51 +0000
changeset 469613 fe4a28a68cfc
parent 469612 fe7c0d7c09e5
child 469615 a17f163841e9
child 469616 17059c813d40
push id35876
push useropoprus@mozilla.com
push dateTue, 16 Apr 2019 09:47:48 +0000
treeherdermozilla-central@fe4a28a68cfc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbzbarsky
bugs1541679
milestone68.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 1541679 - Skip pseudo frames when finding multicol containing block for reframing. r=bzbarsky This patch does something similar to GetIBContainingBlockFor() because pseudo frames are not good reframe target. Differential Revision: https://phabricator.services.mozilla.com/D26858
layout/base/crashtests/1541679.html
layout/base/crashtests/crashtests.list
layout/base/nsCSSFrameConstructor.cpp
new file mode 100644
--- /dev/null
+++ b/layout/base/crashtests/1541679.html
@@ -0,0 +1,20 @@
+<style>
+* {
+  columns: 0px;
+  column-span: all;
+  overflow-x: auto
+}
+</style>
+<script>
+function go() {
+  a.appendChild(c)
+}
+</script>
+<body onload=go()>
+  <menu></menu>
+  <menuitem id="a">
+    <table id="b">
+      <thead>
+        f|_Nz;[fnx?(
+        <tr id="c">
+
--- a/layout/base/crashtests/crashtests.list
+++ b/layout/base/crashtests/crashtests.list
@@ -562,8 +562,10 @@ load 1510080.html
 load 1510485.html
 pref(layout.css.column-span.enabled,true) load 1511535.html
 load 1511563.html
 pref(layout.css.column-span.enabled,true) load 1524382.html
 pref(layout.css.column-span.enabled,true) load 1533885.html
 pref(layout.css.column-span.enabled,true) load 1534146.html
 pref(layout.css.column-span.enabled,true) load 1539017.html
 load 1539303.html
+pref(layout.css.column-span.enabled,true) load 1541679.html
+
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -565,26 +565,36 @@ static nsIFrame* GetIBContainingBlockFor
                "no normal ancestor found for ib-split frame "
                "in GetIBContainingBlockFor");
   NS_ASSERTION(parentFrame != aFrame,
                "parentFrame is actually the child frame - bogus reslt");
 
   return parentFrame;
 }
 
+// Find the multicol containing block suitable for reframing.
+//
+// Note: this function may not return a ColumnSetWrapperFrame. For example, if
+// the multicol containing block has "overflow:scroll" style, HTMLScrollFrame is
+// returned because ColumnSetWrapperFrame is the scrolled frame which has the
+// -moz-scrolled-content pseudo style. We may walk up "too far", but in terms of
+// correctness of reframing, it's OK.
 static nsContainerFrame* GetMultiColumnContainingBlockFor(nsIFrame* aFrame) {
   MOZ_ASSERT(aFrame->HasAnyStateBits(NS_FRAME_HAS_MULTI_COLUMN_ANCESTOR),
              "Should only be called if the frame has a multi-column ancestor!");
 
   nsContainerFrame* current = aFrame->GetParent();
-  while (current && !current->IsColumnSetWrapperFrame()) {
+  while (current &&
+         (current->HasAnyStateBits(NS_FRAME_HAS_MULTI_COLUMN_ANCESTOR) ||
+          current->Style()->IsPseudoOrAnonBox())) {
     current = current->GetParent();
   }
 
-  MOZ_ASSERT(current, "No ColumnSetWrapperFrame in a valid column hierarchy?");
+  MOZ_ASSERT(current,
+             "No multicol containing block in a valid column hierarchy?");
 
   return current;
 }
 
 // This is a bit slow, but sometimes we need it.
 static bool ParentIsWrapperAnonBox(nsIFrame* aParent) {
   nsIFrame* maybeAnonBox = aParent;
   if (maybeAnonBox->Style()->GetPseudoType() == PseudoStyleType::cellContent) {
@@ -5951,19 +5961,25 @@ void nsCSSFrameConstructor::AppendFrames
     }
 
     nsFrameList columnSpanSiblings = CreateColumnSpanSiblings(
         aState, aParentFrame, aFrameList,
         // Column content should never be a absolute/fixed positioned containing
         // block. Pass nullptr as aPositionedFrame.
         nullptr);
 
-    FinishBuildingColumns(aState,
-                          GetMultiColumnContainingBlockFor(aParentFrame),
-                          aParentFrame, columnSpanSiblings);
+    nsContainerFrame* columnSetWrapper = aParentFrame->GetParent();
+    while (!columnSetWrapper->IsColumnSetWrapperFrame()) {
+      columnSetWrapper = columnSetWrapper->GetParent();
+    }
+    MOZ_ASSERT(columnSetWrapper,
+               "No ColumnSetWrapperFrame ancestor for -moz-column-content?");
+
+    FinishBuildingColumns(aState, columnSetWrapper, 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);