-
Notifications
You must be signed in to change notification settings - Fork 719
[css-animations-2] Make animation shorthand syntax future-proof #6946
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
Maybe one could just prefix the animation name with a double dash This seems to work in browsers (latest Chrome, Firefox and Safari): <style>
@keyframes --move {
to {
transform: translateY(2em);
}
}
p {
animation: --move 1s alternate infinite;
}
style>
<p>👀p> As a data URI:
And if I read the spec correctly, |
That's also a solution that would work without a spec. change. Though I'm afraid without new syntax the pressure to switch away from names that can clash with keywords isn't big enough. Sebastian |
… style system. r=emilio This patch introduces animation-composition longhand but we don't accept it in @Keyframe rule for now. I will support this for @Keyframe in the patch series. Besides, the shorthand of animation doesn't include animation-composition. The spec issue is: w3c/csswg-drafts#6946. We could fix the shorthand once this spec issue gets updated. Differential Revision: https://phabricator.services.mozilla.com/D150299
… style system. r=emilio This patch introduces animation-composition longhand but we don't accept it in @Keyframe rule for now. I will support this for @Keyframe in the patch series. Besides, the shorthand of animation doesn't include animation-composition. The spec issue is: w3c/csswg-drafts#6946. We could fix the shorthand once this spec issue gets updated. Differential Revision: https://phabricator.services.mozilla.com/D150299
It seems that making animation shorthand supports animation-composition may be very tricky, so it's unlikely to include animation-composition into the shorthand for now, per spec issue: w3c/csswg-drafts#6946. WebKit also supports the longhand only on STP (Safari Technology Preview), so it should be fine to enable the longhand property only on Firefox Nightly, for experiemental testing. Differential Revision: https://phabricator.services.mozilla.com/D154934
Given the implementation of Sebastian |
Simplest solution to unblock is add it to the shorthand as a reset-only sub-property. But agree that it would be useful to get this solved going forward. |
The CSS Working Group just discussed The full IRC log of that discussion |
The CSS Working Group just discussed
The full IRC log of that discussion |
Given our existing disambiguation rules favoring an animation-name coming last (and the canonical order being written to support that), I don't think an option having animation-name coming first is possible actually. We could still adopt @flackr's suggestion of the new longhands only being allowed after an unambiguous animation-name (that is, a keyword that's either not valid for any Animations 1 longhand, or one that is valid for a property but which has already had that property claim an earlier keyword, like in This is why I favor some variety of optional prefix for the animation-name, and only allowing the Animations 2+ longhands if animation-name is written with this prefix. We have a precedent for using the The other possibility is using a keyword prefix. This has the advantage of probably being what we'll need to accommodate more custom-idents, like the timeline phase name - it might get added to the shorthand as @fantasai pointed out in the call that this "prefix an ambiguous value with a disambiguating keyword" currently has only been used in functions; it hasn't shown up in a property value yet, so we should make sure it's what we want before we establish the precedent. So, my suggestion is that we allow the animation-name component to be prefixed with a |
Summarizing the suggestions for the syntax changes so far:
Sebastian |
That was the initial suggestion and also what I and obviously @tabatkins would prefer.
If we're going with prefixes, we still need to enforce the order somehow. Otherwise we'll have another potential name conflict. That time between the prefix keyword and the longhands keywords. I'd actually separate future custom-idents with another slash instead. This avoids any conflicts of the longhand keywords. Though it would also always require to set the value for the animation name. So it would be
One of my initial suggestions. It would clearly distinguish the name from the keywords, though given that we normally don't have names as strings probably not the best option. And it would still require a solution like above for future name-longhands.
As Tab wrote, this makes it even more complicated for authors to match what's a name and what's a keyword.
See my point on this above.
As Tab wrote, this is not possible given that the name currently comes last when interpreting the values. Sebastian |
We currently allow the timeline to be specified as part of the animation shorthand. This is (for now) incorrect per a recent CSSWG resolution [1]. This CL changes all WPTs to use the animation-timeline longhand instead. I've also added a setup assertion (or equivalent) to many tests to prevent UAs without support for animation-timeline from running the animations on the document timeline (which could lead to unstable/timing-dependent failure results). [1] w3c/csswg-drafts#6946 (comment) Fixed: 1397870 Change-Id: I97cd13481ea00a074f50c132562d9bb83e7bc4dd
We currently allow the timeline to be specified as part of the animation shorthand. This is (for now) incorrect per a recent CSSWG resolution [1]. This CL changes all WPTs to use the animation-timeline longhand instead. I've also added a setup assertion (or equivalent) to many tests to prevent UAs without support for animation-timeline from running the animations on the document timeline (which could lead to unstable/timing-dependent failure results). [1] w3c/csswg-drafts#6946 (comment) Fixed: 1397870 Change-Id: I97cd13481ea00a074f50c132562d9bb83e7bc4dd
We currently allow the timeline to be specified as part of the animation shorthand. This is (for now) incorrect per a recent CSSWG resolution [1]. This CL changes all WPTs to use the animation-timeline longhand instead. I've also added a setup assertion (or equivalent) to many tests to prevent UAs without support for animation-timeline from running the animations on the document timeline (which could lead to unstable/timing-dependent failure results). [1] w3c/csswg-drafts#6946 (comment) Fixed: 1397870 Change-Id: I97cd13481ea00a074f50c132562d9bb83e7bc4dd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4084846 Reviewed-by: Kevin EllisCommit-Queue: Anders Hartvoll Ruud Cr-Commit-Position: refs/heads/main@{#1082380}
We currently allow the timeline to be specified as part of the animation shorthand. This is (for now) incorrect per a recent CSSWG resolution [1]. This CL changes all WPTs to use the animation-timeline longhand instead. I've also added a setup assertion (or equivalent) to many tests to prevent UAs without support for animation-timeline from running the animations on the document timeline (which could lead to unstable/timing-dependent failure results). [1] w3c/csswg-drafts#6946 (comment) Fixed: 1397870 Change-Id: I97cd13481ea00a074f50c132562d9bb83e7bc4dd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4084846 Reviewed-by: Kevin EllisCommit-Queue: Anders Hartvoll Ruud Cr-Commit-Position: refs/heads/main@{#1082380}
We currently allow the timeline to be specified as part of the animation shorthand. This is (for now) incorrect per a recent CSSWG resolution [1]. This CL changes all WPTs to use the animation-timeline longhand instead. I've also added a setup assertion (or equivalent) to many tests to prevent UAs without support for animation-timeline from running the animations on the document timeline (which could lead to unstable/timing-dependent failure results). [1] w3c/csswg-drafts#6946 (comment) Fixed: 1397870 Change-Id: I97cd13481ea00a074f50c132562d9bb83e7bc4dd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4084846 Reviewed-by: Kevin EllisCommit-Queue: Anders Hartvoll Ruud Cr-Commit-Position: refs/heads/main@{#1082380}
…and should set timeline to auto, a=testonly Automatic update from web-platform-tests [scroll-animations] The animation shorthand should set timeline to auto We currently allow the timeline to be specified as part of the animation shorthand. This is (for now) incorrect per a recent CSSWG resolution [1]. This CL changes all WPTs to use the animation-timeline longhand instead. I've also added a setup assertion (or equivalent) to many tests to prevent UAs without support for animation-timeline from running the animations on the document timeline (which could lead to unstable/timing-dependent failure results). [1] w3c/csswg-drafts#6946 (comment) Fixed: 1397870 Change-Id: I97cd13481ea00a074f50c132562d9bb83e7bc4dd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4084846 Reviewed-by: Kevin EllisCommit-Queue: Anders Hartvoll Ruud Cr-Commit-Position: refs/heads/main@{#1082380} -- wpt-commits: 7bb84834a2de052462b03f12c5625a65a391869c wpt-pr: 37385
…and should set timeline to auto, a=testonly Automatic update from web-platform-tests [scroll-animations] The animation shorthand should set timeline to auto We currently allow the timeline to be specified as part of the animation shorthand. This is (for now) incorrect per a recent CSSWG resolution [1]. This CL changes all WPTs to use the animation-timeline longhand instead. I've also added a setup assertion (or equivalent) to many tests to prevent UAs without support for animation-timeline from running the animations on the document timeline (which could lead to unstable/timing-dependent failure results). [1] w3c/csswg-drafts#6946 (comment) Fixed: 1397870 Change-Id: I97cd13481ea00a074f50c132562d9bb83e7bc4dd Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4084846 Reviewed-by: Kevin EllisCommit-Queue: Anders Hartvoll Ruud Cr-Commit-Position: refs/heads/main@{#1082380} -- wpt-commits: 7bb84834a2de052462b03f12c5625a65a391869c wpt-pr: 37385
The CSS Working Group just discussed
The full IRC log of that discussion |
This patch introduces animation-composition longhand but we don't accept it in @Keyframe rule for now. I will support this for @Keyframe in the patch series. Besides, the shorthand of animation doesn't include animation-composition. The spec issue is: w3c/csswg-drafts#6946. We could fix the shorthand once this spec issue gets updated. Differential Revision: https://phabricator.services.mozilla.com/D150299
This patch introduces animation-composition longhand but we don't accept it in @Keyframe rule for now. I will support this for @Keyframe in the patch series. Besides, the shorthand of animation doesn't include animation-composition. The spec issue is: w3c/csswg-drafts#6946. We could fix the shorthand once this spec issue gets updated. Differential Revision: https://phabricator.services.mozilla.com/D150299
This patch introduces animation-composition longhand but we don't accept it in @Keyframe rule for now. I will support this for @Keyframe in the patch series. Besides, the shorthand of animation doesn't include animation-composition. The spec issue is: w3c/csswg-drafts#6946. We could fix the shorthand once this spec issue gets updated. Differential Revision: https://phabricator.services.mozilla.com/D150299
This patch introduces animation-composition longhand but we don't accept it in @Keyframe rule for now. I will support this for @Keyframe in the patch series. Besides, the shorthand of animation doesn't include animation-composition. The spec issue is: w3c/csswg-drafts#6946. We could fix the shorthand once this spec issue gets updated. Differential Revision: https://phabricator.services.mozilla.com/D150299
This patch introduces animation-composition longhand but we don't accept it in @Keyframe rule for now. I will support this for @Keyframe in the patch series. Besides, the shorthand of animation doesn't include animation-composition. The spec issue is: w3c/csswg-drafts#6946. We could fix the shorthand once this spec issue gets updated. Differential Revision: https://phabricator.services.mozilla.com/D150299
This patch introduces animation-composition longhand but we don't accept it in @Keyframe rule for now. I will support this for @Keyframe in the patch series. Besides, the shorthand of animation doesn't include animation-composition. The spec issue is: w3c/csswg-drafts#6946. We could fix the shorthand once this spec issue gets updated. Differential Revision: https://phabricator.services.mozilla.com/D150299
This patch introduces animation-composition longhand but we don't accept it in @Keyframe rule for now. I will support this for @Keyframe in the patch series. Besides, the shorthand of animation doesn't include animation-composition. The spec issue is: w3c/csswg-drafts#6946. We could fix the shorthand once this spec issue gets updated. Differential Revision: https://phabricator.services.mozilla.com/D150299
This patch introduces animation-composition longhand but we don't accept it in @Keyframe rule for now. I will support this for @Keyframe in the patch series. Besides, the shorthand of animation doesn't include animation-composition. The spec issue is: w3c/csswg-drafts#6946. We could fix the shorthand once this spec issue gets updated. Differential Revision: https://phabricator.services.mozilla.com/D150299
In order to avoid this unexpected fail, we add this tentative serialization for animation shorthand. 1. animation-timeline is reset-only in this shorthand, so we don't parse it and we reset it to `vec![auto]`. 2. we don't serialize the shorthand if animation-timeline is not the initial value. This is a tentative solution and we should update it with the new syntax once the spec gets update (w3c/csswg-drafts#6946). Differential Revision: https://phabricator.services.mozilla.com/D217164 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1908819 gecko-commit: 4f6324b24c80d4fdf4d825d009ec84d9c76e85d7 gecko-reviewers: firefox-style-system-reviewers, layout-reviewers, emilio
…. r=firefox-style-system-reviewers,layout-reviewers,emilio In order to avoid this unexpected fail, we add this tentative serialization for animation shorthand. 1. animation-timeline is reset-only in this shorthand, so we don't parse it and we reset it to `vec![auto]`. 2. we don't serialize the shorthand if animation-timeline is not the initial value. This is a tentative solution and we should update it with the new syntax once the spec gets update (w3c/csswg-drafts#6946). Differential Revision: https://phabricator.services.mozilla.com/D217164
…. r=firefox-style-system-reviewers,layout-reviewers,emilio In order to avoid this unexpected fail, we add this tentative serialization for animation shorthand. 1. animation-timeline is reset-only in this shorthand, so we don't parse it and we reset it to `vec![auto]`. 2. we don't serialize the shorthand if animation-timeline is not the initial value. This is a tentative solution and we should update it with the new syntax once the spec gets update (w3c/csswg-drafts#6946). Differential Revision: https://phabricator.services.mozilla.com/D217164
In order to avoid this unexpected fail, we add this tentative serialization for animation shorthand. 1. animation-timeline is reset-only in this shorthand, so we don't parse it and we reset it to `vec![auto]`. 2. we don't serialize the shorthand if animation-timeline is not the initial value. This is a tentative solution and we should update it with the new syntax once the spec gets update (w3c/csswg-drafts#6946). Differential Revision: https://phabricator.services.mozilla.com/D217164 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1908819 gecko-commit: 4f6324b24c80d4fdf4d825d009ec84d9c76e85d7 gecko-reviewers: firefox-style-system-reviewers, layout-reviewers, emilio
…. r=firefox-style-system-reviewers,layout-reviewers,emilio In order to avoid this unexpected fail, we add this tentative serialization for animation shorthand. 1. animation-timeline is reset-only in this shorthand, so we don't parse it and we reset it to `vec![auto]`. 2. we don't serialize the shorthand if animation-timeline is not the initial value. This is a tentative solution and we should update it with the new syntax once the spec gets update (w3c/csswg-drafts#6946). Differential Revision: https://phabricator.services.mozilla.com/D217164
Should the css-animations-2 spec be updated to remove |
…` and `animation-range-end` reset-only longhands of the `animation` shorthand https://bugs.webkit.org/show_bug.cgi?id=287398 Reviewed by Tim Nguyen. The CSS WG resolved in w3c/csswg-drafts#6946 (comment) to make the new CSS properties introduced to support Scroll-driven Animations – `animation-timeline`, `animation-range-start` and `animation-range-end` – reset-only longhands of the `animation` shorthand. To that end, we add those properties to the `"longhands"` property for `animation` in `CSSProperties.json` and deal with them when relevant: - in `CSSPropertyParser.cpp` we set `nullptr` values for those properties to reset any value previously set and ensure those properties have the right initial values for serialization purposes. - in `ComputedStyleExtractor.cpp` we return an empty value whenever we attempt to serialize the `animation` shorthand in the computed style when one of those reset-only longhands was otherwise set. - in `ShorthandSerializer.cpp` we do similar work when serializing the specified value. * LayoutTests/imported/w3c/web-platform-tests/css/css-animations/parsing/animation-shorthand-expected.txt: * LayoutTests/imported/w3c/web-platform-tests/scroll-animations/css/animation-shorthand-expected.txt: * Source/WebCore/css/CSSProperties.json: * Source/WebCore/css/ComputedStyleExtractor.cpp: (WebCore::animationShorthandValue): * Source/WebCore/css/ShorthandSerializer.cpp: (WebCore::ShorthandSerializer::serializeLayered const): * Source/WebCore/css/parser/CSSPropertyParser.cpp: (WebCore::initialValueForLonghand): (WebCore::consumeAnimationValueForShorthand): (WebCore::CSSPropertyParser::consumeAnimationShorthand): Canonical link: https://commits.webkit.org/290168@main
Splitting this from the discussion in #6930.
The
animation
shorthand property has this note regarding the interpretation of its syntax:So keywords are preferrably interpreted as values for longhand properties other than
animation-name
.This comes with the big downside that no new keywords can be introduced without risking to break existing pages. This is, if a keyword is introduced that is currently used as a
value, it will then be interpreted as the keyword for another longhand instead.Therefore, I suggest to change the syntax of
animation
in a way that theanimation-name
value can always be clearly distinguished from the other values.Two solutions that come to my mind are separating it by a slash or allowing to define it as a
.Of course, the existing syntax still needs to be supported as well to avoid breaking the web. And the transition to a new one will take many years.
Though in order to allow extending the existing features or add new ones (like
animation-composition
) that are covered by the shorthand I believe this is a necessary change.Sebastian
The text was updated successfully, but these errors were encountered: