Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[cssom] In what order should properties be serialized? #2509

Closed
csnardi opened this issue Apr 6, 2018 · 4 comments
Closed

[cssom] In what order should properties be serialized? #2509

csnardi opened this issue Apr 6, 2018 · 4 comments
Labels

Comments

@csnardi
Copy link
Contributor

csnardi commented Apr 6, 2018

https://drafts.csswg.org/cssom/#serialize-a-css-declaration-block defines that shorthands should be considered in preferred order. However, it says no such thing for general longhands/properties, or how shorthands should be ordered with longhands.

This can lead to interoperability concerns, for example in https://wpt.fyi/css/css-variables/variable-cssText.html with the test of target9 where Firefox/Chrome order margin-right first but Edge orders margin-top first.

@upsuper upsuper added the cssom-1 label Apr 6, 2018
@upsuper
Copy link
Member

upsuper commented Apr 6, 2018

CSS declaration block is an ordered collection of declarations according to its definition, and that that order is the specified order. Serialization is based on that order.

That means, in this case, margin: var(--prop); margin-top: 10px; should be parsed into

/* margin-top: {placeholder}; <- dropped in favor of the later one */
margin-right: {placeholder};
margin-bottom: {placeholder};
margin-left: {placeholder};
margin-top: 10px; /* <- the new one */

So I would say the behavior of Chrome and Firefox is expected, and this is a bug of Edge.

@upsuper
Copy link
Member

upsuper commented Apr 6, 2018

I tend to close this as the spec should be clear enough for this case. The test should be fixed.

@csnardi
Copy link
Contributor Author

csnardi commented Apr 7, 2018

Yep, I didn't see that section. Looks like the canonical order should be specified in the individual specs, which for some properties isn't quite clear, e.g. for border (https://drafts.csswg.org/css-backgrounds-3/#propdef-border). I would suppose it should be border-top, border-right, border-bottom, border-left, but Chrome seems to serialize border-top, border-bottom, border-left, border-right and Firefox border-top, border-left, border-bottom, border-right. However, it isn't particularly clear which would be right.

@csnardi csnardi closed this as completed Apr 7, 2018
csnardi added a commit to csnardi/web-platform-tests that referenced this issue Apr 7, 2018
See w3c/csswg-drafts#2509 (comment) for the change to target9. Also remove the extra whitespace in target8 and target1 which causes these tests to fail in Chrome and Firefox.

Also remove testcase.propertyName from each test's name as this doesn't exist and just outputs undefined.
@upsuper
Copy link
Member

upsuper commented Apr 7, 2018

Yeah, in that case probably issues should be raised to the individual specs to make them specify the canonical order clearly.

upsuper pushed a commit to web-platform-tests/wpt that referenced this issue Apr 7, 2018
See w3c/csswg-drafts#2509 (comment) for the change to target9. Also remove the extra whitespace in target8 and target1 which causes these tests to fail in Chrome and Firefox.

Also remove testcase.propertyName from each test's name as this doesn't exist and just outputs undefined.
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 15, 2018
…rty serialization, a=testonly

Automatic update from web-platform-testsUpdate expected cssText for custom property serialization

See w3c/csswg-drafts#2509 (comment) for the change to target9. Also remove the extra whitespace in target8 and target1 which causes these tests to fail in Chrome and Firefox.

Also remove testcase.propertyName from each test's name as this doesn't exist and just outputs undefined.

wpt-commits: d0d62244432d329aa22e9278d3e83184301c3e7f
wpt-pr: 10353
wpt-commits: d0d62244432d329aa22e9278d3e83184301c3e7f
wpt-pr: 10353
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this issue Oct 2, 2019
…rty serialization, a=testonly

Automatic update from web-platform-testsUpdate expected cssText for custom property serialization

See w3c/csswg-drafts#2509 (comment) for the change to target9. Also remove the extra whitespace in target8 and target1 which causes these tests to fail in Chrome and Firefox.

Also remove testcase.propertyName from each test's name as this doesn't exist and just outputs undefined.

wpt-commits: d0d62244432d329aa22e9278d3e83184301c3e7f
wpt-pr: 10353
wpt-commits: d0d62244432d329aa22e9278d3e83184301c3e7f
wpt-pr: 10353

UltraBlame original commit: 291cf72732db4c39939013dcf4376b0847c7965e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this issue Oct 2, 2019
…rty serialization, a=testonly

Automatic update from web-platform-testsUpdate expected cssText for custom property serialization

See w3c/csswg-drafts#2509 (comment) for the change to target9. Also remove the extra whitespace in target8 and target1 which causes these tests to fail in Chrome and Firefox.

Also remove testcase.propertyName from each test's name as this doesn't exist and just outputs undefined.

wpt-commits: d0d62244432d329aa22e9278d3e83184301c3e7f
wpt-pr: 10353
wpt-commits: d0d62244432d329aa22e9278d3e83184301c3e7f
wpt-pr: 10353

UltraBlame original commit: 291cf72732db4c39939013dcf4376b0847c7965e
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this issue Oct 2, 2019
…rty serialization, a=testonly

Automatic update from web-platform-testsUpdate expected cssText for custom property serialization

See w3c/csswg-drafts#2509 (comment) for the change to target9. Also remove the extra whitespace in target8 and target1 which causes these tests to fail in Chrome and Firefox.

Also remove testcase.propertyName from each test's name as this doesn't exist and just outputs undefined.

wpt-commits: d0d62244432d329aa22e9278d3e83184301c3e7f
wpt-pr: 10353
wpt-commits: d0d62244432d329aa22e9278d3e83184301c3e7f
wpt-pr: 10353

UltraBlame original commit: 291cf72732db4c39939013dcf4376b0847c7965e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants