Stop using parse_compound_selector for negation parsing.

This makes the code easier to work with, and fixes a bug where we don't currently
reject pseudo-elements within :not().

MozReview-Commit-ID: Cgl9w0PBsN3

components/selectors/parser.rs

@@ -830,7 +830,6 @@ impl<Impl: SelectorImpl> ToCss for Component<Impl> {
             // Pseudo-classes
             Negation(ref arg) => {
                 dest.write_str(":not(")?;
-                debug_assert!(single_simple_selector(arg));
                 for component in arg.iter() {
                     component.to_css(dest)?;
                 }
@@ -1052,8 +1051,7 @@ fn parse_selector<'i, 't, P, E, Impl>(
     'outer_loop: loop {
         // Parse a sequence of simple selectors.
         parsed_pseudo_element =
-            parse_compound_selector(parser, input, &mut sequence,
+            parse_compound_selector(parser, input, &mut sequence)?;
-                                    /* inside_negation = */ false)?;
         if parsed_pseudo_element {
             break;
         }
@@ -1120,8 +1118,8 @@ impl<Impl: SelectorImpl> Selector<Impl> {
 }
 
 /// * `Err(())`: Invalid selector, abort
-/// * `Ok(None)`: Not a type selector, could be something else. `input` was not consumed.
+/// * `Ok(false)`: Not a type selector, could be something else. `input` was not consumed.
-/// * `Ok(Some(vec))`: Length 0 (`*|*`), 1 (`*|E` or `ns|*`) or 2 (`|E` or `ns|E`)
+/// * `Ok(true)`: Length 0 (`*|*`), 1 (`*|E` or `ns|*`) or 2 (`|E` or `ns|E`)
 fn parse_type_selector<'i, 't, P, E, Impl>(parser: &P, input: &mut CssParser<'i, 't>,
                                            sequence: &mut ParseVec<Impl>)
                                            -> Result<bool, ParseError<'i, SelectorParseError<'i, E>>>
@@ -1423,34 +1421,36 @@ fn parse_negation<'i, 't, P, E, Impl>(parser: &P,
                                                 ParseError<'i, SelectorParseError<'i, E>>>
     where P: Parser<'i, Impl=Impl, Error=E>, Impl: SelectorImpl
 {
-    let mut v = ParseVec::new();
+    // We use a sequence because a type selector may be represented as two Components.
-    parse_compound_selector(parser, input, &mut v, /* inside_negation = */ true)?;
+    let mut sequence = ParseVec::new();
 
-    if single_simple_selector(&v) {
+    // Consume any leading whitespace.
-        Ok(Component::Negation(v.into_vec().into_boxed_slice()))
+    loop {
-    } else {
+        let position = input.position();
-        Err(ParseError::Custom(SelectorParseError::NonSimpleSelectorInNegation))
+        if !matches!(input.next_including_whitespace(), Ok(Token::WhiteSpace(_))) {
-    }
+            input.reset(position);
-}
+            break
-
-// A single type selector can be represented as two components
-fn single_simple_selector<Impl: SelectorImpl>(v: &[Component<Impl>]) -> bool {
-    v.len() == 1 || (
-        v.len() == 2 &&
-        match v[1] {
-            Component::LocalName(_) | Component::ExplicitUniversalType => {
-                debug_assert!(matches!(v[0],
-                    Component::ExplicitAnyNamespace |
-                    Component::ExplicitNoNamespace |
-                    Component::DefaultNamespace(_) |
-                    Component::Namespace(..)
-                ));
-                true
-            }
-            _ => false,
         }
-    )
+    }
 
+    // Get exactly one simple selector. The parse logic in the caller will verify
+    // that there are no trailing tokens after we're done.
+    if !parse_type_selector(parser, input, &mut sequence)? {
+        match parse_one_simple_selector(parser, input, /* inside_negation = */ true)? {
+            Some(SimpleSelectorParseResult::SimpleSelector(s)) => {
+                sequence.push(s);
+            },
+            None => {
+                return Err(ParseError::Custom(SelectorParseError::EmptySelector));
+            },
+            Some(SimpleSelectorParseResult::PseudoElement(_)) => {
+                return Err(ParseError::Custom(SelectorParseError::NonSimpleSelectorInNegation));
+            }
+        }
+    }
+
+    // Success.
+    Ok(Component::Negation(sequence.into_vec().into_boxed_slice()))
 }
 
 /// simple_selector_sequence
@@ -1463,8 +1463,7 @@ fn single_simple_selector<Impl: SelectorImpl>(v: &[Component<Impl>]) -> bool {
 fn parse_compound_selector<'i, 't, P, E, Impl>(
     parser: &P,
     input: &mut CssParser<'i, 't>,
-    mut sequence: &mut ParseVec<Impl>,
+    mut sequence: &mut ParseVec<Impl>)
-    inside_negation: bool)
     -> Result<bool, ParseError<'i, SelectorParseError<'i, E>>>
     where P: Parser<'i, Impl=Impl, Error=E>, Impl: SelectorImpl
 {
@@ -1482,11 +1481,7 @@ fn parse_compound_selector<'i, 't, P, E, Impl>(
             // If there was no explicit type selector, but there is a
             // default namespace, there is an implicit "<defaultns>|*" type
             // selector.
-            //
+            sequence.push(Component::DefaultNamespace(url))
-            // Note that this doesn't apply to :not() and :matches() per spec.
-            if !inside_negation {
-                sequence.push(Component::DefaultNamespace(url))
-            }
         }
     } else {
         empty = false;
@@ -1494,7 +1489,7 @@ fn parse_compound_selector<'i, 't, P, E, Impl>(
 
     let mut pseudo = false;
     loop {
-        match parse_one_simple_selector(parser, input, inside_negation)? {
+        match parse_one_simple_selector(parser, input, /* inside_negation = */ false)? {
             None => break,
             Some(SimpleSelectorParseResult::SimpleSelector(s)) => {
                 sequence.push(s);