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

[css-animations] Serialize all keyframe names as strings #2435

Open
csnardi opened this issue Mar 13, 2018 · 4 comments
Open

[css-animations] Serialize all keyframe names as strings #2435

csnardi opened this issue Mar 13, 2018 · 4 comments
Labels
css-animations-1 Current Work

Comments

@csnardi
Copy link
Contributor

csnardi commented Mar 13, 2018

After #118 and #801, support for naming a keyframe the same as a reserved keyword by using a <string> was added to parsing and CSSOM setting. The WPT test for this part of the spec seems to assume that if a keyword could be parsed as an <ident>, then it would be serialized as a string (with quotation marks); otherwise, no quotation marks would be used. However, this seems needlessly complex; wouldn't it be easiest just to serialize all keyframe names as strings? That way, it would be ensured that re-parsing the output would generate an identical rule, without having to check if the name is an <ident>.

@fantasai fantasai added the css-animations-1 Current Work label Mar 29, 2018
@fantasai
Copy link
Collaborator

This is likely Web-incompatible, since Firefox and Blink currently return bare identifiers in getComputedStyle.
It's also inconsistent with the principle that CSS-internal user-chosen names use identifiers and strings are only used when interfacing with an external system.

I think what would make more sense is to exclude the CSS-wide keywords and 'none' from being animation names, as the <custom-ident> type recommends. We could still parse in strings as required by #118 but they would compute to <custom-ident> and any that match an excluded keyword would be invalid. This is effectively what we do for line names in CSS Grid, for example.

@css-meeting-bot
Copy link
Member

The CSS Working Group just discussed Serialize all keyframe names as strings, and agreed to the following:

  • RESOLVED: serialize all keyframes as identifiers
The full IRC log of that discussion <dael> Topic: Serialize all keyframe names as strings
<dael> github: https://github.com//issues/2435
<fantasai> https://github.com//issues/2435
<dael> fantasai: Question about...as we were going through c omputed values do we need to preserve a distinction between animation names that are strings and ones that are identifies
<dael> fantasai: You can use either for keyframes
<dael> fantasai: Main issue afaict is we can't serialize things as strings. They have to fundimentally be identifiers. One case we can't treat them to compute as the same there's CSS-wide identifiers like 'none' where if you wanted to name your animation that you can't refer to it in the property
<dael> fantasai: I don't think it's an issue.
<dael> fantasai: Nice if we didn't have to maintain the difference
<dael> myles_: Web compat risk?
<Rossen_> 'auto'
<dael> fantasai: If someone relies o n web animation name as 'initial' 'inherit' or 'none' yes. Otherwise no.
<dael> Rossen_: Any other concerns, comments, suggestions?
<dael> Rossen_: Objections?
<dael> fantasai: Proposal is serialize all keyframes as identifiers
<dael> Rossen_: Objections?
<dael> RESOLVED: serialize all keyframes as identifiers
<dael> dbaron: Somethinggs wouldn't round trip?
<dael> fantasai: CSS wide things. I propose we make them invalid and that's why they don't round trip.
<dael> dbaron: Is someone volunteering to change first?
<dael> Rossen_: I don't hear any. Are you dbaron ?
<dael> dbaron: Not particularly
<dael> fantasai: I believe current behavior was a recent change. 2016.
<dael> fantasai: Before 2016 you could not have a keyframe name that is a string. That was a webcompat problem because webkit supported a string in that position as animation name. We changed to allow strings or idents but decided to keep them distinct. Only reason is if you have 'initial' or one of those values.
<dael> fantasai: It's r eally about will wemaintain that if you put in a string or an ident both end up as an ident
<dael> fremy: We still don't support strings in Edge. If someone is using 'none' they're doubly failing. I don't feel like making the change is end of the world
<dael> dbaron: Other question; are you proposing this for values of prop, @Keyframes or both
<fantasai> https://github.com//issues/118
<dael> fantasai: Anywhere accepting a string as an animation name.
<dael> fantasai: Proposal is everywhere issue #118 says we can accept a string we treat it at parse time as an ident
<dael> myles_: Do we need a rule for idents that aren't 'auto' and the list?
<dael> fantasai: Have that rule already.
<fantasai> https://www.w3.org/TR/css-values-3/#custom-idents
<dael> fremy: Yes, because you need it for font-family
<dbaron> font-family is extra-weird
<dael> Rossen_: Still not hearing reason to change resolution

@ewilligers
Copy link
Contributor

Do we need a more precise resolution before updating the spec and tests? I gather from a PR discussion that we need to serialize as strings whenever we do not have a valid animation-name custom identifier (e.g. '"Initial"', '"None"', '" x "'). Also, do we want to ban the empty string '""' ?

@yisibl
Copy link
Contributor

yisibl commented Oct 6, 2022

Is there any progress here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-animations-1 Current Work
Projects
None yet
Development

No branches or pull requests

5 participants