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-2] Consider not generating a CSSAnimation with null timeline #7364

Closed
BorisChiou opened this issue Jun 14, 2022 · 4 comments
Closed

Comments

@BorisChiou
Copy link
Contributor

BorisChiou commented Jun 14, 2022

I'd like to make this proposal for the following cases:

  1. Set animation-timeline: none initially
  2. Set animation-timeline: unknown-timeline initially
  3. Change animation-timeline from a valid timeline to none
  4. Change animation-timeline from a valid timeline to an unknown-timeline

animation-timeline is defined in [css-animations-2], and the timeline name is defined in [scroll-animations]. However, it's unclear to us for null timeline case.

There is a wpt for case 1 and case 2. In this test case, we expect to hold the current time at zero. This means we still generate the CSS animation with null timeline.

However, there is another wpt for case 3 and case 4. In this test case, we expect to discard the CSS animations (or say, the CSS animations are cancelled or not in effect).

This may confuse people because all the cases should have the same behavior in general, just like what we do for @keyframes.
That is, if there is no matching @keyframes rule, we don't generate a CSSAnimation. If the @keyframes rule later matches, we generate the CSSAnimation then. That is defined in step 3 of the procedure to generate keyframe objects:

If there is no @keyframes at-rule with <keyframes-name> matching name, abort this procedure. In this case no animation is generated, and any existing animation matching name is canceled.

So just want to raise this spec issue for discussion. And based on the comment @birtles left in https://bugzilla.mozilla.org/show_bug.cgi?id=1774060#c4, for these 4 cases:

Option 1:

  1. Set animation-timeline: none initially -> No CSSAnimation is generated
  2. Set animation-timeline: unknown-timeline initially -> No CSSAnimation is generated
  3. Change animation-timeline from a valid timeline to none -> The existing CSSAnimation is cancelled.
  4. Change animation-timeline from a valid timeline to an unknown-timeline -> The existing CSSAnimation is cancelled.

Option 2:
The alternative is to always generate a CSSAnimation and set the timeline to null when animation-timeline is none or some unknown-timeline and follow the procedure for setting a timeline to null defined in [web-animations-2].

Mozilla prefers the option 1 for now, and so I'd like to update this wpt based on the option 1.

Do we have any concerns for this?

cc @kevers-google, @andruud, @hiikezoe

@BorisChiou BorisChiou changed the title [css-animations-2] Consider not to generate the CSS animations with null timeline [css-animations-2] Consider not to generate a CSSAnimation with null timeline Jun 14, 2022
@BorisChiou BorisChiou changed the title [css-animations-2] Consider not to generate a CSSAnimation with null timeline [css-animations-2] Consider not generating a CSSAnimation with null timeline Jun 14, 2022
@birtles
Copy link
Contributor

birtles commented Jun 15, 2022

Option 1:

  1. Set animation-timeline: none initially -> No CSSAnimation is generated
  2. Set animation-timeline: unknown-timeline initially -> No CSSAnimation is generated
  3. Change animation-timeline from a valid timeline to none -> The existing CSSAnimation is cancelled.
  4. Change animation-timeline from a valid timeline to an unknown-timeline -> The existing CSSAnimation is cancelled.

Option 2: The alternative is to always generate a CSSAnimation and set the timeline to null when animation-timeline is none or some unknown-timeline and follow the procedure for setting a timeline to null defined in [web-animations-2].

Thanks Boris for this great summary. I was thinking over this problem again this morning and I think I overlooked something.

I'm afraid I'm unfamiliar with how scroll animations are expected to be used, but do we expect authors to update the computed style of animation-timeline such that the progress is preserved?

If so, I think option 2 would be preferred.

Presumably, in that case, when animation-timeline is updated from one valid timeline to another (including auto), we would end up running the procedure to set the timeline of an animation.

In order to be consistent, when setting the timeline to none or some unknown timeline, we should probably also run that same procedure.

That would suggest that having a non-null timeline is not a prerequisite for generating a CSSAnimation object.

@BorisChiou
Copy link
Contributor Author

Thanks, Brian. Option 2 also makes sense to me. It seems Chrome Canary doesn't preserve the progress when changing the timeline from scroll-timeline to none. However, [web-animations-2] is still under development, so just want to make sure which option is what we expect for the future.

@BorisChiou
Copy link
Contributor Author

The step 11 of setting-the-timeline in web-animations-2:

If from finite timeline and previous progress is resolved,
-> Run the procedure to set the current time to previous progress * [end time].

So this is a general case when changing the timeline from a scroll-timeline to any other one (including null timeline).
If there is an end time and previous progress, we set the current time for the animation.

If this is the use case that the developers want, option 2 may be what we need.

@BorisChiou
Copy link
Contributor Author

Close this per the previous comment. We should rely on the procedure to set the timeline of an animation. So if the behavior is inconsistent, we should update the procedure.

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

No branches or pull requests

2 participants