Bug 1512026 - Handle nested slots correctly in slotted matching and invalidation. r=heycam, a=RyanVM
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 13 Dec 2018 02:17:53 +0000
changeset 509092 da461fa0a394f281b886840bd768f7dc8a1f25d0
parent 509091 f77371a128ae6e098e2bd4c482ec705b4eac2b6c
child 509093 17a1d1be96f474b846b1bdbb82786c3238407b53
push id1905
push userffxbld-merge
push dateMon, 21 Jan 2019 12:33:13 +0000
treeherdermozilla-release@c2fca1944d8c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam, RyanVM
bugs1512026
milestone65.0
Bug 1512026 - Handle nested slots correctly in slotted matching and invalidation. r=heycam, a=RyanVM The patch and test should be pretty much self-descriptive. Differential Revision: https://phabricator.services.mozilla.com/D14063
servo/components/selectors/matching.rs
servo/components/selectors/tree.rs
servo/components/style/invalidation/element/invalidator.rs
testing/web-platform/tests/css/css-scoping/slotted-nested.html
--- a/servo/components/selectors/matching.rs
+++ b/servo/components/selectors/matching.rs
@@ -405,16 +405,17 @@ enum Rightmost {
     No,
 }
 
 #[inline(always)]
 fn next_element_for_combinator<E>(
     element: &E,
     combinator: Combinator,
     selector: &SelectorIter<E::Impl>,
+    context: &MatchingContext<E::Impl>,
 ) -> Option<E>
 where
     E: Element,
 {
     match combinator {
         Combinator::NextSibling | Combinator::LaterSibling => element.prev_sibling_element(),
         Combinator::Child | Combinator::Descendant => {
             match element.parent_element() {
@@ -445,21 +446,30 @@ where
             if !selector.clone().is_featureless_host_selector() {
                 return None;
             }
 
             element.containing_shadow_host()
         },
         Combinator::SlotAssignment => {
             debug_assert!(
+                context.current_host.is_some(),
+                "Should not be trying to match slotted rules in a non-shadow-tree context"
+            );
+            debug_assert!(
                 element
                     .assigned_slot()
                     .map_or(true, |s| s.is_html_slot_element())
             );
-            element.assigned_slot()
+            let scope = context.current_host?;
+            let mut current_slot = element.assigned_slot()?;
+            while current_slot.containing_shadow_host().unwrap().opaque() != scope {
+                current_slot = current_slot.assigned_slot()?;
+            }
+            Some(current_slot)
         },
         Combinator::PseudoElement => element.pseudo_element_originating_element(),
     }
 }
 
 fn matches_complex_selector_internal<E, F>(
     mut selector_iter: SelectorIter<E::Impl>,
     element: &E,
@@ -506,17 +516,22 @@ where
             SelectorMatchingResult::NotMatchedAndRestartFromClosestDescendant
         },
         Combinator::Child |
         Combinator::Descendant |
         Combinator::SlotAssignment |
         Combinator::PseudoElement => SelectorMatchingResult::NotMatchedGlobally,
     };
 
-    let mut next_element = next_element_for_combinator(element, combinator, &selector_iter);
+    let mut next_element = next_element_for_combinator(
+        element,
+        combinator,
+        &selector_iter,
+        &context,
+    );
 
     // Stop matching :visited as soon as we find a link, or a combinator for
     // something that isn't an ancestor.
     let mut visited_handling = if element.is_link() || combinator.is_sibling() {
         VisitedHandlingMode::AllLinksUnvisited
     } else {
         context.visited_handling()
     };
@@ -570,17 +585,22 @@ where
             // matching on the next candidate element.
             _ => {},
         }
 
         if element.is_link() {
             visited_handling = VisitedHandlingMode::AllLinksUnvisited;
         }
 
-        next_element = next_element_for_combinator(&element, combinator, &selector_iter);
+        next_element = next_element_for_combinator(
+            &element,
+            combinator,
+            &selector_iter,
+            &context,
+        );
     }
 }
 
 #[inline]
 fn matches_local_name<E>(element: &E, local_name: &LocalName<E::Impl>) -> bool
 where
     E: Element,
 {
--- a/servo/components/selectors/tree.rs
+++ b/servo/components/selectors/tree.rs
@@ -6,19 +6,18 @@
 //! between layout and style.
 
 use crate::attr::{AttrSelectorOperation, CaseSensitivity, NamespaceConstraint};
 use crate::matching::{ElementSelectorFlags, MatchingContext};
 use crate::parser::SelectorImpl;
 use std::fmt::Debug;
 use std::ptr::NonNull;
 
-/// Opaque representation of an Element, for identity comparisons. We use
-/// NonZeroPtrMut to get the NonZero optimization.
-#[derive(Clone, Debug, Eq, Hash, PartialEq)]
+/// Opaque representation of an Element, for identity comparisons.
+#[derive(Copy, Clone, Debug, Eq, Hash, PartialEq)]
 pub struct OpaqueElement(NonNull<()>);
 
 unsafe impl Send for OpaqueElement {}
 
 impl OpaqueElement {
     /// Creates a new OpaqueElement from an arbitrarily-typed pointer.
     pub fn new<T>(ptr: &T) -> Self {
         unsafe {
--- a/servo/components/style/invalidation/element/invalidator.rs
+++ b/servo/components/style/invalidation/element/invalidator.rs
@@ -466,35 +466,44 @@ where
         any_descendant
     }
 
     fn invalidate_slotted_elements(&mut self, invalidations: &[Invalidation<'b>]) -> bool {
         if invalidations.is_empty() {
             return false;
         }
 
+        let slot = self.element;
+        self.invalidate_slotted_elements_in_slot(slot, invalidations)
+    }
+
+    fn invalidate_slotted_elements_in_slot(
+        &mut self,
+        slot: E,
+        invalidations: &[Invalidation<'b>],
+    ) -> bool {
         let mut any = false;
 
         let mut sibling_invalidations = InvalidationVector::new();
-        let element = self.element;
-        for node in element.slotted_nodes() {
+        for node in slot.slotted_nodes() {
             let element = match node.as_element() {
                 Some(e) => e,
                 None => continue,
             };
 
-            any |= self.invalidate_child(
-                element,
-                invalidations,
-                &mut sibling_invalidations,
-                DescendantInvalidationKind::Slotted,
-            );
-
-            // FIXME(emilio): Need to handle nested slotted nodes if `element`
-            // is itself a <slot>.
+            if element.is_html_slot_element() {
+                any |= self.invalidate_slotted_elements_in_slot(element, invalidations);
+            } else {
+                any |= self.invalidate_child(
+                    element,
+                    invalidations,
+                    &mut sibling_invalidations,
+                    DescendantInvalidationKind::Slotted,
+                );
+            }
 
             debug_assert!(
                 sibling_invalidations.is_empty(),
                 "::slotted() shouldn't have sibling combinators to the right, \
                  this makes no sense! {:?}",
                 sibling_invalidations
             );
         }
new file mode 100644
--- /dev/null
+++ b/testing/web-platform/tests/css/css-scoping/slotted-nested.html
@@ -0,0 +1,49 @@
+<!doctype html>
+<link rel="href" href="https://mozilla.org" title="Mozilla">
+<link rel="author" href="mailto:emilio@crisal.io" title="Emilio Cobos Álvarez">
+<link rel="help" href="https://drafts.csswg.org/css-scoping/#slotted-pseudo">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<div id="host"><p>This text should be green</p></div>
+<script>
+  let shadow = host.attachShadow({ mode: "open" });
+  shadow.innerHTML = `
+    <style>
+      /* This is not expected to match */
+      .container ::slotted(p) {
+        color: red !important;
+      }
+
+      /* This _is_ expected to match */
+      #nested ::slotted(p) {
+        background-color: green;
+      }
+    </style>
+    <div id="nested"><slot></slot></div>
+  `;
+
+  let nested = shadow.querySelector("#nested").attachShadow({ mode: "open" });
+  nested.innerHTML = `
+    <style>
+      .container ::slotted(p) {
+        color: green;
+      }
+    </style>
+    <div class="container">
+      <slot></slot>
+    </div>
+  `;
+
+  let p = document.querySelector("p");
+  test(function() {
+    assert_equals(getComputedStyle(p).color, "rgb(0, 128, 0)");
+    assert_equals(getComputedStyle(p).backgroundColor, "rgb(0, 128, 0)");
+  }, "Slotted matches rules against the slot in the right tree");
+  test(function() {
+    nested.querySelector(".container").classList.remove("container");
+    assert_not_equals(getComputedStyle(p).color, "rgb(0, 128, 0)");
+
+    nested.host.removeAttribute("id");
+    assert_not_equals(getComputedStyle(p).backgroundColor, "rgb(0, 128, 0)");
+  }, "Style invalidation works correctly for nested slots");
+</script>