Bug 1425757: Don't support a list of selectors in ::slotted yet. r?heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Sun, 17 Dec 2017 18:45:51 +0100
changeset 712566 3676ad0f8f6a5382e8015c7a19be03a4528d214b
parent 712564 9bbfbd5126023e1cf530a20a38e7a4b227a5ed62
child 712648 5e42206d34a23c6bc16b9c0b3c0d3edb516fb493
push id93368
push userbmo:emilio@crisal.io
push dateSun, 17 Dec 2017 17:46:49 +0000
reviewersheycam
bugs1425757
milestone59.0a1
Bug 1425757: Don't support a list of selectors in ::slotted yet. r?heycam MozReview-Commit-ID: G0I0gM2sWTh
servo/components/selectors/matching.rs
servo/components/selectors/parser.rs
servo/components/style/selector_map.rs
--- a/servo/components/selectors/matching.rs
+++ b/servo/components/selectors/matching.rs
@@ -550,28 +550,26 @@ fn matches_simple_selector<E, F>(
     flags_setter: &mut F,
 ) -> bool
 where
     E: Element,
     F: FnMut(&E, ElementSelectorFlags),
 {
     match *selector {
         Component::Combinator(_) => unreachable!(),
-        Component::Slotted(ref selectors) => {
+        Component::Slotted(ref selector) => {
             context.shared.nesting_level += 1;
             let result =
                 element.assigned_slot().is_some() &&
-                selectors.iter().any(|s| {
-                    matches_complex_selector(
-                        s.iter(),
-                        element,
-                        context.shared,
-                        flags_setter,
-                    )
-                });
+                matches_complex_selector(
+                    selector.iter(),
+                    element,
+                    context.shared,
+                    flags_setter,
+                );
             context.shared.nesting_level -= 1;
             result
         }
         Component::PseudoElement(ref pseudo) => {
             element.match_pseudo_element(pseudo, context.shared)
         }
         Component::LocalName(LocalName { ref name, ref lower_name }) => {
             let is_html = element.is_html_element_in_html_document();
--- a/servo/components/selectors/parser.rs
+++ b/servo/components/selectors/parser.rs
@@ -210,41 +210,49 @@ impl<Impl: SelectorImpl> SelectorList<Im
     }
 
     /// Creates a SelectorList from a Vec of selectors. Used in tests.
     pub fn from_vec(v: Vec<Selector<Impl>>) -> Self {
         SelectorList(SmallVec::from_vec(v))
     }
 }
 
+/// Parses one compound selector suitable for nested stuff like ::-moz-any, etc.
+fn parse_inner_compound_selector<'i, 't, P, Impl>(
+    parser: &P,
+    input: &mut CssParser<'i, 't>,
+) -> Result<Selector<Impl>, ParseError<'i, P::Error>>
+where
+    P: Parser<'i, Impl=Impl>,
+    Impl: SelectorImpl,
+{
+    let location = input.current_source_location();
+    let selector = Selector::parse(parser, input)?;
+    // Ensure they're actually all compound selectors.
+    if selector.iter_raw_match_order().any(|s| s.is_combinator()) {
+        return Err(location.new_custom_error(
+            SelectorParseErrorKind::NonCompoundSelector
+        ))
+    }
+
+    Ok(selector)
+}
+
 /// Parse a comma separated list of compound selectors.
 pub fn parse_compound_selector_list<'i, 't, P, Impl>(
     parser: &P,
     input: &mut CssParser<'i, 't>,
 ) -> Result<Box<[Selector<Impl>]>, ParseError<'i, P::Error>>
 where
     P: Parser<'i, Impl=Impl>,
     Impl: SelectorImpl,
 {
-    let location = input.current_source_location();
-    let selectors = input.parse_comma_separated(|input| {
-        Selector::parse(parser, input)
-    })?;
-
-    // Ensure they're actually all compound selectors.
-    if selectors
-        .iter()
-        .flat_map(|x| x.iter_raw_match_order())
-        .any(|s| s.is_combinator()) {
-        return Err(location.new_custom_error(
-            SelectorParseErrorKind::NonCompoundSelector
-        ))
-    }
-
-    Ok(selectors.into_boxed_slice())
+    input.parse_comma_separated(|input| {
+        parse_inner_compound_selector(parser, input)
+    }).map(|selectors| selectors.into_boxed_slice())
 }
 
 /// Ancestor hashes for the bloom filter. We precompute these and store them
 /// inline with selectors to optimize cache performance during matching.
 /// This matters a lot.
 ///
 /// We use 4 hashes, which is copied from Gecko, who copied it from WebKit.
 /// Note that increasing the number of hashes here will adversely affect the
@@ -756,18 +764,22 @@ pub enum Component<Impl: SelectorImpl> {
     LastOfType,
     OnlyOfType,
     NonTSPseudoClass(Impl::NonTSPseudoClass),
     /// The ::slotted() pseudo-element (which isn't actually a pseudo-element,
     /// and probably should be a pseudo-class):
     ///
     /// https://drafts.csswg.org/css-scoping/#slotted-pseudo
     ///
-    /// The selectors here are compound selectors, that is, no combinators.
-    Slotted(Box<[Selector<Impl>]>),
+    /// The selector here is a compound selector, that is, no combinators.
+    ///
+    /// NOTE(emilio): This should support a list of selectors, but as of this
+    /// writing no other browser does, and that allows them to put ::slotted()
+    /// in the rule hash, so we do that too.
+    Slotted(Selector<Impl>),
     PseudoElement(Impl::PseudoElement),
 }
 
 impl<Impl: SelectorImpl> Component<Impl> {
     /// Compute the ancestor hash to check against the bloom filter.
     fn ancestor_hash(&self, quirks_mode: QuirksMode) -> Option<u32> {
         match *self {
             Component::LocalName(LocalName { ref name, ref lower_name }) => {
@@ -992,24 +1004,19 @@ impl<Impl: SelectorImpl> ToCss for Compo
                 (_, _) => write!(dest, "{}n{:+}", a, b),
             }
         }
 
         match *self {
             Combinator(ref c) => {
                 c.to_css(dest)
             }
-            Slotted(ref selectors) => {
+            Slotted(ref selector) => {
                 dest.write_str("::slotted(")?;
-                let mut iter = selectors.iter();
-                iter.next().expect("At least one selector").to_css(dest)?;
-                for other in iter {
-                    dest.write_str(", ")?;
-                    other.to_css(dest)?;
-                }
+                selector.to_css(dest)?;
                 dest.write_char(')')
             }
             PseudoElement(ref p) => {
                 p.to_css(dest)
             }
             ID(ref s) => {
                 dest.write_char('#')?;
                 display_to_css_identifier(s, dest)
@@ -1300,17 +1307,17 @@ where
         Err(e) => Err(e)
     }
 }
 
 #[derive(Debug)]
 enum SimpleSelectorParseResult<Impl: SelectorImpl> {
     SimpleSelector(Component<Impl>),
     PseudoElement(Impl::PseudoElement),
-    SlottedPseudo(Box<[Selector<Impl>]>),
+    SlottedPseudo(Selector<Impl>),
 }
 
 #[derive(Debug)]
 enum QNamePrefix<Impl: SelectorImpl> {
     ImplicitNoNamespace, // `foo` in attr selectors
     ImplicitAnyNamespace, // `foo` in type selectors, without a default ns
     ImplicitDefaultNamespace(Impl::NamespaceUrl),  // `foo` in type selectors, with a default ns
     ExplicitNoNamespace,  // `|foo`
@@ -1719,23 +1726,23 @@ where
                 for state_selector in state_selectors.drain() {
                     builder.push_simple_selector(state_selector);
                 }
 
                 pseudo = true;
                 empty = false;
                 break
             }
-            SimpleSelectorParseResult::SlottedPseudo(selectors) => {
+            SimpleSelectorParseResult::SlottedPseudo(selector) => {
                 empty = false;
                 slot = true;
                 if !builder.is_empty() {
                     builder.push_combinator(Combinator::SlotAssignment);
                 }
-                builder.push_simple_selector(Component::Slotted(selectors));
+                builder.push_simple_selector(Component::Slotted(selector));
                 // FIXME(emilio): ::slotted() should support ::before and
                 // ::after after it, so we shouldn't break, but we shouldn't
                 // push more type selectors either.
                 break;
             }
         }
     }
     if empty {
@@ -1852,17 +1859,17 @@ where
             };
             let is_pseudo_element = !is_single_colon ||
                 P::pseudo_element_allows_single_colon(&name);
             if is_pseudo_element {
                 let parse_result = if is_functional {
                     if P::parse_slotted(parser) && name.eq_ignore_ascii_case("slotted") {
                         SimpleSelectorParseResult::SlottedPseudo(
                             input.parse_nested_block(|input| {
-                                parse_compound_selector_list(
+                                parse_inner_compound_selector(
                                     parser,
                                     input,
                                 )
                             })?
                         )
                     } else {
                         SimpleSelectorParseResult::PseudoElement(
                             input.parse_nested_block(|input| {
@@ -2484,17 +2491,16 @@ pub mod tests {
         assert!(parse("::slotted()").is_err());
         assert!(parse("::slotted(div)").is_ok());
         assert!(parse("::slotted(div).foo").is_err());
         assert!(parse("::slotted(div + bar)").is_err());
         assert!(parse("::slotted(div) + foo").is_err());
         assert!(parse("div ::slotted(div)").is_ok());
         assert!(parse("div + slot::slotted(div)").is_ok());
         assert!(parse("div + slot::slotted(div.foo)").is_ok());
-        assert!(parse("div + slot::slotted(.foo, bar, .baz)").is_ok());
     }
 
     #[test]
     fn test_pseudo_iter() {
         let selector = &parse("q::before").unwrap().0[0];
         assert!(!selector.is_universal());
         let mut iter = selector.iter();
         assert_eq!(iter.next(), Some(&Component::PseudoElement(PseudoElement::Before)));
--- a/servo/components/style/selector_map.rs
+++ b/servo/components/style/selector_map.rs
@@ -410,16 +410,17 @@ fn specific_bucket_for<'a>(
         Component::ID(ref id) => Bucket::ID(id),
         Component::Class(ref class) => Bucket::Class(class),
         Component::LocalName(ref selector) => {
             Bucket::LocalName {
                 name: &selector.name,
                 lower_name: &selector.lower_name,
             }
         }
+        Component::Slotted(ref selector) => find_bucket(selector.iter()),
         _ => Bucket::Universal
     }
 }
 
 /// Searches a compound selector from left to right, and returns the appropriate
 /// bucket for it.
 #[inline(always)]
 fn find_bucket<'a>(mut iter: SelectorIter<'a, SelectorImpl>) -> Bucket<'a> {