Bug 1539267 - Bug 1539171 - Follow the list-item definition from the spec a bit more closely. r=mats
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 27 Mar 2019 15:03:20 +0000
changeset 466390 1803e27751d646abe24ff364147c64bfe55920b4
parent 466389 70dfe7fae016bb4d37d618fe3deaf92250e7fe4a
child 466391 99c4c6989e06ed8597cc570df993129821e095b2
push id81575
push userealvarez@mozilla.com
push dateWed, 27 Mar 2019 15:04:06 +0000
treeherderautoland@1803e27751d6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1539267, 1539171
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 1539267 - Bug 1539171 - Follow the list-item definition from the spec a bit more closely. r=mats The HTML restriction doesn't match any browser. This matches Edge, though I filed https://github.com/w3c/csswg-drafts/issues/3766 about the pseudo-element condition. Differential Revision: https://phabricator.services.mozilla.com/D24936
layout/base/nsGenConList.cpp
layout/generic/crashtests/crashtests.list
servo/components/style/style_adjuster.rs
testing/web-platform/tests/css/css-lists/list-item-definition-ref.html
testing/web-platform/tests/css/css-lists/list-item-definition.html
--- a/layout/base/nsGenConList.cpp
+++ b/layout/base/nsGenConList.cpp
@@ -112,17 +112,20 @@ bool nsGenConList::NodeAfter(const nsGen
     }
     if (pseudoType2 == -2) {
       pseudoType2 = -1;
     }
   }
 
   int32_t cmp = nsLayoutUtils::DoCompareTreePosition(content1, content2,
                                                      pseudoType1, -pseudoType2);
-  MOZ_ASSERT(cmp != 0, "same content, different frames");
+  // DoCompareTreePosition doesn't know about XBL anonymous content, and we
+  // probably shouldn't bother teaching it about it.
+  MOZ_ASSERT(cmp != 0 || content1->GetParent()->GetXBLBinding(),
+             "same content, different frames");
   return cmp > 0;
 }
 
 void nsGenConList::Insert(nsGenConNode* aNode) {
   // Check for append.
   if (mList.isEmpty() || NodeAfter(aNode, mList.getLast())) {
     mList.insertBack(aNode);
   } else if (mLastInserted && mLastInserted != mList.getLast() &&
--- a/layout/generic/crashtests/crashtests.list
+++ b/layout/generic/crashtests/crashtests.list
@@ -32,17 +32,17 @@ load 323493-1.html
 load 323495-1.html
 load 324318-1.html
 load 328946-1.html
 load 331284-1.xhtml
 load 331292.html
 load 334105-1.xhtml
 load 334107-1.xhtml
 load 334147-1.xhtml
-load 334148-1.xhtml
+asserts(1-11) load 334148-1.xhtml # XBL interacts poorly with CSS counters.
 load 334602-1.html
 load 337412-1.html
 load 337883-1.html
 load 337883-2.html
 load 339769-1.html
 load 342322-1.html
 load 343206-1.xhtml
 load 344557-1.html
--- a/servo/components/style/style_adjuster.rs
+++ b/servo/components/style/style_adjuster.rs
@@ -723,34 +723,28 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
         }
     }
 
     /// For HTML elements with 'display:list-item' we add a default 'counter-increment:list-item'
     /// unless 'counter-increment' already has a value for 'list-item'.
     ///
     /// https://drafts.csswg.org/css-lists-3/#declaring-a-list-item
     #[cfg(feature = "gecko")]
-    fn adjust_for_list_item<E>(&mut self, element: Option<E>)
-    where
-        E: TElement,
-    {
+    fn adjust_for_list_item(&mut self) {
         use crate::properties::longhands::counter_increment::computed_value::T as ComputedIncrement;
         use crate::values::generics::counters::CounterPair;
         use crate::values::specified::list::MozListReversed;
         use crate::values::CustomIdent;
 
         if self.style.get_box().clone_display() != Display::ListItem {
             return;
         }
         if self.style.pseudo.is_some() {
             return;
         }
-        if !element.map_or(false, |e| e.is_html_element()) {
-            return;
-        }
         // Note that we map <li value=INTEGER> to 'counter-set: list-item INTEGER;
         // counter-increment: list-item 0;' so we'll return here unless the author
         // explicitly specified something else.
         let increments = self.style.get_counters().clone_counter_increment();
         if increments.iter().any(|i| i.name.0 == atom!("list-item")) {
             return;
         }
 
@@ -828,13 +822,13 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
         }
         #[cfg(feature = "servo")]
         {
             self.adjust_for_text_decorations_in_effect();
         }
         #[cfg(feature = "gecko")]
         {
             self.adjust_for_appearance(element);
-            self.adjust_for_list_item(element);
+            self.adjust_for_list_item();
         }
         self.set_bits();
     }
 }
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-lists/list-item-definition-ref.html
@@ -0,0 +1,11 @@
+<!doctype html>
+<title>CSS Test Reference</title>
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="author" href="https://mozilla.org" title="Mozilla">
+<ol>
+  <svg style="display: list-item"></svg>
+  <img style="display: list-item">
+  <img style="display: list-item" alt="Foo">
+  <li value="4">Foo
+  <li>Bar
+</ol>
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-lists/list-item-definition.html
@@ -0,0 +1,15 @@
+<!doctype html>
+<title>The definition of what a list-item is only depends on the display value, and doesn't account for pseudo-elements</title>
+<link rel="help" href="https://drafts.csswg.org/css-lists/#list-item">
+<link rel="help" href="https://bugzilla.mozilla.org/show_bug.cgi?id=1539171">
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="author" href="https://mozilla.org" title="Mozilla">
+<link rel="match" href="list-item-definition-ref.html">
+<!-- TODO: Test pseudo-elements, see https://github.com/w3c/csswg-drafts/issues/3766 -->
+<ol>
+  <svg style="display: list-item"></svg>
+  <img style="display: list-item">
+  <img style="display: list-item" alt="Foo">
+  <div style="display: list-item">Foo</div>
+  <li>Bar
+</ol>