Skip to content

Commit

Permalink
Auto merge of #17537 - jyc:default-namespace-serialization, r=<try>
Browse files Browse the repository at this point in the history
Some fixes to selector serialization re: namespaces and universal selector

- Fix eliding default namespace when serializing
- Fix shortest serialization property when namespace prefix is `*|` and there is no default namespace
- Omit universal selector when serializing to match `cssom/serialize-namespaced-type-selectors` (again so we get the shortest serialization)

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes is part of a series to fix #17182

<!-- Either: -->

I'd like to land #17501 first, because it allows some tests for this to work.

- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- 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/17537)
<!-- Reviewable:end -->
  • Loading branch information
bors-servo committed Jul 15, 2017
2 parents f9642b3 + a6a37f3 commit 0e5c718
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 196 deletions.
174 changes: 156 additions & 18 deletions components/selectors/parser.rs
Expand Up @@ -763,19 +763,83 @@ impl<Impl: SelectorImpl> ToCss for Selector<Impl> {
// NB: A parse-order iterator is a Rev<>, which doesn't expose as_slice(),
// which we need for |split|. So we split by combinators on a match-order
// sequence and then reverse.
let mut combinators = self.iter_raw_match_order().rev().filter(|x| x.is_combinator());

let mut combinators = self.iter_raw_match_order().rev().filter(|x| x.is_combinator()).peekable();
let compound_selectors = self.iter_raw_match_order().as_slice().split(|x| x.is_combinator()).rev();

let mut combinators_exhausted = false;
for compound in compound_selectors {
debug_assert!(!combinators_exhausted);
for item in compound.iter() {
item.to_css(dest)?;

// https://www.w3.org/TR/cssom-1/#serializing-selectors

let (namespace, first_non_namespace) = match &compound[0] {
&Component::ExplicitAnyNamespace |
&Component::ExplicitNoNamespace |
&Component::DefaultNamespace(_) |
&Component::Namespace(_, _) => (Some(&compound[0]), 1),
_ => (None, 0)
};

// 1. If there is only one simple selector in the compound selectors
// which is a universal selector, append the result of
// serializing the universal selector to s.
if first_non_namespace == compound.len() - 1 {
match (combinators.peek(), &compound[first_non_namespace]) {
// We have to be careful here, because if there is a pseudo
// element "combinator" there isn't really just the one
// simple selector. Technically this compound selector
// contains the pseudo element selector as
// well--Combinator::PseudoElement doesn't exist in the
// spec.
(Some(&&Component::Combinator(Combinator::PseudoElement)), _) => (),
(_, &Component::ExplicitUniversalType) => {
// Iterate over everything so we serialize the namespace
// too.
for simple in compound.iter() {
simple.to_css(dest)?;
}
continue
}
(_, _) => (),
}
}

// 2. Otherwise, for each simple selector in the compound selectors
// that is not a universal selector of which the namespace prefix
// maps to a namespace that is not the default namespace
// serialize the simple selector and append the result to s.
//
// See https://github.com/w3c/csswg-drafts/issues/1606, which is
// proposing to change this to match up with the behavior asserted
// in cssom/serialize-namespaced-type-selectors.html, which the
// following code tries to match.
for simple in compound.iter() {
match (namespace, simple) {
(Some(&Component::DefaultNamespace(_)),
&Component::ExplicitUniversalType) => continue,
(None,
&Component::ExplicitUniversalType) => continue,
_ => ()
}
simple.to_css(dest)?;
}

// 3. If this is not the last part of the chain of the selector
// append a single SPACE (U+0020), followed by the combinator
// ">", "+", "~", ">>", "||", as appropriate, followed by another
// single SPACE (U+0020) if the combinator was not whitespace, to
// s.
match combinators.next() {
Some(c) => c.to_css(dest)?,
None => combinators_exhausted = true,
};

// 4. If this is the last part of the chain of the selector and
// there is a pseudo-element, append "::" followed by the name of
// the pseudo-element, to s.
//
// (we handle this above)
}

Ok(())
Expand Down Expand Up @@ -1017,13 +1081,30 @@ fn parse_type_selector<'i, 't, P, E, Impl, S>(parser: &P, input: &mut CssParser<
sink.push(Component::DefaultNamespace(url))
}
QNamePrefix::ExplicitNamespace(prefix, url) => {
sink.push(Component::Namespace(prefix, url))
sink.push(match parser.default_namespace() {
Some(ref default_url) if url == *default_url => Component::DefaultNamespace(url),
_ => Component::Namespace(prefix, url),
})
}
QNamePrefix::ExplicitNoNamespace => {
sink.push(Component::ExplicitNoNamespace)
}
QNamePrefix::ExplicitAnyNamespace => {
sink.push(Component::ExplicitAnyNamespace)
match parser.default_namespace() {
// Element type selectors that have no namespace
// component (no namespace separator) represent elements
// without regard to the element's namespace (equivalent
// to "*|") unless a default namespace has been declared
// for namespaced selectors (e.g. in CSS, in the style
// sheet). If a default namespace has been declared,
// such selectors will represent only elements in the
// default namespace.
// -- Selectors § 6.1.1
// So we'll have this act the same as the
// QNamePrefix::ImplicitAnyNamespace case.
None => {},
Some(_) => sink.push(Component::ExplicitAnyNamespace),
}
}
QNamePrefix::ImplicitNoNamespace => {
unreachable!() // Not returned with in_attr_selector = false
Expand Down Expand Up @@ -1729,19 +1810,44 @@ pub mod tests {
}
}

fn parse<'i>(input: &'i str) -> Result<SelectorList<DummySelectorImpl>,
ParseError<'i, SelectorParseError<'i, ()>>> {
fn parse<'i>(input: &'i str)
-> Result<SelectorList<DummySelectorImpl>, ParseError<'i, SelectorParseError<'i, ()>>>
{
parse_ns(input, &DummyParser::default())
}

fn parse_expected<'i, 'a>(input: &'i str, expected: Option<&'a str>)
-> Result<SelectorList<DummySelectorImpl>, ParseError<'i, SelectorParseError<'i, ()>>>
{
parse_ns_expected(input, &DummyParser::default(), expected)
}

fn parse_ns<'i>(input: &'i str, parser: &DummyParser)
-> Result<SelectorList<DummySelectorImpl>,
ParseError<'i, SelectorParseError<'i, ()>>> {
-> Result<SelectorList<DummySelectorImpl>, ParseError<'i, SelectorParseError<'i, ()>>>
{
parse_ns_expected(input, parser, None)
}

fn parse_ns_expected<'i, 'a>(
input: &'i str,
parser: &DummyParser,
expected: Option<&'a str>
) -> Result<SelectorList<DummySelectorImpl>, ParseError<'i, SelectorParseError<'i, ()>>>
{
let mut parser_input = ParserInput::new(input);
let result = SelectorList::parse(parser, &mut CssParser::new(&mut parser_input));
if let Ok(ref selectors) = result {
assert_eq!(selectors.0.len(), 1);
assert_eq!(selectors.0[0].to_css_string(), input);
// We can't assume that the serialized parsed selector will equal
// the input; for example, if there is no default namespace, '*|foo'
// should serialize to 'foo'.
assert_eq!(
selectors.0[0].to_css_string(),
match expected {
Some(x) => x,
None => input
}
);
}
result
}
Expand Down Expand Up @@ -1780,16 +1886,34 @@ pub mod tests {
lower_name: DummyAtom::from("e")
})), specificity(0, 0, 1))
))));
// https://github.com/servo/servo/issues/16020
assert_eq!(parse("*|e"), Ok(SelectorList::from_vec(vec!(
// When the default namespace is not set, *| should be elided.
// https://github.com/servo/servo/pull/17537
assert_eq!(parse_expected("*|e", Some("e")), Ok(SelectorList::from_vec(vec!(
Selector::from_vec(vec!(
Component::ExplicitAnyNamespace,
Component::LocalName(LocalName {
name: DummyAtom::from("e"),
lower_name: DummyAtom::from("e")
})
), specificity(0, 0, 1))
))));
// When the default namespace is set, *| should _not_ be elided (as foo
// is no longer equivalent to *|foo--the former is only for foo in the
// default namespace).
// https://github.com/servo/servo/issues/16020
assert_eq!(
parse_ns("*|e", &DummyParser {
default_ns: Some(DummyAtom::from("https://mozilla.org")),
ns_prefixes: Default::default(),
}),
Ok(SelectorList::from_vec(vec!(
Selector::from_vec(vec!(
Component::ExplicitAnyNamespace,
Component::LocalName(LocalName {
name: DummyAtom::from("e"),
lower_name: DummyAtom::from("e")
})
), specificity(0, 0, 1)))))
);
assert_eq!(parse("*"), Ok(SelectorList::from_vec(vec!(
Selector::from_vec(vec!(
Component::ExplicitUniversalType,
Expand All @@ -1801,12 +1925,22 @@ pub mod tests {
Component::ExplicitUniversalType,
), specificity(0, 0, 0))
))));
assert_eq!(parse("*|*"), Ok(SelectorList::from_vec(vec!(
assert_eq!(parse_expected("*|*", Some("*")), Ok(SelectorList::from_vec(vec!(
Selector::from_vec(vec!(
Component::ExplicitAnyNamespace,
Component::ExplicitUniversalType,
), specificity(0, 0, 0))
))));
assert_eq!(
parse_ns("*|*", &DummyParser {
default_ns: Some(DummyAtom::from("https://mozilla.org")),
ns_prefixes: Default::default(),
}),
Ok(SelectorList::from_vec(vec!(
Selector::from_vec(vec!(
Component::ExplicitAnyNamespace,
Component::ExplicitUniversalType,
), specificity(0, 0, 0)))))
);
assert_eq!(parse(".foo:lang(en-US)"), Ok(SelectorList::from_vec(vec!(
Selector::from_vec(vec!(
Component::Class(DummyAtom::from("foo")),
Expand Down Expand Up @@ -2026,10 +2160,11 @@ pub mod tests {
].into_boxed_slice()
)), specificity(0, 0, 0))
))));
assert_eq!(parse_ns(":not(*|*)", &parser), Ok(SelectorList::from_vec(vec!(
// *| should be elided if there is no default namespace.
// 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::ExplicitAnyNamespace,
Component::ExplicitUniversalType,
].into_boxed_slice()
)), specificity(0, 0, 0))
Expand Down Expand Up @@ -2060,7 +2195,10 @@ pub mod tests {

#[test]
fn test_universal() {
let selector = &parse("*|*::before").unwrap().0[0];
let selector = &parse_ns("*|*::before", &DummyParser {
default_ns: Some(DummyAtom::from("https://mozilla.org")),
ns_prefixes: Default::default(),
}).unwrap().0[0];
assert!(selector.is_universal());
}

Expand Down

This file was deleted.

0 comments on commit 0e5c718

Please sign in to comment.