The attached test case passes in Gecko, Chrome, and WebKit, logging the following to the devtools console: INPUT: "* > *{ }" OUTPUT: "* > * { }" ROUNDTRIP: "* > * { }" PASS But fails in Stylo: INPUT: "* > *{ }" OUTPUT: "** { }" ROUNDTRIP: "" FAIL This test case is a simplified version of the output of Jesse Ruderman's CSS fuzzer: http://www.squarefree.com/2009/03/16/css-grammar-fuzzer/

I'll take a look.

Simon, it appears that Servo parser can't handle legal compound selectors like "* > *" which would match any elements that are children of something. I looked through the parser and confirmed that all the atoms are read correctly. However, the builder structure holds the tokens without specifying an order between simple_selectors and combinators: http://searchfox.org/mozilla-central/rev/48ea452803907f2575d81021e8678634e8067fc2/servo/components/selectors/builder.rs#56. If you agree that this is a bug (and not an acceptable deviation from Gecko behavior), I'll work on a patch to change the builder structure to impose an ordering.

(In reply to Brad Werth [:bradwerth] from comment #4) > However, the builder structure holds the tokens without specifying an order between simple_selectors and combinators. I see that I spoke too soon. I see that the structure attempts to order the combinators against the selectors by carrying an corresponding length. I'll see if I can figure out what's going on. I'm still interested in your insights, of course.

Comment on attachment 8900930 [details] Bug 1391169 Part 2: Add a test of cssText with combinations of namespaces, universal selectors, and combinators. https://reviewboard.mozilla.org/r/172370/#review178292 ::: dom/base/test/test_style_cssText.html:8 (Diff revision 2) > +<meta charset="UTF-8"> > +<title>Test for Bug 1391169</title> > +<script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script> > +<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/> > +<style id="style"></style> > +<style>@namespace blahBLAH url(test_style_cssText.css)</style> An @namespace rule’s URL does not need to point to a valid file, so test_style_cssText.css can be removed. (It’s just an identifier, http://example.net/foo/bar would work just as well.) The scope of a @namespace rule is the stylesheet that contains it. Separate <style> elements introduce separate stylesheets, so this rule is doing nothing. It should instead be prepended to `input` in `testData()`. I think this fails on try because the blahBLAH|* selectors cause a parse error because of the undeclared namespace prefix.

Comment on attachment 8901350 [details] Bug 1391169 Part 1: Servo-side change Selector to_css function to handle combinators in between universal selectors. https://reviewboard.mozilla.org/r/172812/#review178294 ::: servo/components/selectors/parser.rs (Diff revision 1) > // Iterate over everything so we serialize the namespace > // too. > for simple in compound.iter() { > simple.to_css(dest)?; > } > - continue This seems wrong, or I’m missing something. The 'for' loop just above this 'continue' is intended to run instead of the one a few lines below, not in addition to. Why remove this 'continue'?

Right, this is a bit confusing. For cache locality, we want the Selector struct to contain a flat array of 'Component's (mixed simple selectors and combinators) stored in matching order. At the level of compound selectors (the things separated by combinators), we need matching to be right-to-left (in CSS source order) because we’re starting from the DOM node being matched and then walking up the tree. Within one compound selector the simple selectors can be matched in any order but we do it left-to-right because for example an element name can be matched more quickly than an attribute lookup, and we can stop at the first negative result. This is to obtain this non-linear ordering without doing extra memory allocation during parsing that SelectorBuilder is so convoluted. It stores a flat array of simple selectors, and separately tuples of (combinators, number of simple selectors since the previous combinator or start of the selector).

Comment on attachment 8901350 [details] Bug 1391169 Part 1: Servo-side change Selector to_css function to handle combinators in between universal selectors. https://reviewboard.mozilla.org/r/172812/#review178294 > This seems wrong, or I’m missing something. The 'for' loop just above this 'continue' is intended to run instead of the one a few lines below, not in addition to. Why remove this 'continue'? You're right -- I was oversimplifying. The updated version behaves correctly for the test cases I can think to exercise. I added some tests within Servo to further highlight the issue, which was this: The four step algorithm at https://drafts.csswg.org/cssom/#serializing-selectors is written in the form "1, otherwise 2, 3, 4". The current implementation treats this as "either 1 and 3 and 4, or 2" and this patch implements this as "either 1 or 2, then 3 and 4", which I believe is the correct interpretation. It appears to be the necessary interpretation to correctly serialize "* > *".

(In reply to Brad Werth [:bradwerth] from comment #13) > The four step algorithm at > https://drafts.csswg.org/cssom/#serializing-selectors is written in the form > "1, otherwise 2, 3, 4". The current implementation treats this as "either 1 > and 3 and 4, or 2" Correction: the current implementation does "either 1, or 2 and 3 and 4".

Comment on attachment 8901350 [details] Bug 1391169 Part 1: Servo-side change Selector to_css function to handle combinators in between universal selectors. https://reviewboard.mozilla.org/r/172812/#review180314

Comment on attachment 8900930 [details] Bug 1391169 Part 2: Add a test of cssText with combinations of namespaces, universal selectors, and combinators. https://reviewboard.mozilla.org/r/172370/#review180316

Looks good. Please make a pull request on https://github.com/servo/servo/ for part 1 and ping me or one of the other Servo reviewers to mark it r=SimonSapin.

Pushed by simon.sapin@exyr.org: https://hg.mozilla.org/integration/autoland/rev/e68ecb05643f Part 2: Add a test of cssText with combinations of namespaces, universal selectors, and combinators. r=SimonSapin

https://github.com/servo/servo/pull/18344 (part 1) is in autoland at https://hg.mozilla.org/integration/autoland/rev/06223c54aef5

Too late for 56. Mass won't fix for 56.