Bug 1512026 - Handle nested slots correctly in slotted matching and invalidation. r=heycam

The patch and test should be pretty much self-descriptive.

Differential Revision: https://phabricator.services.mozilla.com/D14063

--HG--
extra : moz-landing-system : lando

servo/components/selectors/matching.rs

@@ -410,6 +410,7 @@ fn next_element_for_combinator<E>(
     element: &E,
     combinator: Combinator,
     selector: &SelectorIter<E::Impl>,
+    context: &MatchingContext<E::Impl>,
 ) -> Option<E>
 where
     E: Element,
@@ -449,12 +450,21 @@ where
             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(),
     }
@@ -511,7 +521,12 @@ where
         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.
@@ -575,7 +590,12 @@ where
             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,
+        );
     }
 }
 

servo/components/selectors/tree.rs

@@ -11,9 +11,8 @@ 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 {}

servo/components/style/invalidation/element/invalidator.rs

@@ -471,25 +471,34 @@ where
             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(),

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>