-
Notifications
You must be signed in to change notification settings - Fork 711
[css-variables] Are custom properties strings? #8533
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
Comments
I don't have a strong opinion on this. I'm fine with specifying they just stay as uninterpreted character sequences (rather than token sequences, specifically), so long as it's still clear and correctly implemented that comment insertion happens when you substitute things next to each other that would reparse differently. (Chrome still doesn't do this correctly when serializing, but does at least parse correctly; (The reason Anders is filing this, btw, is that currently we preserve both the original string and the tokens for every custom property, and this is a noticeable memory burden on some sites like Amazon. It would be good to preserve only one of them, and since the original string is required to be preserved for author-observable behavior, dropping the tokens seems like the more viable one.) |
https://w3c.github.io/csswg-drafts/css-variables/#serializing-custom-props
So it seems to me that Oh but good point about #8376, then I'm not sure what's best. |
In #8376 there's a suggestion that we just do the "looks numeric" check for So given |
Given I think this issue also applies to properties declared with arbitrary substitution values (eg. Given |
No, this is specifically not the case when serializing custom properties; see https://w3c.github.io/csswg-drafts/css-variables/#serializing-custom-props
It's not yet clear if we'll allow style() queries on non-custom properties, but I can definitely say that if we do allow them, then no, that query would match. We'd at minimum base the comparison on the used-value serialization, which absolutely normalizes these things. This'll have nothing to do with whatever we do to custom properties. |
Ok, so this is a browser issue. style.cssText = '--x: /* comment */ 1 /* comment */'
style.cssText; // (Chrome) `--x: 1 ;` (FF) `--x: 1 /* comment */;`
style.setProperty('--x', '/* comment */ 1 /* comment */')
style.getPropertyValue('--x') // (Chrome) ` 1 ` (FF) `1 /* comment */` Both may store comment representations with whitespace representations. FF does not remove trailing whitespaces (cf. #6484). If |
@cdoublev is right here. Comments are effectively not dropped within the value, but the bounds of the value are still determined by tokens, and comments do not produce tokens. Example 17 in Variables is not possible per css-syntax.
Fully acknowledged, but it's also a spec issue. :-) |
CSS Syntax 3 says (emphasize added):
https://drafts.csswg.org/css-syntax-3/#serialization But either implementations must preserve comments, or CSS Variables should not require serializing with comments. I also note that serializing
https://drafts.csswg.org/cssom-1/#serialize-a-css-declaration I deleted my comment where I said that serializing exactly as specified only for Preserving the whole declaration string would also prevent |
Is a custom property a token sequence that only serializes as the original string, or is it that original string? The difference can be seen in this example:
If custom properties are token sequences, this style query matches, but not if they are strings.
The point of serializing using the original string was to preserve "unknown stuff", and maintaining that idea probably means that we don't transform that unknown stuff before comparisons. Otherwise e.g.
style(--uuid: 12345678-12e2-8d9b-a456-426614174000)
would match--uuid: 12345678-1200-8d9b-a456-426614174000
(example stolen from https://drafts.csswg.org/css-variables/#serializing-custom-props, then modified).Note that
style(--x:1)
would still match e.g.--x:1.00
according to the current plan in #8376, since the literal side matches<number>
.@tabatkins
The text was updated successfully, but these errors were encountered: