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 --HG-- extra : moz-landing-system : lando
2024-10-17 23:35:34 +00:00 · 2018-11-16 21:46:23 +00:00 · 2018-11-16 21:46:23 +00:00 · 1209bed8a8
commit 1209bed8a8
parent 5e324dfa7b
13 changed files with 237 additions and 2 deletions
--- a/layout/base/nsCSSFrameConstructor.cpp
+++ b/layout/base/nsCSSFrameConstructor.cpp
@ -11201,6 +11201,7 @@ nsCSSFrameConstructor::ConstructBlock(nsFrameConstructorState& aState,
  if (!MayNeedToCreateColumnSpanSiblings(blockFrame, childItems)) {
    // No need to create column-span siblings.
    blockFrame->SetInitialChildList(kPrincipalList, childItems);
+    CreateBulletFrameForListItemIfNeeded(blockFrame);
    return;
  }

@ -11209,6 +11210,17 @@ nsCSSFrameConstructor::ConstructBlock(nsFrameConstructorState& aState,
  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;
--- a/layout/base/nsLayoutUtils.cpp
+++ b/layout/base/nsLayoutUtils.cpp
@ -6444,10 +6444,11 @@ nsLayoutUtils::GetFirstLinePosition(WritingMode aWM,
      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);
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@ -1551,6 +1551,12 @@ fuzzy-if(Android,0-1,0-1) needs-focus == 568441.html 568441-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
--- 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
@ -28,10 +28,12 @@
  </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>
--- a/layout/reftests/w3c-css/submitted/contain/contain-size-multicol-003.html
+++ b/layout/reftests/w3c-css/submitted/contain/contain-size-multicol-003.html
@ -38,10 +38,12 @@
 </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.-->

--- a/testing/web-platform/meta/css/css-multicol/multicol-span-all-list-item-001.html.ini
+++ 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]
--- a/testing/web-platform/meta/css/css-multicol/multicol-span-all-list-item-002.html.ini
+++ 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]
--- a/testing/web-platform/tests/css/css-multicol/multicol-span-all-list-item-001-ref.html
+++ 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>
--- a/testing/web-platform/tests/css/css-multicol/multicol-span-all-list-item-001.html
+++ 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>
--- a/testing/web-platform/tests/css/css-multicol/multicol-span-all-list-item-002-ref.html
+++ 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>
--- a/testing/web-platform/tests/css/css-multicol/multicol-span-all-list-item-002.html
+++ 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
@ -28,10 +28,12 @@
  </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>
--- 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
@ -38,10 +38,12 @@
 </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.-->