From 390e688058a6b4f29216d71c7b46df92b3f5ec35 Mon Sep 17 00:00:00 2001 From: Simon Sapin Date: Tue, 16 May 2017 16:14:23 +0200 Subject: [PATCH] Fix serialization of namespace and universal selectors Fix #16017 Fix #16020 --- components/selectors/matching.rs | 16 +- components/selectors/parser.rs | 340 +++++++++++++++++++++++++------ components/style/stylist.rs | 15 +- tests/unit/style/stylesheets.rs | 25 +-- 4 files changed, 310 insertions(+), 86 deletions(-) diff --git a/components/selectors/matching.rs b/components/selectors/matching.rs index ada15089061..e328edaa8bb 100644 --- a/components/selectors/matching.rs +++ b/components/selectors/matching.rs @@ -3,7 +3,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ use bloom::BloomFilter; use parser::{CaseSensitivity, Combinator, ComplexSelector, Component, LocalName}; -use parser::{Selector, SelectorInner, SelectorIter}; +use parser::{Selector, SelectorInner, SelectorIter, SelectorImpl}; use std::borrow::Borrow; use tree::Element; @@ -299,8 +299,18 @@ fn matches_simple_selector( let name = if element.is_html_element_in_html_document() { lower_name } else { name }; element.get_local_name() == name.borrow() } - Component::Namespace(ref namespace) => { - element.get_namespace() == namespace.url.borrow() + Component::ExplicitUniversalType | + Component::ExplicitAnyNamespace => { + true + } + Component::Namespace(_, ref url) | + Component::DefaultNamespace(ref url) => { + element.get_namespace() == url.borrow() + } + Component::ExplicitNoNamespace => { + // Rust type’s default, not default namespace + let empty_string = ::NamespaceUrl::default(); + element.get_namespace() == empty_string.borrow() } // TODO: case-sensitivity depends on the document type and quirks mode Component::ID(ref id) => { diff --git a/components/selectors/parser.rs b/components/selectors/parser.rs index 82571d9bd5e..0484906257b 100644 --- a/components/selectors/parser.rs +++ b/components/selectors/parser.rs @@ -431,10 +431,17 @@ impl Combinator { #[derive(Eq, PartialEq, Clone)] pub enum Component { Combinator(Combinator), + + ExplicitAnyNamespace, + ExplicitNoNamespace, + DefaultNamespace(Impl::NamespaceUrl), + Namespace(Impl::NamespacePrefix, Impl::NamespaceUrl), + + ExplicitUniversalType, + LocalName(LocalName), + ID(Impl::Identifier), Class(Impl::ClassName), - LocalName(LocalName), - Namespace(Namespace), // Attribute selectors AttrExists(AttrSelector), // [foo] @@ -489,8 +496,9 @@ impl Component { None } }, - Component::Namespace(ref namespace) => { - Some(namespace.url.precomputed_hash()) + Component::DefaultNamespace(ref url) | + Component::Namespace(_, ref url) => { + Some(url.precomputed_hash()) }, Component::ID(ref id) => { Some(id.precomputed_hash()) @@ -646,7 +654,15 @@ impl ToCss for Component { display_to_css_identifier(s, dest) } LocalName(ref s) => s.to_css(dest), - Namespace(ref ns) => ns.to_css(dest), + ExplicitUniversalType => dest.write_char('*'), + + DefaultNamespace(_) => Ok(()), + ExplicitNoNamespace => dest.write_char('|'), + ExplicitAnyNamespace => dest.write_str("*|"), + Namespace(ref prefix, _) => { + display_to_css_identifier(prefix, dest)?; + dest.write_char('|') + } // Attribute selectors AttrExists(ref a) => { @@ -673,7 +689,7 @@ impl ToCss for Component { // Pseudo-classes Negation(ref arg) => { dest.write_str(":not(")?; - debug_assert!(arg.len() <= 1 || (arg.len() == 2 && matches!(arg[0], Component::Namespace(_)))); + debug_assert!(single_simple_selector(arg)); for component in arg.iter() { component.to_css(dest)?; } @@ -832,10 +848,12 @@ fn complex_selector_specificity(selector: &ComplexSelector) where Impl: SelectorImpl { match *simple_selector { Component::Combinator(..) => unreachable!(), - Component::LocalName(..) => - specificity.element_selectors += 1, - Component::ID(..) => - specificity.id_selectors += 1, + Component::LocalName(..) => { + specificity.element_selectors += 1 + } + Component::ID(..) => { + specificity.id_selectors += 1 + } Component::Class(..) | Component::AttrExists(..) | Component::AttrEqual(..) | @@ -859,10 +877,16 @@ fn complex_selector_specificity(selector: &ComplexSelector) Component::NthLastOfType(..) | Component::FirstOfType | Component::LastOfType | Component::OnlyOfType | - Component::NonTSPseudoClass(..) => - specificity.class_like_selectors += 1, - - Component::Namespace(..) => (), + Component::NonTSPseudoClass(..) => { + specificity.class_like_selectors += 1 + } + Component::ExplicitUniversalType | + Component::ExplicitAnyNamespace | + Component::ExplicitNoNamespace | + Component::DefaultNamespace(..) | + Component::Namespace(..) => { + // Does not affect specificity + } Component::Negation(ref negated) => { for ss in negated.iter() { simple_selector_specificity(&ss, specificity); @@ -990,10 +1014,22 @@ fn parse_type_selector(parser: &P, input: &mut CssParser, sequence: &mu None => Ok(false), Some((namespace, local_name)) => { match namespace { - NamespaceConstraint::Specific(ns) => { - sequence.push(Component::Namespace(ns)) - }, - NamespaceConstraint::Any => (), + QNamePrefix::ImplicitAnyNamespace => {} + QNamePrefix::ImplicitDefaultNamespace(url) => { + sequence.push(Component::DefaultNamespace(url)) + } + QNamePrefix::ExplicitNamespace(prefix, url) => { + sequence.push(Component::Namespace(prefix, url)) + } + QNamePrefix::ExplicitNoNamespace => { + sequence.push(Component::ExplicitNoNamespace) + } + QNamePrefix::ExplicitAnyNamespace => { + sequence.push(Component::ExplicitAnyNamespace) + } + QNamePrefix::ImplicitNoNamespace => { + unreachable!() // Not returned with in_attr_selector = false + } } match local_name { Some(name) => { @@ -1002,7 +1038,9 @@ fn parse_type_selector(parser: &P, input: &mut CssParser, sequence: &mu name: from_cow_str(name), })) } - None => (), + None => { + sequence.push(Component::ExplicitUniversalType) + } } Ok(true) } @@ -1015,22 +1053,29 @@ enum SimpleSelectorParseResult { PseudoElement(Impl::PseudoElementSelector), } +#[derive(Debug)] +enum QNamePrefix { + 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` + ExplicitAnyNamespace, // `*|foo` + ExplicitNamespace(Impl::NamespacePrefix, Impl::NamespaceUrl), // `prefix|foo` +} + /// * `Err(())`: Invalid selector, abort /// * `Ok(None)`: Not a simple selector, could be something else. `input` was not consumed. /// * `Ok(Some((namespace, local_name)))`: `None` for the local name means a `*` universal selector fn parse_qualified_name<'i, 't, P, Impl> (parser: &P, input: &mut CssParser<'i, 't>, in_attr_selector: bool) - -> Result, Option>)>, ()> + -> Result, Option>)>, ()> where P: Parser, Impl: SelectorImpl { let default_namespace = |local_name| { let namespace = match parser.default_namespace() { - Some(url) => NamespaceConstraint::Specific(Namespace { - prefix: None, - url: url - }), - None => NamespaceConstraint::Any, + Some(url) => QNamePrefix::ImplicitDefaultNamespace(url), + None => QNamePrefix::ImplicitAnyNamespace, }; Ok(Some((namespace, local_name))) }; @@ -1056,15 +1101,12 @@ fn parse_qualified_name<'i, 't, P, Impl> let prefix = from_cow_str(value); let result = parser.namespace_for_prefix(&prefix); let url = result.ok_or(())?; - explicit_namespace(input, NamespaceConstraint::Specific(Namespace { - prefix: Some(prefix), - url: url - })) + explicit_namespace(input, QNamePrefix::ExplicitNamespace(prefix, url)) }, _ => { input.reset(position); if in_attr_selector { - Ok(Some((NamespaceConstraint::Specific(Default::default()), Some(value)))) + Ok(Some((QNamePrefix::ImplicitNoNamespace, Some(value)))) } else { default_namespace(Some(value)) } @@ -1074,7 +1116,9 @@ fn parse_qualified_name<'i, 't, P, Impl> Ok(Token::Delim('*')) => { let position = input.position(); match input.next_including_whitespace() { - Ok(Token::Delim('|')) => explicit_namespace(input, NamespaceConstraint::Any), + Ok(Token::Delim('|')) => { + explicit_namespace(input, QNamePrefix::ExplicitAnyNamespace) + } _ => { input.reset(position); if in_attr_selector { @@ -1086,7 +1130,7 @@ fn parse_qualified_name<'i, 't, P, Impl> } }, Ok(Token::Delim('|')) => { - explicit_namespace(input, NamespaceConstraint::Specific(Default::default())) + explicit_namespace(input, QNamePrefix::ExplicitNoNamespace) } _ => { input.reset(position); @@ -1104,7 +1148,28 @@ fn parse_attribute_selector(parser: &P, input: &mut CssParser) None => return Err(()), Some((_, None)) => unreachable!(), Some((namespace, Some(local_name))) => AttrSelector { - namespace: namespace, + namespace: match namespace { + QNamePrefix::ImplicitNoNamespace | + QNamePrefix::ExplicitNoNamespace => { + NamespaceConstraint::Specific(Namespace { + prefix: None, + url: Impl::NamespaceUrl::default(), + }) + } + QNamePrefix::ExplicitNamespace(prefix, url) => { + NamespaceConstraint::Specific(Namespace { + prefix: Some(prefix), + url: url, + }) + } + QNamePrefix::ExplicitAnyNamespace => { + NamespaceConstraint::Any + } + QNamePrefix::ImplicitAnyNamespace | + QNamePrefix::ImplicitDefaultNamespace(_) => { + unreachable!() // Not returned with in_attr_selector = true + } + }, lower_name: from_ascii_lowercase(&local_name), name: from_cow_str(local_name), }, @@ -1187,17 +1252,33 @@ fn parse_negation(parser: &P, let mut v = ParseVec::new(); parse_compound_selector(parser, input, &mut v, /* inside_negation = */ true)?; - let allow = v.len() <= 1 || - (v.len() == 2 && matches!(v[0], Component::Namespace(_)) && - matches!(v[1], Component::LocalName(_))); - - if allow { + if single_simple_selector(&v) { Ok(Component::Negation(v.into_vec().into_boxed_slice())) } else { Err(()) } } +// A single type selector can be represented as two components +fn single_simple_selector(v: &[Component]) -> 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, + } + ) + +} + /// simple_selector_sequence /// : [ type_selector | universal ] [ HASH | class | attrib | pseudo | negation ]* /// | [ HASH | class | attrib | pseudo | negation ]+ @@ -1228,10 +1309,7 @@ fn parse_compound_selector( // // Note that this doesn't apply to :not() and :matches() per spec. if !inside_negation { - sequence.push(Component::Namespace(Namespace { - prefix: None, - url: url - })); + sequence.push(Component::DefaultNamespace(url)) } } } else { @@ -1554,13 +1632,61 @@ pub mod tests { assert_eq!(parse(":lang(4)"), Err(())) ; assert_eq!(parse(":lang(en US)"), Err(())) ; assert_eq!(parse("EeÉ"), Ok(SelectorList(vec!(Selector { - inner: SelectorInner::from_vec(vec!(Component::LocalName(LocalName { + inner: SelectorInner::from_vec(vec!( + Component::LocalName(LocalName { name: DummyAtom::from("EeÉ"), - lower_name: DummyAtom::from("eeÉ") })), - ), + lower_name: DummyAtom::from("eeÉ") + }), + )), pseudo_element: None, specificity: specificity(0, 0, 1), })))); + assert_eq!(parse("|e"), Ok(SelectorList(vec!(Selector { + inner: SelectorInner::from_vec(vec!( + Component::ExplicitNoNamespace, + Component::LocalName(LocalName { + name: DummyAtom::from("e"), + lower_name: DummyAtom::from("e") + }), + )), + pseudo_element: None, + specificity: specificity(0, 0, 1), + })))); + // https://github.com/servo/servo/issues/16020 + assert_eq!(parse("*|e"), Ok(SelectorList(vec!(Selector { + inner: SelectorInner::from_vec(vec!( + Component::ExplicitAnyNamespace, + Component::LocalName(LocalName { + name: DummyAtom::from("e"), + lower_name: DummyAtom::from("e") + }), + )), + pseudo_element: None, + specificity: specificity(0, 0, 1), + })))); + assert_eq!(parse("*"), Ok(SelectorList(vec!(Selector { + inner: SelectorInner::from_vec(vec!( + Component::ExplicitUniversalType, + )), + pseudo_element: None, + specificity: specificity(0, 0, 0), + })))); + assert_eq!(parse("|*"), Ok(SelectorList(vec!(Selector { + inner: SelectorInner::from_vec(vec!( + Component::ExplicitNoNamespace, + Component::ExplicitUniversalType, + )), + pseudo_element: None, + specificity: specificity(0, 0, 0), + })))); + assert_eq!(parse("*|*"), Ok(SelectorList(vec!(Selector { + inner: SelectorInner::from_vec(vec!( + Component::ExplicitAnyNamespace, + Component::ExplicitUniversalType, + )), + pseudo_element: None, + specificity: specificity(0, 0, 0), + })))); assert_eq!(parse(".foo:lang(en-US)"), Ok(SelectorList(vec!(Selector { inner: SelectorInner::from_vec(vec![ Component::Class(DummyAtom::from("foo")), @@ -1616,10 +1742,7 @@ pub mod tests { assert_eq!(parse_ns("svg|circle", &parser), Ok(SelectorList(vec![Selector { inner: SelectorInner::from_vec( vec![ - Component::Namespace(Namespace { - prefix: Some(DummyAtom("svg".into())), - url: SVG.into(), - }), + Component::Namespace(DummyAtom("svg".into()), SVG.into()), Component::LocalName(LocalName { name: DummyAtom::from("circle"), lower_name: DummyAtom::from("circle"), @@ -1628,6 +1751,15 @@ pub mod tests { pseudo_element: None, specificity: specificity(0, 0, 1), }]))); + assert_eq!(parse_ns("svg|*", &parser), Ok(SelectorList(vec![Selector { + inner: SelectorInner::from_vec( + vec![ + Component::Namespace(DummyAtom("svg".into()), SVG.into()), + Component::ExplicitUniversalType, + ]), + pseudo_element: None, + specificity: specificity(0, 0, 0), + }]))); // Default namespace does not apply to attribute selectors // https://github.com/mozilla/servo/pull/1652 // but it does apply to implicit type selectors @@ -1636,10 +1768,7 @@ pub mod tests { assert_eq!(parse_ns("[Foo]", &parser), Ok(SelectorList(vec!(Selector { inner: SelectorInner::from_vec( vec![ - Component::Namespace(Namespace { - prefix: None, - url: MATHML.into(), - }), + Component::DefaultNamespace(MATHML.into()), Component::AttrExists(AttrSelector { name: DummyAtom::from("Foo"), lower_name: DummyAtom::from("foo"), @@ -1656,10 +1785,7 @@ pub mod tests { assert_eq!(parse_ns("e", &parser), Ok(SelectorList(vec!(Selector { inner: SelectorInner::from_vec( vec!( - Component::Namespace(Namespace { - prefix: None, - url: MATHML.into(), - }), + Component::DefaultNamespace(MATHML.into()), Component::LocalName(LocalName { name: DummyAtom::from("e"), lower_name: DummyAtom::from("e") }), @@ -1667,6 +1793,61 @@ pub mod tests { pseudo_element: None, specificity: specificity(0, 0, 1), })))); + assert_eq!(parse_ns("*", &parser), Ok(SelectorList(vec!(Selector { + inner: SelectorInner::from_vec( + vec!( + Component::DefaultNamespace(MATHML.into()), + Component::ExplicitUniversalType, + )), + pseudo_element: None, + specificity: specificity(0, 0, 0), + })))); + assert_eq!(parse_ns("*|*", &parser), Ok(SelectorList(vec!(Selector { + inner: SelectorInner::from_vec( + vec!( + Component::ExplicitAnyNamespace, + Component::ExplicitUniversalType, + )), + pseudo_element: None, + specificity: specificity(0, 0, 0), + })))); + // Default namespace applies to universal and type selectors inside :not and :matches, + // but not otherwise. + assert_eq!(parse_ns(":not(.cl)", &parser), Ok(SelectorList(vec!(Selector { + inner: SelectorInner::from_vec(vec!( + Component::DefaultNamespace(MATHML.into()), + Component::Negation(vec![ + Component::Class(DummyAtom::from("cl")) + ].into_boxed_slice()), + )), + pseudo_element: None, + specificity: specificity(0, 1, 0), + })))); + assert_eq!(parse_ns(":not(*)", &parser), Ok(SelectorList(vec!(Selector { + inner: SelectorInner::from_vec(vec!( + Component::DefaultNamespace(MATHML.into()), + Component::Negation(vec![ + Component::DefaultNamespace(MATHML.into()), + Component::ExplicitUniversalType, + ].into_boxed_slice()), + )), + pseudo_element: None, + specificity: specificity(0, 0, 0), + })))); + assert_eq!(parse_ns(":not(e)", &parser), Ok(SelectorList(vec!(Selector { + inner: SelectorInner::from_vec(vec!( + Component::DefaultNamespace(MATHML.into()), + Component::Negation(vec![ + Component::DefaultNamespace(MATHML.into()), + Component::LocalName(LocalName { + name: DummyAtom::from("e"), + lower_name: DummyAtom::from("e") + }), + ].into_boxed_slice()) + )), + pseudo_element: None, + specificity: specificity(0, 0, 1), + })))); assert_eq!(parse("[attr |= \"foo\"]"), Ok(SelectorList(vec![Selector { inner: SelectorInner::from_vec( vec![ @@ -1727,10 +1908,7 @@ pub mod tests { assert_eq!(parse_ns(":not(svg|circle)", &parser), Ok(SelectorList(vec!(Selector { inner: SelectorInner::from_vec(vec!(Component::Negation( vec![ - Component::Namespace(Namespace { - prefix: Some(DummyAtom("svg".into())), - url: SVG.into(), - }), + Component::Namespace(DummyAtom("svg".into()), SVG.into()), Component::LocalName(LocalName { name: DummyAtom::from("circle"), lower_name: DummyAtom::from("circle") @@ -1740,6 +1918,46 @@ pub mod tests { pseudo_element: None, specificity: specificity(0, 0, 1), })))); + // https://github.com/servo/servo/issues/16017 + assert_eq!(parse_ns(":not(*)", &parser), Ok(SelectorList(vec!(Selector { + inner: SelectorInner::from_vec(vec!(Component::Negation( + vec![ + Component::ExplicitUniversalType, + ].into_boxed_slice() + ))), + pseudo_element: None, + specificity: specificity(0, 0, 0), + })))); + assert_eq!(parse_ns(":not(|*)", &parser), Ok(SelectorList(vec!(Selector { + inner: SelectorInner::from_vec(vec!(Component::Negation( + vec![ + Component::ExplicitNoNamespace, + Component::ExplicitUniversalType, + ].into_boxed_slice() + ))), + pseudo_element: None, + specificity: specificity(0, 0, 0), + })))); + assert_eq!(parse_ns(":not(*|*)", &parser), Ok(SelectorList(vec!(Selector { + inner: SelectorInner::from_vec(vec!(Component::Negation( + vec![ + Component::ExplicitAnyNamespace, + Component::ExplicitUniversalType, + ].into_boxed_slice() + ))), + pseudo_element: None, + specificity: specificity(0, 0, 0), + })))); + assert_eq!(parse_ns(":not(svg|*)", &parser), Ok(SelectorList(vec!(Selector { + inner: SelectorInner::from_vec(vec!(Component::Negation( + vec![ + Component::Namespace(DummyAtom("svg".into()), SVG.into()), + Component::ExplicitUniversalType, + ].into_boxed_slice() + ))), + pseudo_element: None, + specificity: specificity(0, 0, 0), + })))); } struct TestVisitor { diff --git a/components/style/stylist.rs b/components/style/stylist.rs index d8b76bd01b0..8fe290c7e8e 100644 --- a/components/style/stylist.rs +++ b/components/style/stylist.rs @@ -1366,9 +1366,20 @@ impl SelectorMap { let mut rules_list = vec![]; for rule in self.other.iter() { - if rule.selector.inner.complex.iter_raw().next().is_none() { - rules_list.push(rule.to_applicable_declaration_block(cascade_level)); + let mut iter = rule.selector.inner.complex.iter_raw(); + match iter.next() { + None => {} + Some(&Component::ExplicitUniversalType) => match iter.next() { + None => {} + Some(&Component::ExplicitAnyNamespace) => match iter.next() { + None => {} + _ => continue + }, + _ => continue + }, + _ => continue } + rules_list.push(rule.to_applicable_declaration_block(cascade_level)) } sort_by_key(&mut rules_list, diff --git a/tests/unit/style/stylesheets.rs b/tests/unit/style/stylesheets.rs index 1cef0f377be..81d8a6fffd4 100644 --- a/tests/unit/style/stylesheets.rs +++ b/tests/unit/style/stylesheets.rs @@ -92,10 +92,7 @@ fn test_parse_stylesheet() { selectors: SelectorList(vec![ Selector { inner: SelectorInner::from_vec(vec![ - Component::Namespace(Namespace { - prefix: None, - url: NsAtom::from("http://www.w3.org/1999/xhtml") - }), + Component::DefaultNamespace(NsAtom::from("http://www.w3.org/1999/xhtml")), Component::LocalName(LocalName { name: local_name!("input"), lower_name: local_name!("input"), @@ -129,10 +126,7 @@ fn test_parse_stylesheet() { selectors: SelectorList(vec![ Selector { inner: SelectorInner::from_vec(vec![ - Component::Namespace(Namespace { - prefix: None, - url: NsAtom::from("http://www.w3.org/1999/xhtml") - }), + Component::DefaultNamespace(NsAtom::from("http://www.w3.org/1999/xhtml")), Component::LocalName(LocalName { name: local_name!("html"), lower_name: local_name!("html"), @@ -143,10 +137,7 @@ fn test_parse_stylesheet() { }, Selector { inner: SelectorInner::from_vec(vec![ - Component::Namespace(Namespace { - prefix: None, - url: NsAtom::from("http://www.w3.org/1999/xhtml") - }), + Component::DefaultNamespace(NsAtom::from("http://www.w3.org/1999/xhtml")), Component::LocalName(LocalName { name: local_name!("body"), lower_name: local_name!("body"), @@ -169,16 +160,10 @@ fn test_parse_stylesheet() { selectors: SelectorList(vec![ Selector { inner: SelectorInner::from_vec(vec![ - Component::Namespace(Namespace { - prefix: None, - url: NsAtom::from("http://www.w3.org/1999/xhtml") - }), + Component::DefaultNamespace(NsAtom::from("http://www.w3.org/1999/xhtml")), Component::ID(Atom::from("d1")), Component::Combinator(Combinator::Child), - Component::Namespace(Namespace { - prefix: None, - url: NsAtom::from("http://www.w3.org/1999/xhtml") - }), + Component::DefaultNamespace(NsAtom::from("http://www.w3.org/1999/xhtml")), Component::Class(Atom::from("ok")), ]), pseudo_element: None,