Auto merge of #19610 - emilio:slotted-list, r=xidorn

style: Don't support a list of selectors in ::slotted yet.

Bug: 1425757
Reviewed-by: xidorn
MozReview-Commit-ID: G0I0gM2sWTh

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19610)
<!-- Reviewable:end -->

3 files changed, 62 insertions, 35 deletions

diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs
index 1097c288c14..60699d9d60d 100644
--- a/components/selectors/matching.rs
+++ b/components/selectors/matching.rs
@@ -555,18 +555,16 @@ where
 {
     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
         }
diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs
index 91417a8cf62..3584cb4eb25 100644
--- a/components/selectors/parser.rs
+++ b/components/selectors/parser.rs
@@ -215,31 +215,39 @@ impl<Impl: SelectorImpl> SelectorList<Impl> {
     }
 }
 
-/// Parse a comma separated list of compound selectors.
-pub fn parse_compound_selector_list<'i, 't, P, Impl>(
+/// 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<Box<[Selector<Impl>]>, ParseError<'i, P::Error>>
+) -> Result<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)
-    })?;
-
+    let selector = 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()) {
+    if selector.iter_raw_match_order().any(|s| s.is_combinator()) {
         return Err(location.new_custom_error(
             SelectorParseErrorKind::NonCompoundSelector
         ))
     }
 
-    Ok(selectors.into_boxed_slice())
+    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,
+{
+    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
@@ -761,8 +769,12 @@ pub enum Component<Impl: SelectorImpl> {
     ///
     /// 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),
 }
 
@@ -997,14 +1009,9 @@ impl<Impl: SelectorImpl> ToCss for Component<Impl> {
             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) => {
@@ -1305,7 +1312,7 @@ where
 enum SimpleSelectorParseResult<Impl: SelectorImpl> {
     SimpleSelector(Component<Impl>),
     PseudoElement(Impl::PseudoElement),
-    SlottedPseudo(Box<[Selector<Impl>]>),
+    SlottedPseudo(Selector<Impl>),
 }
 
 #[derive(Debug)]
@@ -1724,13 +1731,13 @@ where
                 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.
@@ -1857,7 +1864,7 @@ where
                     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,
                                 )
@@ -2489,7 +2496,10 @@ pub mod tests {
         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());
+        assert!(parse("slot::slotted(div,foo)::first-line").is_err());
+        // TODO
+        assert!(parse("::slotted(div)::before").is_err());
+        assert!(parse("slot::slotted(div,foo)").is_err());
     }
 
     #[test]
diff --git a/components/style/selector_map.rs b/components/style/selector_map.rs
index 24ce93fa170..ef70949da68 100644
--- a/components/style/selector_map.rs
+++ b/components/style/selector_map.rs
@@ -415,6 +415,25 @@ fn specific_bucket_for<'a>(
                 lower_name: &selector.lower_name,
             }
         }
+        // ::slotted(..) isn't a normal pseudo-element, so we can insert it on
+        // the rule hash normally without much problem. For example, in a
+        // selector like:
+        //
+        //   div::slotted(span)::before
+        //
+        // It looks like:
+        //
+        //  [
+        //    LocalName(div),
+        //    Combinator(SlotAssignment),
+        //    Slotted(span),
+        //    Combinator::PseudoElement,
+        //    PseudoElement(::before),
+        //  ]
+        //
+        // So inserting `span` in the rule hash makes sense since we want to
+        // match the slotted <span>.
+        Component::Slotted(ref selector) => find_bucket(selector.iter()),
         _ => Bucket::Universal
     }
 }