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

Match whitespace for custom property serialization #10318

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@csnardi
Member

csnardi commented Apr 4, 2018

When serializing custom properties, they must be preserved exactly as authored (https://crbug.com/661854). variable-invalidation.html added additional whitespace to the expectation, going against this part of the spec. This change makes the test pass in Firefox, Chrome, and Edge.

Match whitespace for custom property serialization
When serializing custom properties, they must be preserved exactly as authored (https://crbug.com/661854). variable-invalidation.html added additional whitespace to the expectation, going against this part of the spec. This change makes the test pass in Firefox, Chrome, and Edge.
@w3c-bots

This comment has been minimized.

w3c-bots commented Apr 4, 2018

Build PASSED

Started: 2018-04-04 22:20:59
Finished: 2018-04-04 22:26:25

View more information about this build on:

@csnardi

This comment has been minimized.

Member

csnardi commented Apr 17, 2018

@upsuper Could you review this? Fairly similar to #10353.

@upsuper

I believe that the working group has resolved to something different that, when parsing, whitespaces are trimmed from start and end of token stream, and when serializing, a whitespace is always added after the colon.

The specs may need some update if they haven't been, so do implemenations.

@upsuper

This comment has been minimized.

Member

upsuper commented Apr 17, 2018

It is possible that I was wrong... Let me find out the resolution...

@csnardi

This comment has been minimized.

Member

csnardi commented Apr 17, 2018

I think you're correct @upsuper, I believe that's w3c/csswg-drafts#1986 you're describing? If I read that correctly, the way the test currently is would be correct per that change (and so this PR should be closed).

@upsuper

This comment has been minimized.

Member

upsuper commented Apr 17, 2018

So the resolution was made in w3c/csswg-drafts#774. There was a conflicting resolution in w3c/csswg-drafts#881 which describes what Firefox currently does, which we probably should close.

@csnardi

This comment has been minimized.

Member

csnardi commented Apr 17, 2018

I believe this PR is then incorrect in at least some form for both the current spec and the proposed changes, so I'm closing this.

@csnardi csnardi closed this Apr 17, 2018

@csnardi csnardi deleted the csnardi:variable-whitespace branch Apr 18, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment