style: Implement complex :not().

This fixes the failures in bug 1671573 and just works thanks to the
invalidation improvements I did for :is / :where.

Added a couple tests for invalidation which is the tricky bit. 001 is a
very straight-forward test, 002 is the :is test but with :is() replaced
by double-:not().

This also fixes default namespaces inside :is() / :where(), which are
supposed to get ignored, but aren't. Added tests for that and for the
pre-existing :not() behavior which Chrome doesn't quite get right.

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

components/selectors/Cargo.toml

@@ -27,7 +27,6 @@ phf = "0.8"
 precomputed-hash = "0.1"
 servo_arc = { version = "0.1", path = "../servo_arc" }
 smallvec = "1.0"
-thin-slice = "0.1.0"
 to_shmem = { path = "../to_shmem" }
 to_shmem_derive = { path = "../to_shmem_derive" }
 

components/selectors/builder.rs

@@ -326,7 +326,7 @@ where
             Component::NonTSPseudoClass(..) => {
                 specificity.class_like_selectors += 1;
             },
-            Component::Is(ref list) => {
+            Component::Negation(ref list) | Component::Is(ref list) => {
                 // https://drafts.csswg.org/selectors/#specificity-rules:
                 //
                 //     The specificity of an :is() pseudo-class is replaced by the
@@ -346,11 +346,6 @@ where
             Component::Namespace(..) => {
                 // Does not affect specificity
             },
-            Component::Negation(ref negated) => {
-                for ss in negated.iter() {
-                    simple_selector_specificity(&ss, specificity);
-                }
-            },
         }
     }
 

components/selectors/context.rs

@@ -238,10 +238,10 @@ where
     where
         F: FnOnce(&mut Self) -> R,
     {
-        debug_assert!(!self.in_negation, "Someone messed up parsing?");
+        let old_in_negation = self.in_negation;
         self.in_negation = true;
         let result = self.nest(f);
-        self.in_negation = false;
+        self.in_negation = old_in_negation;
         result
     }
 

components/selectors/lib.rs

@@ -18,7 +18,6 @@ extern crate phf;
 extern crate precomputed_hash;
 extern crate servo_arc;
 extern crate smallvec;
-extern crate thin_slice;
 extern crate to_shmem;
 #[macro_use]
 extern crate to_shmem_derive;

components/selectors/matching.rs

@@ -853,14 +853,13 @@ where
             }
             false
         }),
-        Component::Negation(ref negated) => context.shared.nest_for_negation(|context| {
+        Component::Negation(ref list) => context.shared.nest_for_negation(|context| {
-            let mut local_context = LocalMatchingContext {
+            for selector in &**list {
-                matches_hover_and_active_quirk: MatchesHoverAndActiveQuirk::No,
+                if matches_complex_selector(selector.iter(), element, context, flags_setter) {
-                shared: context,
+                    return false;
-            };
+                }
-            !negated
+            }
-                .iter()
+            true
-                .all(|ss| matches_simple_selector(ss, element, &mut local_context, flags_setter))
         }),
     }
 }

components/selectors/parser.rs

@@ -21,7 +21,6 @@ use std::borrow::{Borrow, Cow};
 use std::fmt::{self, Debug, Display, Write};
 use std::iter::Rev;
 use std::slice;
-use thin_slice::ThinBoxedSlice;
 
 /// A trait that represents a pseudo-element.
 pub trait PseudoElement: Sized + ToCss {
@@ -76,9 +75,10 @@ fn to_ascii_lowercase(s: &str) -> Cow<str> {
 bitflags! {
     /// Flags that indicate at which point of parsing a selector are we.
     struct SelectorParsingState: u8 {
-        /// Whether we're inside a negation. If we're inside a negation, we're
+        /// Whether we should avoid adding default namespaces to selectors that
-        /// not allowed to add another negation or such, for example.
+        /// aren't type or universal selectors.
-        const INSIDE_NEGATION = 1 << 0;
+        const SKIP_DEFAULT_NAMESPACE = 1 << 0;
+
         /// Whether we've parsed a ::slotted() pseudo-element already.
         ///
         /// If so, then we can only parse a subset of pseudo-elements, and
@@ -162,7 +162,6 @@ pub enum SelectorParseErrorKind<'i> {
     NoQualifiedNameInAttributeSelector(Token<'i>),
     EmptySelector,
     DanglingCombinator,
-    NonSimpleSelectorInNegation,
     NonCompoundSelector,
     NonPseudoElementAfterSlotted,
     InvalidPseudoElementAfterSlotted,
@@ -180,7 +179,6 @@ pub enum SelectorParseErrorKind<'i> {
     InvalidQualNameInAttr(Token<'i>),
     ExplicitNamespaceUnexpectedToken(Token<'i>),
     ClassNeedsIdent(Token<'i>),
-    EmptyNegation,
 }
 
 macro_rules! with_all_bounds {
@@ -1036,16 +1034,7 @@ pub enum Component<Impl: SelectorImpl> {
     AttributeOther(Box<AttrSelectorWithOptionalNamespace<Impl>>),
 
     /// Pseudo-classes
-    ///
+    Negation(Box<[Selector<Impl>]>),
-    /// CSS3 Negation only takes a simple simple selector, but we still need to
-    /// treat it as a compound selector because it might be a type selector
-    /// which we represent as a namespace and a localname.
-    ///
-    /// Note: if/when we upgrade this to CSS4, which supports combinators, we
-    /// need to think about how this should interact with
-    /// visit_complex_selector, and what the consumers of those APIs should do
-    /// about the presence of combinators in negation.
-    Negation(ThinBoxedSlice<Component<Impl>>),
     FirstChild,
     LastChild,
     OnlyChild,
@@ -1121,10 +1110,9 @@ impl<Impl: SelectorImpl> Component<Impl> {
     pub fn maybe_allowed_after_pseudo_element(&self) -> bool {
         match *self {
             Component::NonTSPseudoClass(..) => true,
-            Component::Negation(ref components) => components
+            Component::Negation(ref selectors) |
-                .iter()
+            Component::Is(ref selectors) |
-                .all(|c| c.maybe_allowed_after_pseudo_element()),
+            Component::Where(ref selectors) => {
-            Component::Is(ref selectors) | Component::Where(ref selectors) => {
                 selectors.iter().all(|selector| {
                     selector
                         .iter_raw_match_order()
@@ -1148,9 +1136,13 @@ impl<Impl: SelectorImpl> Component<Impl> {
             *self
         );
         match *self {
-            Component::Negation(ref components) => !components
+            Component::Negation(ref selectors) => {
-                .iter()
+                !selectors.iter().all(|selector| {
-                .all(|c| c.matches_for_stateless_pseudo_element()),
+                    selector
+                        .iter_raw_match_order()
+                        .all(|c| c.matches_for_stateless_pseudo_element())
+                })
+            },
             Component::Is(ref selectors) | Component::Where(ref selectors) => {
                 selectors.iter().any(|selector| {
                     selector
@@ -1182,14 +1174,6 @@ impl<Impl: SelectorImpl> Component<Impl> {
                     return false;
                 }
             },
-            Negation(ref negated) => {
-                for component in negated.iter() {
-                    if !component.visit(visitor) {
-                        return false;
-                    }
-                }
-            },
-
             AttributeInNoNamespaceExists {
                 ref local_name,
                 ref local_name_lower,
@@ -1239,7 +1223,7 @@ impl<Impl: SelectorImpl> Component<Impl> {
                 }
             },
 
-            Is(ref list) | Where(ref list) => {
+            Negation(ref list) | Is(ref list) | Where(ref list) => {
                 if !visitor.visit_selector_list(&list) {
                     return false;
                 }
@@ -2152,44 +2136,14 @@ where
     P: Parser<'i, Impl = Impl>,
     Impl: SelectorImpl,
 {
-    let state = state | SelectorParsingState::INSIDE_NEGATION;
+    let list = SelectorList::parse_with_state(
+        parser,
+        input,
+        state | SelectorParsingState::SKIP_DEFAULT_NAMESPACE | SelectorParsingState::DISALLOW_PSEUDOS,
+        ParseErrorRecovery::DiscardList
+    )?;
 
-    // We use a sequence because a type selector may be represented as two Components.
+    Ok(Component::Negation(list.0.into_vec().into_boxed_slice()))
-    let mut sequence = SmallVec::<[Component<Impl>; 2]>::new();
-
-    input.skip_whitespace();
-
-    // Get exactly one simple selector. The parse logic in the caller will verify
-    // that there are no trailing tokens after we're done.
-    let is_type_sel = match parse_type_selector(parser, input, state, &mut sequence) {
-        Ok(result) => result,
-        Err(ParseError {
-            kind: ParseErrorKind::Basic(BasicParseErrorKind::EndOfInput),
-            ..
-        }) => return Err(input.new_custom_error(SelectorParseErrorKind::EmptyNegation)),
-        Err(e) => return Err(e.into()),
-    };
-    if !is_type_sel {
-        match parse_one_simple_selector(parser, input, state)? {
-            Some(SimpleSelectorParseResult::SimpleSelector(s)) => {
-                sequence.push(s);
-            },
-            None => {
-                return Err(input.new_custom_error(SelectorParseErrorKind::EmptyNegation));
-            },
-            Some(SimpleSelectorParseResult::PseudoElement(_)) |
-            Some(SimpleSelectorParseResult::PartPseudo(_)) |
-            Some(SimpleSelectorParseResult::SlottedPseudo(_)) => {
-                let e = SelectorParseErrorKind::NonSimpleSelectorInNegation;
-                return Err(input.new_custom_error(e));
-            },
-        }
-    }
-
-    // Success.
-    Ok(Component::Negation(
-        sequence.into_vec().into_boxed_slice().into(),
-    ))
 }
 
 /// simple_selector_sequence
@@ -2225,7 +2179,8 @@ where
             if let Some(url) = parser.default_namespace() {
                 // If there was no explicit type selector, but there is a
                 // default namespace, there is an implicit "<defaultns>|*" type
-                // selector. Except for :host, where we ignore it.
+                // selector. Except for :host() or :not() / :is() / :where(),
+                // where we ignore it.
                 //
                 // https://drafts.csswg.org/css-scoping/#host-element-in-tree:
                 //
@@ -2240,10 +2195,22 @@ where
                 //     given selector is allowed to match a featureless element,
                 //     it must do so while ignoring the default namespace.
                 //
-                if !matches!(
+                // https://drafts.csswg.org/selectors-4/#matches
-                    result,
+                //
-                    SimpleSelectorParseResult::SimpleSelector(Component::Host(..))
+                //     Default namespace declarations do not affect the compound
-                ) {
+                //     selector representing the subject of any selector within
+                //     a :is() pseudo-class, unless that compound selector
+                //     contains an explicit universal selector or type selector.
+                //
+                //     (Similar quotes for :where() / :not())
+                //
+                let ignore_default_ns =
+                    state.intersects(SelectorParsingState::SKIP_DEFAULT_NAMESPACE) ||
+                    matches!(
+                        result,
+                        SimpleSelectorParseResult::SimpleSelector(Component::Host(..))
+                    );
+                if !ignore_default_ns {
                     builder.push_simple_selector(Component::DefaultNamespace(url));
                 }
             }
@@ -2297,7 +2264,7 @@ where
     let inner = SelectorList::parse_with_state(
         parser,
         input,
-        state | SelectorParsingState::DISALLOW_PSEUDOS,
+        state | SelectorParsingState::SKIP_DEFAULT_NAMESPACE | SelectorParsingState::DISALLOW_PSEUDOS,
         parser.is_and_where_error_recovery(),
     )?;
     Ok(component(inner.0.into_vec().into_boxed_slice()))
@@ -2327,11 +2294,6 @@ where
             return Ok(Component::Host(Some(parse_inner_compound_selector(parser, input, state)?)));
         },
         "not" => {
-            if state.intersects(SelectorParsingState::INSIDE_NEGATION) {
-                return Err(input.new_custom_error(
-                    SelectorParseErrorKind::UnexpectedIdent("not".into())
-                ));
-            }
             return parse_negation(parser, input, state)
         },
         _ => {}
@@ -3080,9 +3042,13 @@ pub mod tests {
                 vec![
                     Component::DefaultNamespace(MATHML.into()),
                     Component::Negation(
-                        vec![Component::Class(DummyAtom::from("cl"))]
+                        vec![
-                            .into_boxed_slice()
+                            Selector::from_vec(
-                            .into(),
+                                vec![Component::Class(DummyAtom::from("cl"))],
+                                specificity(0, 1, 0),
+                                Default::default(),
+                            )
+                        ].into_boxed_slice()
                     ),
                 ],
                 specificity(0, 1, 0),
@@ -3096,11 +3062,16 @@ pub mod tests {
                     Component::DefaultNamespace(MATHML.into()),
                     Component::Negation(
                         vec![
-                            Component::DefaultNamespace(MATHML.into()),
+                            Selector::from_vec(
-                            Component::ExplicitUniversalType,
+                                vec![
+                                    Component::DefaultNamespace(MATHML.into()),
+                                    Component::ExplicitUniversalType,
+                                ],
+                                specificity(0, 0, 0),
+                                Default::default(),
+                            )
                         ]
-                        .into_boxed_slice()
+                        .into_boxed_slice(),
-                        .into(),
                     ),
                 ],
                 specificity(0, 0, 0),
@@ -3114,14 +3085,19 @@ pub mod tests {
                     Component::DefaultNamespace(MATHML.into()),
                     Component::Negation(
                         vec![
-                            Component::DefaultNamespace(MATHML.into()),
+                            Selector::from_vec(
-                            Component::LocalName(LocalName {
+                                vec![
-                                name: DummyAtom::from("e"),
+                                    Component::DefaultNamespace(MATHML.into()),
-                                lower_name: DummyAtom::from("e"),
+                                    Component::LocalName(LocalName {
-                            }),
+                                        name: DummyAtom::from("e"),
+                                        lower_name: DummyAtom::from("e"),
+                                    }),
+                                ],
+                                specificity(0, 0, 1),
+                                Default::default(),
+                            ),
                         ]
                         .into_boxed_slice()
-                        .into(),
                     ),
                 ],
                 specificity(0, 0, 1),
@@ -3216,47 +3192,23 @@ pub mod tests {
             )]))
         );
         parser.default_ns = None;
-        assert!(parse(":not(#provel.old)").is_err());
+        assert!(parse(":not(#provel.old)").is_ok());
-        assert!(parse(":not(#provel > old)").is_err());
+        assert!(parse(":not(#provel > old)").is_ok());
         assert!(parse("table[rules]:not([rules=\"none\"]):not([rules=\"\"])").is_ok());
-        assert_eq!(
-            parse(":not(#provel)"),
-            Ok(SelectorList::from_vec(vec![Selector::from_vec(
-                vec![Component::Negation(
-                    vec![Component::ID(DummyAtom::from("provel"))]
-                        .into_boxed_slice()
-                        .into(),
-                )],
-                specificity(1, 0, 0),
-                Default::default(),
-            )]))
-        );
-        assert_eq!(
-            parse_ns(":not(svg|circle)", &parser),
-            Ok(SelectorList::from_vec(vec![Selector::from_vec(
-                vec![Component::Negation(
-                    vec![
-                        Component::Namespace(DummyAtom("svg".into()), SVG.into()),
-                        Component::LocalName(LocalName {
-                            name: DummyAtom::from("circle"),
-                            lower_name: DummyAtom::from("circle"),
-                        }),
-                    ]
-                    .into_boxed_slice()
-                    .into(),
-                )],
-                specificity(0, 0, 1),
-                Default::default(),
-            )]))
-        );
         // https://github.com/servo/servo/issues/16017
         assert_eq!(
             parse_ns(":not(*)", &parser),
             Ok(SelectorList::from_vec(vec![Selector::from_vec(
                 vec![Component::Negation(
-                    vec![Component::ExplicitUniversalType]
+                    vec![
-                        .into_boxed_slice()
+                        Selector::from_vec(
-                        .into(),
+                            vec![
+                                Component::ExplicitUniversalType
+                            ],
+                            specificity(0, 0, 0),
+                            Default::default(),
+                        )
+                    ].into_boxed_slice()
                 )],
                 specificity(0, 0, 0),
                 Default::default(),
@@ -3267,11 +3219,16 @@ pub mod tests {
             Ok(SelectorList::from_vec(vec![Selector::from_vec(
                 vec![Component::Negation(
                     vec![
-                        Component::ExplicitNoNamespace,
+                        Selector::from_vec(
-                        Component::ExplicitUniversalType,
+                            vec![
+                                Component::ExplicitNoNamespace,
+                                Component::ExplicitUniversalType,
+                            ],
+                            specificity(0, 0, 0),
+                            Default::default(),
+                        )
                     ]
-                    .into_boxed_slice()
+                    .into_boxed_slice(),
-                    .into(),
                 )],
                 specificity(0, 0, 0),
                 Default::default(),
@@ -3281,27 +3238,17 @@ pub mod tests {
         // https://github.com/servo/servo/pull/17537
         assert_eq!(
             parse_ns_expected(":not(*|*)", &parser, Some(":not(*)")),
-            Ok(SelectorList::from_vec(vec![Selector::from_vec(
-                vec![Component::Negation(
-                    vec![Component::ExplicitUniversalType]
-                        .into_boxed_slice()
-                        .into(),
-                )],
-                specificity(0, 0, 0),
-                Default::default(),
-            )]))
-        );
-
-        assert_eq!(
-            parse_ns(":not(svg|*)", &parser),
             Ok(SelectorList::from_vec(vec![Selector::from_vec(
                 vec![Component::Negation(
                     vec![
-                        Component::Namespace(DummyAtom("svg".into()), SVG.into()),
+                        Selector::from_vec(
-                        Component::ExplicitUniversalType,
+                            vec![
-                    ]
+                                Component::ExplicitUniversalType
-                    .into_boxed_slice()
+                            ],
-                    .into(),
+                            specificity(0, 0, 0),
+                            Default::default()
+                        )
+                    ].into_boxed_slice()
                 )],
                 specificity(0, 0, 0),
                 Default::default(),