style: Move unboxing to style, and handle display: contents before ot…

…her suppressions.

This also adopts the resolution of [1] while at it, and switches XUL to not
support display: contents until a use case appears.

This makes our behavior consistent both with the spec and also in terms of
handling dynamic changes to stuff that would otherwise get suppressed.

Also makes us consistent with both Blink and WebKit in terms of computed style.
We were the only ones respecting "behaves as display: none" without actually
computing to display: none. Will file a spec issue to get that changed.

It also makes us match Blink and WebKit in terms of respecting display: contents
before other suppressions, see the reftest which I didn't write as a WPT
(because there's no spec supporting neither that or the opposite of what we do),
where a <g> element respects display: contents even though if it had any other
kind of display value we'd suppress the frame for it and all the descendants
since it's an SVG element in a non-SVG subtree.

Also, this removes the page-break bit from the display: contents loop, which I
think is harmless.

As long as the tests under style are based in namespace id / node name /
traversal parent, this should not make style sharing go wrong in any way, since
that's the first style sharing check we do at [2].

The general idea under this change is making all nodes with computed style of
display: contents actually honor it. Otherwise there's no way of making the
setup sound except re-introducing something similar to all the state tracking
removed in bug 1303605.

[1]: w3c/csswg-drafts#2167
[2]: https://searchfox.org/mozilla-central/rev/fca4426325624fecbd493c31389721513fc49fef/servo/components/style/sharing/mod.rs#700

Bug: 1453702
Reviewed-by: mats, xidorn
MozReview-Commit-ID: JoCKnGYEleD

components/style/dom.rs

@@ -434,6 +434,15 @@ pub trait TElement:
     /// Return whether this element is an element in the HTML namespace.
     fn is_html_element(&self) -> bool;
 
+    /// Return whether this element is an element in the MathML namespace.
+    fn is_mathml_element(&self) -> bool;
+
+    /// Return whether this element is an element in the SVG namespace.
+    fn is_svg_element(&self) -> bool;
+
+    /// Return whether this element is an element in the XUL namespace.
+    fn is_xul_element(&self) -> bool { false }
+
     /// Return the list of slotted nodes of this node.
     fn slotted_nodes(&self) -> &[Self::ConcreteNode] {
         &[]

components/style/gecko/wrapper.rs

@@ -673,11 +673,6 @@ impl<'le> GeckoElement<'le> {
         self.as_node().node_info().mInner.mNamespaceID
     }
 
-    #[inline]
-    fn is_xul_element(&self) -> bool {
-        self.namespace_id() == (structs::root::kNameSpaceID_XUL as i32)
-    }
-
     #[inline]
     fn has_id(&self) -> bool {
         self.as_node()
@@ -826,7 +821,7 @@ impl<'le> GeckoElement<'le> {
         match self.parent_element() {
             Some(e) => {
                 e.local_name() == &*local_name!("use") &&
-                    e.namespace() == &*ns!("http://www.w3.org/2000/svg")
+                e.is_svg_element()
             },
             None => false,
         }
@@ -1058,7 +1053,22 @@ impl<'le> TElement for GeckoElement<'le> {
 
     #[inline]
     fn is_html_element(&self) -> bool {
-        self.namespace_id() == (structs::root::kNameSpaceID_XHTML as i32)
+        self.namespace_id() == structs::kNameSpaceID_XHTML as i32
+    }
+
+    #[inline]
+    fn is_mathml_element(&self) -> bool {
+        self.namespace_id() == structs::kNameSpaceID_MathML as i32
+    }
+
+    #[inline]
+    fn is_svg_element(&self) -> bool {
+        self.namespace_id() == structs::kNameSpaceID_SVG as i32
+    }
+
+    #[inline]
+    fn is_xul_element(&self) -> bool {
+        self.namespace_id() == structs::root::kNameSpaceID_XUL as i32
     }
 
     /// Return the list of slotted nodes of this node.

components/style/style_adjuster.rs

@@ -5,6 +5,7 @@
 //! A struct to encapsulate all the style fixups and flags propagations
 //! a computed style needs in order for it to adhere to the CSS spec.
 
+use Atom;
 use app_units::Au;
 use dom::TElement;
 use properties::{self, CascadeFlags, ComputedValues, StyleBuilder};
@@ -23,6 +24,84 @@ pub struct StyleAdjuster<'a, 'b: 'a> {
     style: &'a mut StyleBuilder<'b>,
 }
 
+#[cfg(feature = "gecko")]
+fn is_topmost_svg_svg_element<E>(e: E) -> bool
+where
+    E: TElement,
+{
+    debug_assert!(e.is_svg_element());
+    if e.local_name() != &*atom!("svg") {
+        return false;
+    }
+
+    let parent = match e.traversal_parent() {
+        Some(n) => n,
+        None => return true,
+    };
+
+    if !parent.is_svg_element() {
+        return true;
+    }
+
+    parent.local_name() == &*atom!("foreignObject")
+}
+
+// https://drafts.csswg.org/css-display/#unbox
+#[cfg(feature = "gecko")]
+fn is_effective_display_none_for_display_contents<E>(element: E) -> bool
+where
+    E: TElement,
+{
+    // FIXME(emilio): This should be an actual static.
+    lazy_static! {
+        static ref SPECIAL_HTML_ELEMENTS: [Atom; 16] = [
+            atom!("br"), atom!("wbr"), atom!("meter"), atom!("progress"),
+            atom!("canvas"), atom!("embed"), atom!("object"), atom!("audio"),
+            atom!("iframe"), atom!("img"), atom!("video"), atom!("frame"),
+            atom!("frameset"), atom!("input"), atom!("textarea"),
+            atom!("select"),
+        ];
+    }
+
+    // https://drafts.csswg.org/css-display/#unbox-svg
+    //
+    // There's a note about "Unknown elements", but there's not a good way to
+    // know what that means, or to get that information from here, and no other
+    // UA implements this either.
+    lazy_static! {
+        static ref SPECIAL_SVG_ELEMENTS: [Atom; 6] = [
+            atom!("svg"), atom!("a"), atom!("g"), atom!("use"),
+            atom!("tspan"), atom!("textPath"),
+        ];
+    }
+
+    // https://drafts.csswg.org/css-display/#unbox-html
+    if element.is_html_element() {
+        let local_name = element.local_name();
+        return SPECIAL_HTML_ELEMENTS.iter().any(|name| &**name == local_name);
+    }
+
+    // https://drafts.csswg.org/css-display/#unbox-svg
+    if element.is_svg_element() {
+        if is_topmost_svg_svg_element(element) {
+            return true;
+        }
+        let local_name = element.local_name();
+        return !SPECIAL_SVG_ELEMENTS.iter().any(|name| &**name == local_name);
+    }
+
+    // https://drafts.csswg.org/css-display/#unbox-mathml
+    //
+    // We always treat XUL as display: none. We don't use display:
+    // contents in XUL anyway, so should be fine to be consistent with
+    // MathML unless there's a use case for it.
+    if element.is_mathml_element() || element.is_xul_element() {
+        return true;
+    }
+
+    false
+}
+
 impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
     /// Trivially constructs a new StyleAdjuster.
     #[inline]
@@ -377,20 +456,35 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
         }
     }
 
-    /// Native anonymous content converts display:contents into display:inline.
+    /// Handles the relevant sections in:
+    ///
+    /// https://drafts.csswg.org/css-display/#unbox-html
+    ///
+    /// And forbidding display: contents in pseudo-elements, at least for now.
     #[cfg(feature = "gecko")]
-    fn adjust_for_prohibited_display_contents(&mut self) {
-        // TODO: We should probably convert display:contents into display:none
-        // in some cases too: https://drafts.csswg.org/css-display/#unbox
-        //
+    fn adjust_for_prohibited_display_contents<E>(&mut self, element: Option<E>)
+    where
+        E: TElement,
+    {
+        if self.style.get_box().clone_display() != Display::Contents {
+            return;
+        }
+
         // FIXME(emilio): ::before and ::after should support display: contents,
         // see bug 1418138.
-        if self.style.pseudo.is_none() || self.style.get_box().clone_display() != Display::Contents
-        {
+        if self.style.pseudo.is_some() {
+            self.style.mutate_box().set_display(Display::Inline);
             return;
         }
 
-        self.style.mutate_box().set_display(Display::Inline);
+        let element = match element {
+            Some(e) => e,
+            None => return,
+        };
+
+        if is_effective_display_none_for_display_contents(element) {
+            self.style.mutate_box().set_display(Display::None);
+        }
     }
 
     /// If a <fieldset> has grid/flex display type, we need to inherit
@@ -657,7 +751,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
         self.adjust_for_visited(element);
         #[cfg(feature = "gecko")]
         {
-            self.adjust_for_prohibited_display_contents();
+            self.adjust_for_prohibited_display_contents(element);
             self.adjust_for_fieldset_content(layout_parent_style);
         }
         self.adjust_for_top_layer();