Bug 1774588 - Parsing for relative selectors in `:has()`. r=emilio

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

servo/components/malloc_size_of/lib.rs

@@ -746,7 +746,8 @@ where
             Component::Scope |
             Component::ParentSelector |
             Component::Nth(..) |
-            Component::Host(None) => 0,
+            Component::Host(None) |
+            Component::RelativeSelectorAnchor => 0,
         }
     }
 }

servo/components/selectors/builder.rs

@@ -354,7 +354,8 @@ where
             Component::ExplicitAnyNamespace |
             Component::ExplicitNoNamespace |
             Component::DefaultNamespace(..) |
-            Component::Namespace(..) => {
+            Component::Namespace(..) |
+            Component::RelativeSelectorAnchor => {
                 // Does not affect specificity
             },
         }

servo/components/selectors/context.rs

@@ -147,6 +147,9 @@ where
     /// Extra implementation-dependent matching data.
     pub extra_data: Impl::ExtraMatchingData<'a>,
 
+    /// The current element we're anchoring on for evaluating the relative selector.
+    current_relative_selector_anchor: Option<OpaqueElement>,
+
     quirks_mode: QuirksMode,
     needs_selector_flags: NeedsSelectorFlags,
     classes_and_ids_case_sensitivity: CaseSensitivity,
@@ -198,6 +201,7 @@ where
             in_negation: false,
             pseudo_element_matching_fn: None,
             extra_data: Default::default(),
+            current_relative_selector_anchor: None,
             _impl: ::std::marker::PhantomData,
         }
     }
@@ -318,4 +322,25 @@ where
     pub fn shadow_host(&self) -> Option<OpaqueElement> {
         self.current_host
     }
+
+    /// Runs F with a deeper nesting level, with the given element as the anchor,
+    /// for a :has(...) selector, for example.
+    #[inline]
+    pub fn nest_for_relative_selector<F, R>(&mut self, anchor: OpaqueElement, f: F) -> R
+    where
+        F: FnOnce(&mut Self) -> R,
+    {
+        // TODO(dshin): Nesting should be rejected at parse time.
+        let original_relative_selector_anchor = self.current_relative_selector_anchor.take();
+        self.current_relative_selector_anchor = Some(anchor);
+        let result = self.nest(f);
+        self.current_relative_selector_anchor = original_relative_selector_anchor;
+        result
+    }
+
+    /// Returns the current anchor element to evaluate the relative selector against.
+    #[inline]
+    pub fn relative_selector_anchor(&self) -> Option<OpaqueElement> {
+        self.current_relative_selector_anchor
+    }
 }

servo/components/selectors/matching.rs

@@ -832,10 +832,34 @@ where
             .nest_for_negation(|context| !list_matches_complex_selector(list, element, context)),
         Component::Has(ref list) => context
             .shared
-            .nest(|context| has_children_matching(list, element, context)),
+            .nest_for_relative_selector(element.opaque(), |context| {
+                if cfg!(debug_assertions) {
+                    for selector in list.iter() {
+                        let mut selector_iter = selector.iter_raw_parse_order_from(0);
+                        assert!(
+                            matches!(selector_iter.next().unwrap(), Component::RelativeSelectorAnchor),
+                            "Relative selector does not start with RelativeSelectorAnchor"
+                        );
+                        assert!(
+                            selector_iter.next().unwrap().is_combinator(),
+                            "Relative combinator does not exist"
+                        );
+                    }
+                }
+                // TODO(dshin): Proper matching for sibling relative combinators.
+                has_children_matching(list, element, context)
+            }),
         Component::Combinator(_) => unsafe {
             debug_unreachable!("Shouldn't try to selector-match combinators")
         },
+        Component::RelativeSelectorAnchor => {
+            let anchor = context.shared.relative_selector_anchor();
+            debug_assert!(
+                anchor.is_some(),
+                "Relative selector outside of relative selector matching?"
+            );
+            anchor.map_or(false, |a| a == element.opaque())
+        },
     }
 }
 

servo/components/selectors/parser.rs

@@ -354,15 +354,24 @@ pub struct SelectorList<Impl: SelectorImpl>(
     #[shmem(field_bound)] pub SmallVec<[Selector<Impl>; 1]>,
 );
 
-/// How to treat invalid selectors in a selector list.
-enum ParseErrorRecovery {
-    /// Discard the entire selector list, this is the default behavior for
-    /// almost all of CSS.
-    DiscardList,
+/// Whether or not we're using forgiving parsing mode
+enum ForgivingParsing {
+    /// Discard the entire selector list upon encountering any invalid selector.
+    /// This is the default behavior for almost all of CSS.
+    No,
     /// Ignore invalid selectors, potentially creating an empty selector list.
     ///
     /// This is the error recovery mode of :is() and :where()
-    IgnoreInvalidSelector,
+    Yes,
+}
+
+/// Flag indicating if we're parsing relative selectors.
+#[derive(Copy, Clone, PartialEq)]
+enum ParseRelative {
+    /// Expect selectors to start with a combinator, assuming descendant combinator if not present.
+    Yes,
+    /// Treat as parse error if any selector begins with a combinator.
+    No,
 }
 
 impl<Impl: SelectorImpl> SelectorList<Impl> {
@@ -381,7 +390,8 @@ impl<Impl: SelectorImpl> SelectorList<Impl> {
             parser,
             input,
             SelectorParsingState::empty(),
-            ParseErrorRecovery::DiscardList,
+            ForgivingParsing::No,
+            ParseRelative::No,
         )
     }
 
@@ -390,23 +400,24 @@ impl<Impl: SelectorImpl> SelectorList<Impl> {
         parser: &P,
         input: &mut CssParser<'i, 't>,
         state: SelectorParsingState,
-        recovery: ParseErrorRecovery,
+        recovery: ForgivingParsing,
+        parse_relative: ParseRelative,
     ) -> Result<Self, ParseError<'i, P::Error>>
     where
         P: Parser<'i, Impl = Impl>,
     {
         let mut values = SmallVec::new();
         loop {
-            let selector = input.parse_until_before(Delimiter::Comma, |input| {
-                parse_selector(parser, input, state)
+            let selector = input.parse_until_before(Delimiter::Comma, |i| {
+                parse_selector(parser, i, state, parse_relative)
             });
 
             let was_ok = selector.is_ok();
             match selector {
                 Ok(selector) => values.push(selector),
                 Err(err) => match recovery {
-                    ParseErrorRecovery::DiscardList => return Err(err),
-                    ParseErrorRecovery::IgnoreInvalidSelector => {
+                    ForgivingParsing::No => return Err(err),
+                    ForgivingParsing::Yes => {
                         if !parser.allow_forgiving_selectors() {
                             return Err(err);
                         }
@@ -447,6 +458,7 @@ where
         parser,
         input,
         state | SelectorParsingState::DISALLOW_PSEUDOS | SelectorParsingState::DISALLOW_COMBINATORS,
+        ParseRelative::No,
     )
 }
 
@@ -891,7 +903,8 @@ impl<Impl: SelectorImpl> Selector<Impl> {
                 PseudoElement(..) |
                 Combinator(..) |
                 Host(None) |
-                Part(..) => component.clone(),
+                Part(..) |
+                RelativeSelectorAnchor => component.clone(),
                 ParentSelector => {
                     specificity += parent_specificity;
                     Is(parent.to_vec().into_boxed_slice())
@@ -1453,6 +1466,12 @@ pub enum Component<Impl: SelectorImpl> {
     PseudoElement(#[shmem(field_bound)] Impl::PseudoElement),
 
     Combinator(Combinator),
+
+    /// Used only for relative selectors, which starts with a combinator
+    /// (With an implied descendant combinator if not specified).
+    ///
+    /// https://drafts.csswg.org/csswg-drafts/selectors-4/#typedef-relative-selector
+    RelativeSelectorAnchor,
 }
 
 impl<Impl: SelectorImpl> Component<Impl> {
@@ -1705,6 +1724,14 @@ impl<Impl: SelectorImpl> ToCss for Selector<Impl> {
             if compound.is_empty() {
                 continue;
             }
+            if let Component::RelativeSelectorAnchor = compound.first().unwrap() {
+                debug_assert!(
+                    compound.len() == 1,
+                    "RelativeLeft should only be a simple selector"
+                );
+                combinators.next().unwrap().to_css_relative(dest)?;
+                continue;
+            }
 
             // 1. If there is only one simple selector in the compound selectors
             //    which is a universal selector, append the result of
@@ -1794,18 +1821,45 @@ impl<Impl: SelectorImpl> ToCss for Selector<Impl> {
     }
 }
 
+impl Combinator {
+    fn to_css_internal<W>(&self, dest: &mut W, prefix_space: bool) -> fmt::Result
+    where
+        W: fmt::Write,
+    {
+        if matches!(
+            *self,
+            Combinator::PseudoElement | Combinator::Part | Combinator::SlotAssignment
+        ) {
+            return Ok(());
+        }
+        if prefix_space {
+            dest.write_char(' ')?;
+        }
+        match *self {
+            Combinator::Child => dest.write_str("> "),
+            Combinator::Descendant => Ok(()),
+            Combinator::NextSibling => dest.write_str("+ "),
+            Combinator::LaterSibling => dest.write_str("~ "),
+            Combinator::PseudoElement | Combinator::Part | Combinator::SlotAssignment => unsafe {
+                debug_unreachable!("Already handled")
+            },
+        }
+    }
+
+    fn to_css_relative<W>(&self, dest: &mut W) -> fmt::Result
+    where
+        W: fmt::Write,
+    {
+        self.to_css_internal(dest, false)
+    }
+}
+
 impl ToCss for Combinator {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result
     where
         W: fmt::Write,
     {
-        match *self {
-            Combinator::Child => dest.write_str(" > "),
-            Combinator::Descendant => dest.write_str(" "),
-            Combinator::NextSibling => dest.write_str(" + "),
-            Combinator::LaterSibling => dest.write_str(" ~ "),
-            Combinator::PseudoElement | Combinator::Part | Combinator::SlotAssignment => Ok(()),
-        }
+        self.to_css_internal(dest, true)
     }
 }
 
@@ -1935,6 +1989,7 @@ impl<Impl: SelectorImpl> ToCss for Component<Impl> {
                 dest.write_str(")")
             },
             NonTSPseudoClass(ref pseudo) => pseudo.to_css(dest),
+            RelativeSelectorAnchor => Ok(()),
         }
     }
 }
@@ -1994,13 +2049,19 @@ fn parse_selector<'i, 't, P, Impl>(
     parser: &P,
     input: &mut CssParser<'i, 't>,
     mut state: SelectorParsingState,
+    parse_relative: ParseRelative,
 ) -> Result<Selector<Impl>, ParseError<'i, P::Error>>
 where
     P: Parser<'i, Impl = Impl>,
     Impl: SelectorImpl,
 {
     let mut builder = SelectorBuilder::default();
-
+    if parse_relative == ParseRelative::Yes {
+        builder.push_simple_selector(Component::RelativeSelectorAnchor);
+        // Do we see a combinator? If so, push that. Otherwise, push a descendant combinator.
+        builder
+            .push_combinator(parse_combinator::<P, Impl>(input).unwrap_or(Combinator::Descendant));
+    }
     'outer_loop: loop {
         // Parse a sequence of simple selectors.
         let empty = parse_compound_selector(parser, &mut state, input, &mut builder)?;
@@ -2021,37 +2082,11 @@ where
             break;
         }
 
-        // Parse a combinator.
-        let combinator;
-        let mut any_whitespace = false;
-        loop {
-            let before_this_token = input.state();
-            match input.next_including_whitespace() {
-                Err(_e) => break 'outer_loop,
-                Ok(&Token::WhiteSpace(_)) => any_whitespace = true,
-                Ok(&Token::Delim('>')) => {
-                    combinator = Combinator::Child;
-                    break;
-                },
-                Ok(&Token::Delim('+')) => {
-                    combinator = Combinator::NextSibling;
-                    break;
-                },
-                Ok(&Token::Delim('~')) => {
-                    combinator = Combinator::LaterSibling;
-                    break;
-                },
-                Ok(_) => {
-                    input.reset(&before_this_token);
-                    if any_whitespace {
-                        combinator = Combinator::Descendant;
-                        break;
-                    } else {
-                        break 'outer_loop;
-                    }
-                },
-            }
-        }
+        let combinator = if let Ok(c) = parse_combinator::<P, Impl>(input) {
+            c
+        } else {
+            break 'outer_loop;
+        };
 
         if !state.allows_combinators() {
             return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState));
@@ -2059,8 +2094,35 @@ where
 
         builder.push_combinator(combinator);
     }
+    return Ok(Selector(builder.build()));
+}
 
-    Ok(Selector(builder.build()))
+fn parse_combinator<'i, 't, P, Impl>(input: &mut CssParser<'i, 't>) -> Result<Combinator, ()> {
+    let mut any_whitespace = false;
+    loop {
+        let before_this_token = input.state();
+        match input.next_including_whitespace() {
+            Err(_e) => return Err(()),
+            Ok(&Token::WhiteSpace(_)) => any_whitespace = true,
+            Ok(&Token::Delim('>')) => {
+                return Ok(Combinator::Child);
+            },
+            Ok(&Token::Delim('+')) => {
+                return Ok(Combinator::NextSibling);
+            },
+            Ok(&Token::Delim('~')) => {
+                return Ok(Combinator::LaterSibling);
+            },
+            Ok(_) => {
+                input.reset(&before_this_token);
+                if any_whitespace {
+                    return Ok(Combinator::Descendant);
+                } else {
+                    return Err(());
+                }
+            },
+        }
+    }
 }
 
 impl<Impl: SelectorImpl> Selector<Impl> {
@@ -2073,7 +2135,12 @@ impl<Impl: SelectorImpl> Selector<Impl> {
     where
         P: Parser<'i, Impl = Impl>,
     {
-        parse_selector(parser, input, SelectorParsingState::empty())
+        parse_selector(
+            parser,
+            input,
+            SelectorParsingState::empty(),
+            ParseRelative::No,
+        )
     }
 }
 
@@ -2481,7 +2548,8 @@ where
         state |
             SelectorParsingState::SKIP_DEFAULT_NAMESPACE |
             SelectorParsingState::DISALLOW_PSEUDOS,
-        ParseErrorRecovery::DiscardList,
+        ForgivingParsing::No,
+        ParseRelative::No,
     )?;
 
     Ok(Component::Negation(list.0.into_vec().into_boxed_slice()))
@@ -2586,7 +2654,7 @@ where
     Ok(empty)
 }
 
-fn parse_is_where_has<'i, 't, P, Impl>(
+fn parse_is_where<'i, 't, P, Impl>(
     parser: &P,
     input: &mut CssParser<'i, 't>,
     state: SelectorParsingState,
@@ -2608,11 +2676,34 @@ where
         state |
             SelectorParsingState::SKIP_DEFAULT_NAMESPACE |
             SelectorParsingState::DISALLOW_PSEUDOS,
-        ParseErrorRecovery::IgnoreInvalidSelector,
+        ForgivingParsing::Yes,
+        ParseRelative::No,
     )?;
     Ok(component(inner.0.into_vec().into_boxed_slice()))
 }
 
+fn parse_has<'i, 't, P, Impl>(
+    parser: &P,
+    input: &mut CssParser<'i, 't>,
+    state: SelectorParsingState,
+) -> Result<Component<Impl>, ParseError<'i, P::Error>>
+where
+    P: Parser<'i, Impl = Impl>,
+    Impl: SelectorImpl,
+{
+    debug_assert!(parser.parse_has());
+    let inner = SelectorList::parse_with_state(
+        parser,
+        input,
+        state |
+            SelectorParsingState::SKIP_DEFAULT_NAMESPACE |
+            SelectorParsingState::DISALLOW_PSEUDOS,
+        ForgivingParsing::No,
+        ParseRelative::Yes,
+    )?;
+    Ok(Component::Has(inner.0.into_vec().into_boxed_slice()))
+}
+
 fn parse_functional_pseudo_class<'i, 't, P, Impl>(
     parser: &P,
     input: &mut CssParser<'i, 't>,
@@ -2628,9 +2719,9 @@ where
         "nth-of-type" => return parse_nth_pseudo_class(parser, input, state, NthType::OfType),
         "nth-last-child" => return parse_nth_pseudo_class(parser, input, state, NthType::LastChild),
         "nth-last-of-type" => return parse_nth_pseudo_class(parser, input, state, NthType::LastOfType),
-        "is" if parser.parse_is_and_where() => return parse_is_where_has(parser, input, state, Component::Is),
-        "where" if parser.parse_is_and_where() => return parse_is_where_has(parser, input, state, Component::Where),
-        "has" if parser.parse_has() => return parse_is_where_has(parser, input, state, Component::Has),
+        "is" if parser.parse_is_and_where() => return parse_is_where(parser, input, state, Component::Is),
+        "where" if parser.parse_is_and_where() => return parse_is_where(parser, input, state, Component::Where),
+        "has" if parser.parse_has() => return parse_has(parser, input, state),
         "host" => {
             if !state.allows_tree_structural_pseudo_classes() {
                 return Err(input.new_custom_error(SelectorParseErrorKind::InvalidState));
@@ -2644,7 +2735,7 @@ where
     }
 
     if parser.parse_is_and_where() && parser.is_is_alias(&name) {
-        return parse_is_where_has(parser, input, state, Component::Is);
+        return parse_is_where(parser, input, state, Component::Is);
     }
 
     if !state.allows_custom_functional_pseudo_classes() {
@@ -2690,7 +2781,8 @@ where
         state |
             SelectorParsingState::SKIP_DEFAULT_NAMESPACE |
             SelectorParsingState::DISALLOW_PSEUDOS,
-        ParseErrorRecovery::DiscardList,
+        ForgivingParsing::No,
+        ParseRelative::No,
     )?;
     Ok(Component::NthOf(NthOfSelectorData::new(
         &nth_data,

testing/web-platform/meta/css/selectors/has-argument-with-explicit-scope.html.ini

@@ -1,5 +0,0 @@
-[has-argument-with-explicit-scope.html]
-  expected:
-    if (os == "android") and fission: [OK, TIMEOUT]
-  [:has(:scope .c) matches expected elements on scope1]
-    expected: FAIL

testing/web-platform/meta/css/selectors/has-basic.html.ini

@@ -1,17 +1,6 @@
 [has-basic.html]
-  expected:
-    if (os == "android") and fission: [OK, TIMEOUT]
-  [:has(> .target) matches expected elements]
-    expected: FAIL
-
   [.parent:has(~ #h) matches expected elements]
     expected: FAIL
 
   [:has(+ #h) matches expected elements]
     expected: FAIL
-
-  [:has(> .parent) matches expected elements]
-    expected: FAIL
-
-  [:has(> .parent, > .target) matches expected elements]
-    expected: FAIL

testing/web-platform/meta/css/selectors/has-matches-to-uninserted-elements.html.ini

@@ -1,9 +1,4 @@
 [has-matches-to-uninserted-elements.html]
-  expected:
-    if (os == "android") and fission: [OK, TIMEOUT]
-  [:has(> child) matches expectedly]
-    expected: FAIL
-
   [:has(~ direct_sibling) matches expectedly]
     expected: FAIL
 
@@ -12,6 +7,3 @@
 
   [:has(~ indirect_sibling) matches expectedly]
     expected: FAIL
-
-  [:has(> *) matches expectedly]
-    expected: FAIL

testing/web-platform/meta/css/selectors/has-relative-argument.html.ini

@@ -1,24 +1,4 @@
 [has-relative-argument.html]
-  expected:
-    if (os == "android") and fission: [OK, TIMEOUT]
-  [.x:has(.d .e) matches expected elements]
-    expected: FAIL
-
-  [.x:has(> .a) matches expected elements]
-    expected: FAIL
-
-  [.x:has(> .a > .b) matches expected elements]
-    expected: FAIL
-
-  [.x:has(> .a .b) matches expected elements]
-    expected: FAIL
-
-  [.x:has(> .a + .b) matches expected elements]
-    expected: FAIL
-
-  [.x:has(> .a ~ .b) matches expected elements]
-    expected: FAIL
-
   [.x:has(+ .a) matches expected elements]
     expected: FAIL
 
@@ -52,12 +32,6 @@
   [.x:has(~ .a + .b .c) matches expected elements]
     expected: FAIL
 
-  [.x:has(> .d) matches expected elements]
-    expected: FAIL
-
-  [.x:has(> .d) .f matches expected elements]
-    expected: FAIL
-
   [.x:has(~ .d ~ .e) matches expected elements]
     expected: FAIL
 
@@ -70,14 +44,5 @@
   [.x:has(+ .d ~ .e) ~ .f matches expected elements]
     expected: FAIL
 
-  [.y:has(> .g .h) matches expected elements]
-    expected: FAIL
-
-  [.y:has(.g .h) matches expected elements]
-    expected: FAIL
-
-  [.y:has(> .g .h) .i matches expected elements]
-    expected: FAIL
-
   [.d ~ .x:has(~ .e) matches expected elements]
     expected: FAIL

testing/web-platform/meta/css/selectors/invalidation/has-in-adjacent-position.html.ini

@@ -1,6 +1,4 @@
 [has-in-adjacent-position.html]
-  expected:
-    if (os == "android") and fission: [OK, TIMEOUT]
   [add .test to previous_sibling_child]
     expected: FAIL
 
@@ -58,9 +56,6 @@
   [insert tree div>div.test before previous_sibling_descendant]
     expected: FAIL
 
-  [insert tree div>div.test before subject]
-    expected: FAIL
-
   [insert tree div>div.test before next_sibling]
     expected: FAIL
 
@@ -70,9 +65,6 @@
   [insert tree div>div.test before next_sibling_descendant]
     expected: FAIL
 
-  [insert tree div>div.test after previous_sibling]
-    expected: FAIL
-
   [insert tree div>div.test after previous_sibling_child]
     expected: FAIL
 
@@ -208,15 +200,9 @@
   [insert element div>div[test_attr\] before previous_sibling_descendant]
     expected: FAIL
 
-  [add the class 'test' again to the element in the tree inserted before subject]
-    expected: FAIL
-
   [add the class 'test' to the element in the tree inserted again before subject]
     expected: FAIL
 
-  [insert element div>div[test_attr\] before subject]
-    expected: FAIL
-
   [add the class 'test' again to the element in the tree inserted before next_sibling]
     expected: FAIL
 
@@ -244,15 +230,9 @@
   [insert element div>div[test_attr\] before next_sibling_descendant]
     expected: FAIL
 
-  [add the class 'test' again to the element in the tree inserted after previous_sibling]
-    expected: FAIL
-
   [add the class 'test' to the element in the tree inserted again after previous_sibling]
     expected: FAIL
 
-  [insert element div>div[test_attr\] after previous_sibling]
-    expected: FAIL
-
   [add the class 'test' again to the element in the tree inserted after previous_sibling_child]
     expected: FAIL
 

testing/web-platform/meta/css/selectors/invalidation/has-in-sibling-position.html.ini

@@ -1,6 +1,4 @@
 [has-in-sibling-position.html]
-  expected:
-    if (os == "android") and fission: [OK, TIMEOUT]
   [add .test to previous_sibling_child]
     expected: FAIL
 
@@ -58,9 +56,6 @@
   [insert element div.test after next_sibling_descendant]
     expected: FAIL
 
-  [insert tree div>div.test before previous_sibling]
-    expected: FAIL
-
   [insert tree div>div.test before previous_sibling_child]
     expected: FAIL
 
@@ -217,15 +212,9 @@
   [insert element div[test_attr\] after next_sibling_descendant]
     expected: FAIL
 
-  [add the class 'test' again to the element in the tree inserted before previous_sibling]
-    expected: FAIL
-
   [add the class 'test' to the element in the tree inserted again before previous_sibling]
     expected: FAIL
 
-  [insert element div>div[test_attr\] before previous_sibling]
-    expected: FAIL
-
   [add the class 'test' again to the element in the tree inserted before previous_sibling_child]
     expected: FAIL
 

testing/web-platform/meta/css/selectors/parsing/parse-has.html.ini

@@ -1,20 +0,0 @@
-[parse-has.html]
-  expected:
-    if (os == "android") and fission: [OK, TIMEOUT]
-  [".a:has(> .b)" should be a valid selector]
-    expected: FAIL
-
-  [".a:has(~ .b)" should be a valid selector]
-    expected: FAIL
-
-  [".a:has(+ .b)" should be a valid selector]
-    expected: FAIL
-
-  [":has()" should be an invalid selector]
-    expected: FAIL
-
-  [":has(123)" should be an invalid selector]
-    expected: FAIL
-
-  [":has(.a, 123)" should be an invalid selector]
-    expected: FAIL