Xidorn Quan [:xidorn] UTC+11 Assignee
	Description • 7 years ago

In layout/reftests/w3c-css/received/css-namespaces-3/syntax-013.xml, something like
>    @namespace "test" {}
>    t { background:lime }
is broken. The style rule in the second line is not recognized, which causes this test to fail.

	Simon Sapin (:SimonSapin)
	Comment 1 • 7 years ago

https://github.com/servo/rust-cssparser/blob/master/src/rules_and_declarations.rs should take care of ending an at-rule at the first semicolon or curly-braces block, whichever comes first. I’ll investigate tomorrow.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 2 • 7 years ago

So at this moment, this reftest doesn't fail, because .xml files are not handled by stylo regardless of whether stylo is enabled. Enabling stylo for xml files is tracked in bug 1370508.

To reproduce this with a stylo build before bug 1370508 lands, you can change the extension of the test file to .html or .xhtml instead.

	Simon Sapin (:SimonSapin)
	Comment 3 • 7 years ago

Fiddling with devtools after renaming the file to .xhtml, I found that CSSOM for that stylesheet looks as expected: one style rule, no @namespace rule. But the selector still doesn’t match.

This is because, in components/style/stylesheets/rule_parser.rs in <TopLevelRuleParser as AtRuleParser>::parse_prelude, we register a namespace prefix even though the @namespace rule can still be invalid by having a {} block instead of a ; semicolon. We need to either:

* Add to cssparser a callback (AtRuleParser trait method) for the semicolon case. There’s nothing to parse there, but we’d move the side effect of namespace registration there.

* Or move registration up the stack to the users of TopLevelRuleParser. There’s two, in components/style/stylesheets/stylesheet.rs and components/style/stylesheets/mod.rs.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 4 • 7 years ago

Have a working patch. Taking it.

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 5 • 7 years ago

Hmmm...

	Xidorn Quan [:xidorn] UTC+11 Assignee
	Comment 6 • 7 years ago

cssparser: https://github.com/servo/rust-cssparser/pull/180
servo: https://github.com/servo/servo/pull/18336

	Pulsebot
	Comment 7 • 7 years ago

Pushed by xquan@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c057e3b320f0
Update expectation for this bug after landing servo/servo#18336.

	Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)
	Comment 8 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/c057e3b320f0