Bug 1491915 - Fix bullet frame creation for columns. r=bzbarsky
authorTing-Yu Lin <tlin@mozilla.com>
Fri, 16 Nov 2018 21:46:23 +0000
changeset 503309 c21bf3a5e752044715f0026f8407eed642c794c6
parent 503308 045f0da5633d362ecad059b35ae5f696c2db2384
child 503310 9a2cf737f3e3d623ad99857070f4646e4be45388
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)
reviewersbzbarsky
bugs1491915
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 1491915 - Fix bullet frame creation for columns. r=bzbarsky The modification to nsLayoutUtils::GetFirstLinePosition() is needed because we need to get the correct first line position from child (i.e. ColumnSet) when there's an outside bullet on ColumnSetWrapperFrame. The difference between the two newly added tests is "overflow: hidden" on the columns. Differential Revision: https://phabricator.services.mozilla.com/D7009
layout/base/nsCSSFrameConstructor.cpp
layout/base/nsLayoutUtils.cpp
layout/reftests/bugs/reftest.list
layout/reftests/w3c-css/submitted/contain/contain-size-multicol-003-ref.html
layout/reftests/w3c-css/submitted/contain/contain-size-multicol-003.html
testing/web-platform/meta/css/css-multicol/multicol-span-all-list-item-001.html.ini
testing/web-platform/meta/css/css-multicol/multicol-span-all-list-item-002.html.ini
testing/web-platform/tests/css/css-multicol/multicol-span-all-list-item-001-ref.html
testing/web-platform/tests/css/css-multicol/multicol-span-all-list-item-001.html
testing/web-platform/tests/css/css-multicol/multicol-span-all-list-item-002-ref.html
testing/web-platform/tests/css/css-multicol/multicol-span-all-list-item-002.html
testing/web-platform/tests/css/vendor-imports/mozilla/mozilla-central-reftests/contain/contain-size-multicol-003-ref.html
testing/web-platform/tests/css/vendor-imports/mozilla/mozilla-central-reftests/contain/contain-size-multicol-003.html
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@@ -11196,24 +11196,36 @@ nsCSSFrameConstructor::ConstructBlock(ns
     blockFrame->SetInitialChildList(kPrincipalList, childItems);
     CreateBulletFrameForListItemIfNeeded(blockFrame);
     return;
   }
 
   if (!MayNeedToCreateColumnSpanSiblings(blockFrame, childItems)) {
     // No need to create column-span siblings.
     blockFrame->SetInitialChildList(kPrincipalList, childItems);
+    CreateBulletFrameForListItemIfNeeded(blockFrame);
     return;
   }
 
   // Extract any initial non-column-span kids, and put them in block frame's
   // child list.
   nsFrameList initialNonColumnSpanKids =
     childItems.Split([](nsIFrame* f) { return f->IsColumnSpan(); });
   blockFrame->SetInitialChildList(kPrincipalList, initialNonColumnSpanKids);
+
+  nsBlockFrame* blockFrameToCreateBullet = blockFrame;
+  if (needsColumn && (*aNewFrame)->StyleList()->mListStylePosition ==
+                       NS_STYLE_LIST_STYLE_POSITION_OUTSIDE) {
+    // 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.
     return;
   }
 
   nsFrameList columnSpanSiblings =
     CreateColumnSpanSiblings(aState, blockFrame, childItems,
                              aPositionedFrameForAbsPosContainer);
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@@ -6439,20 +6439,21 @@ nsLayoutUtils::GetFirstLinePosition(Writ
         // position.
         *aResult = kidPosition +
           aFrame->GetLogicalUsedBorderAndPadding(aWM).BStart(aWM);
         return true;
       }
       return false;
     }
 
-    if (fType == LayoutFrameType::FieldSet) {
+    if (fType == LayoutFrameType::FieldSet ||
+        fType == LayoutFrameType::ColumnSet) {
       LinePosition kidPosition;
       nsIFrame* kid = aFrame->PrincipalChildList().FirstChild();
-      // kid might be a legend frame here, but that's ok.
+      // If aFrame is fieldset, kid might be a legend frame here, but that's ok.
       if (GetFirstLinePosition(aWM, kid, &kidPosition)) {
         *aResult = kidPosition +
           kid->GetLogicalNormalPosition(aWM, aFrame->GetSize()).B(aWM);
         return true;
       }
       return false;
     }
 
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1546,16 +1546,22 @@ fuzzy-if(Android,0-2,0-48) == 563584-11.
 fails-if(Android) random-if(layersGPUAccelerated) fuzzy-if(skiaContent,0-1,0-1200) == 564991-1.html 564991-1-ref.html
 == 565819-1.html 565819-ref.html
 == 565819-2.html 565819-ref.html
 fuzzy-if(Android,0-1,0-1) needs-focus == 568441.html 568441-ref.html
 == 569006-1.html 569006-1-ref.html
 == 571281-1a.html 571281-1-ref.html
 == 571281-1b.html 571281-1-ref.html
 == 571281-1c.html 571281-1-ref.html
+# Bug 1423383: The three tests for bug 571281 are duplicated for testing
+# while layout.css.column-span.enabled is disabled. Remove them once the
+# pref is default on.
+pref(layout.css.column-span.enabled,true) == 571281-1a.html 571281-1-ref.html
+pref(layout.css.column-span.enabled,true) == 571281-1b.html 571281-1-ref.html
+pref(layout.css.column-span.enabled,true) == 571281-1c.html 571281-1-ref.html
 == 571347-1a.html 571347-1-ref.html
 == 571347-1b.html 571347-1-ref.html
 == 571347-2a.html 571347-2-ref.html
 == 571347-2b.html 571347-2-ref.html
 == 571347-2c.html 571347-2-ref.html
 == 571347-2d.html 571347-2-ref.html
 == 571347-3.html 571347-3-ref.html
 == 572598-1.html 572598-ref.html
--- a/layout/reftests/w3c-css/submitted/contain/contain-size-multicol-003-ref.html
+++ b/layout/reftests/w3c-css/submitted/contain/contain-size-multicol-003-ref.html
@@ -23,20 +23,22 @@
     width: -moz-min-content;
   }
   .max {
     width: -moz-max-content;
   }
   </style>
 </head>
 <body>
+  <!-- XXX: This chunk needs to move to a separate test (Bug 1507663)
   <div class="flexBaselineCheck">
   outside before<div class="basic"></div>outside after
   </div>
   <br>
+  -->
 
   <div class="basic min col-width-ref"></div>
   <br>
 
   <div class="basic max col-width-ref"></div>
   <br>
 
   <div class="basic min col-gap-ref col-width-ref"></div>
--- a/layout/reftests/w3c-css/submitted/contain/contain-size-multicol-003.html
+++ b/layout/reftests/w3c-css/submitted/contain/contain-size-multicol-003.html
@@ -33,20 +33,22 @@
   }
   .max {
     width: -moz-max-content;
   }
   </style>
 </head>
 <body>
   <!--CSS Test: A size-contained multicol element should perform baseline alignment as if it had no contents.-->
+  <!-- XXX: This chunk needs to move to a separate test (Bug 1507663)
   <div class="flexBaselineCheck">
   outside before<div class="contain"><div class="innerContents">inner</div></div>outside after
   </div>
   <br>
+  -->
 
   <!--The following tests are used to ensure column-gaps and column-widths continue to contribute to the minimum and maximum width of a size-contained multicol element. Each should render as if it had no contents.-->
 
   <div class="contain min col-width"><div class="innerContents">inner</div></div>
   <br>
 
   <div class="contain max col-width"><div class="innerContents">inner</div></div>
   <br>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/meta/css/css-multicol/multicol-span-all-list-item-001.html.ini
@@ -0,0 +1,2 @@
+[multicol-span-all-list-item-001.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-list-item-002.html.ini
@@ -0,0 +1,2 @@
+[multicol-span-all-list-item-002.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-list-item-001-ref.html
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<html>
+  <meta charset="utf-8">
+  <title>CSS Multi-column Layout Test Reference: columns with list-item and column-span</title>
+  <link rel="author" title="Ting-Yu Lin" href="tlin@mozilla.com">
+  <link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+
+  <style>
+  li {
+    width: 300px;
+    outline: 1px solid black;
+    margin-bottom: 1em;
+  }
+  h3 {
+    outline: 1px solid blue;
+    margin: 0;
+  }
+  </style>
+
+  <body>
+    <ul>
+      <li style="list-style-position: outside;">
+        bullet outside
+        <h3>spanner</h3>
+      </li>
+      <li style="list-style-position: inside;">
+        bullet inside
+        <h3>spanner</h3>
+      </li>
+      <li style="list-style-position: outside;">
+        <h3>spanner (bullet outside)</h3>
+      </li>
+      <li style="list-style-position: inside;">
+        <h3>spanner (bullet inside)</h3>
+      </li>
+      <li style="list-style-position: outside;">
+        <div>
+          <h3>nested spanner (bullet outside)</h3>
+        </div>
+      </li>
+      <li style="list-style-position: inside;">
+        <div>
+          <h3>nested spanner (bullet inside)</h3>
+        </div>
+      </li>
+    </ul>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-multicol/multicol-span-all-list-item-001.html
@@ -0,0 +1,53 @@
+<!DOCTYPE html>
+<html>
+  <meta charset="utf-8">
+  <title>CSS Multi-column Layout Test: columns with list-item and column-span</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-list-item-001-ref.html">
+  <meta name="assert" content="This test checks the columns with list-item are renederd correctly.">
+
+  <style>
+  li {
+    column-count: 1;
+    width: 300px;
+    outline: 1px solid black;
+    margin-bottom: 1em;
+  }
+  h3 {
+    column-span: all;
+    outline: 1px solid blue;
+    margin: 0;
+  }
+  </style>
+
+  <body>
+    <ul>
+      <li style="list-style-position: outside;">
+        bullet outside
+        <h3>spanner</h3>
+      </li>
+      <li style="list-style-position: inside;">
+        bullet inside
+        <h3>spanner</h3>
+      </li>
+      <li style="list-style-position: outside;">
+        <h3>spanner (bullet outside)</h3>
+      </li>
+      <li style="list-style-position: inside;">
+        <h3>spanner (bullet inside)</h3>
+      </li>
+      <li style="list-style-position: outside;">
+        <div>
+          <h3>nested spanner (bullet outside)</h3>
+        </div>
+      </li>
+      <li style="list-style-position: inside;">
+        <div>
+          <h3>nested spanner (bullet inside)</h3>
+        </div>
+      </li>
+    </ul>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-multicol/multicol-span-all-list-item-002-ref.html
@@ -0,0 +1,49 @@
+<!DOCTYPE html>
+<html>
+  <meta charset="utf-8">
+  <title>CSS Multi-column Layout Test Reference: columns with list-item, column-span, and overflow</title>
+  <link rel="author" title="Ting-Yu Lin" href="tlin@mozilla.com">
+  <link rel="author" title="Mozilla" href="http://www.mozilla.org/">
+
+  <style>
+  li {
+    width: 300px;
+    outline: 1px solid black;
+    margin-bottom: 1em;
+    overflow: hidden;
+  }
+  h3 {
+    outline: 1px solid blue;
+    margin: 0;
+  }
+  </style>
+
+  <body>
+    <ul>
+      <li style="list-style-position: outside;">
+        bullet outside
+        <h3>spanner</h3>
+      </li>
+      <li style="list-style-position: inside;">
+        bullet inside
+        <h3>spanner</h3>
+      </li>
+      <li style="list-style-position: outside;">
+        <h3>spanner (bullet outside)</h3>
+      </li>
+      <li style="list-style-position: inside;">
+        <h3>spanner (bullet inside)</h3>
+      </li>
+      <li style="list-style-position: outside;">
+        <div>
+          <h3>nested spanner (bullet outside)</h3>
+        </div>
+      </li>
+      <li style="list-style-position: inside;">
+        <div>
+          <h3>nested spanner (bullet inside)</h3>
+        </div>
+      </li>
+    </ul>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-multicol/multicol-span-all-list-item-002.html
@@ -0,0 +1,54 @@
+<!DOCTYPE html>
+<html>
+  <meta charset="utf-8">
+  <title>CSS Multi-column Layout Test: columns with list-item, column-span, and overflow</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-list-item-002-ref.html">
+  <meta name="assert" content="This test checks the columns with list-item are renederd correctly.">
+
+  <style>
+  li {
+    column-count: 1;
+    width: 300px;
+    outline: 1px solid black;
+    margin-bottom: 1em;
+    overflow: hidden;
+  }
+  h3 {
+    column-span: all;
+    outline: 1px solid blue;
+    margin: 0;
+  }
+  </style>
+
+  <body>
+    <ul>
+      <li style="list-style-position: outside;">
+        bullet outside
+        <h3>spanner</h3>
+      </li>
+      <li style="list-style-position: inside;">
+        bullet inside
+        <h3>spanner</h3>
+      </li>
+      <li style="list-style-position: outside;">
+        <h3>spanner (bullet outside)</h3>
+      </li>
+      <li style="list-style-position: inside;">
+        <h3>spanner (bullet inside)</h3>
+      </li>
+      <li style="list-style-position: outside;">
+        <div>
+          <h3>nested spanner (bullet outside)</h3>
+        </div>
+      </li>
+      <li style="list-style-position: inside;">
+        <div>
+          <h3>nested spanner (bullet inside)</h3>
+        </div>
+      </li>
+    </ul>
+  </body>
+</html>
--- a/testing/web-platform/tests/css/vendor-imports/mozilla/mozilla-central-reftests/contain/contain-size-multicol-003-ref.html
+++ b/testing/web-platform/tests/css/vendor-imports/mozilla/mozilla-central-reftests/contain/contain-size-multicol-003-ref.html
@@ -23,20 +23,22 @@
     width: min-content;
   }
   .max {
     width: max-content;
   }
   </style>
 </head>
 <body>
+  <!-- XXX: This chunk needs to move to a separate test (Bug 1507663)
   <div class="flexBaselineCheck">
   outside before<div class="basic"></div>outside after
   </div>
   <br>
+  -->
 
   <div class="basic min col-width-ref"></div>
   <br>
 
   <div class="basic max col-width-ref"></div>
   <br>
 
   <div class="basic min col-gap-ref col-width-ref"></div>
--- a/testing/web-platform/tests/css/vendor-imports/mozilla/mozilla-central-reftests/contain/contain-size-multicol-003.html
+++ b/testing/web-platform/tests/css/vendor-imports/mozilla/mozilla-central-reftests/contain/contain-size-multicol-003.html
@@ -33,20 +33,22 @@
   }
   .max {
     width: max-content;
   }
   </style>
 </head>
 <body>
   <!--CSS Test: A size-contained multicol element should perform baseline alignment as if it had no contents.-->
+  <!-- XXX: This chunk needs to move to a separate test (Bug 1507663)
   <div class="flexBaselineCheck">
   outside before<div class="contain"><div class="innerContents">inner</div></div>outside after
   </div>
   <br>
+  -->
 
   <!--The following tests are used to ensure column-gaps and column-widths continue to contribute to the minimum and maximum width of a size-contained multicol element. Each should render as if it had no contents.-->
 
   <div class="contain min col-width"><div class="innerContents">inner</div></div>
   <br>
 
   <div class="contain max col-width"><div class="innerContents">inner</div></div>
   <br>