1774060 - Update animations if the referenced scroll-timeline property gets changed

Status: ASSIGNED

(Core :: CSS Transitions and Animations, defect, P3)

Description

[Boris Chiou [:boris]]

We have to make sure we update the CSS animations properly when change scroll-timeline-name and/or scroll-timeline-axis.

Comment 1

[Boris Chiou [:boris]]

Attachment: Bug 1774060 - Update animations if the referenced scroll-timeline property gets changed (by a hashmap with element set).

If scroll-timeline-name or scroll-timeline-axis are changed, we have to
update the scroll animations for the element itself, the following
siblings, and their descendants because scroll-timeline is discoverable
by those elements.

In order to avoid from traversing the entire dom tree, we need to
introduce a new hash table. The key is the timeline name in animation-timeline
we are using, and the value is the animations who are using this
timeline name.

Therefore, if we change scroll-timeline-name, we can just look up the
animations from this hash table, and try to update animations based on
those animation targets. (We may remove animations, so we call
UpdateAnimations() for those animation targets, instead of replacing the
timeline directly.)

Comment 2

[Boris Chiou [:boris]]

Attachment: Bug 1774060 - Update animations if the referenced scroll-timeline property gets changed (by a hashmap with CSSAnimation set).

If scroll-timeline-name or scroll-timeline-axis are changed, we have to
update the scroll animations for the element itself, the following
siblings, and their descendants because scroll-timeline is discoverable
by those elements.

In order to avoid from traversing the entire dom tree, we need to
introduce a new hash table. The key is the timeline name in animation-timeline
we are using, and the value is the animations who are using this
timeline name.

Therefore, if we change scroll-timeline-name, we can just look up the
animations from this hash table, and try to update animations based on
those animation targets. (We may remove animations, so we call
UpdateAnimations() for those animation targets, instead of replacing the
timeline directly.)

Comment 3

[Boris Chiou [:boris]]

Uncleared part (Copied from the comments in the patch):

Hi Brian, sorry for bothering you about this. I guess you may know more about this: What is the expected behavior for "animation-timeline: none;" and "animation-timeline: unknown-timeline", for a CSS animation.

There are 4 cases:

1. set animation-timeline: none initially
2. set animation-timeline: unknown-timeline initially
3. change animation-timeline from a scroll-timeline name to none
4. change animation-timeline from a scroll-timeline name to unknown-timeline

Basically, we set the Animation::mTimeline to null for these cases. But do we have to discard the CSS animation with a null timeline?

I just notice there is a wpt test which expects to keep the CSS animation without a timeline to zero time:
https://chromium-review.googlesource.com/c/chromium/src/+/2874659/2/third_party/blink/web_tests/external/wpt/scroll-animations/css/animation-timeline-none.html.

And these is another wpt (https://searchfox.org/mozilla-central/rev/97c13320e56884daf14016048e9d2182c880f8a9/testing/web-platform/tests/scroll-animations/css/at-scroll-timeline-dynamic.tentative.html#174-196)
which expects the CSS animations with null timeline (case 3 and case 4) have no effect (e.g. this may be caused by the unresolved current time or by discarding the CSS animation).

So now I'm confused about these cases. Should we keep these null-timeline CSS animations (i.e. still generate the CSSAnimation objects) but just make sure they are at zero time, or should we just discard them by returning null CSSAnimation object in UpdateAnimations()? Or maybe we have to use the previous progress in case 3 and case 4 based on web-animation-2 (https://drafts.csswg.org/web-animations/#setting-the-timeline)

Note:
I guess case 1 and case 2 are different from case 3 and case 4. Case 3 and case 4 are changed from a non-null timeline to a null timeline, so they go into the procedure of setting a new timeline. However, case 1 and case 2 are something like setting a null timeline to a null timeline, so they are no-op in the procedure of setting a new timeline.

Comment 4

[Brian Birtles (:birtles)]

Hi Boris!

(In reply to Boris Chiou [:boris] from comment #3)

So now I'm confused about these cases. Should we keep these null-timeline CSS animations (i.e. still generate the CSSAnimation objects) but just make sure they are at zero time, or should we just discard them by returning null CSSAnimation object in UpdateAnimations()? Or maybe we have to use the previous progress in case 3 and case 4 based on web-animation-2 (https://drafts.csswg.org/web-animations/#setting-the-timeline)

I guess this needs to be defined in CSS Animations 2.

One suggestion is to make the same as @keyframes rules. 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.

If the other browser vendors agree with that behavior I think that would mean:

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 scroll-timeline name to none

The existing CSSAnimation is cancelled.

4. change animation-timeline from a scroll-timeline name to unknown-timeline

The existing CSSAnimation is cancelled.

I think that would make sense. 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.

I think not generating a CSSAnimation is more consistent with how we handle @keyframes so I'd raise a spec issue for that and see if Blink agree (in which case it sounds like they would need to update their implementation).

Comment 5

[Boris Chiou [:boris]]

Thanks, Brian.

(In reply to Brian Birtles (:birtles) from comment #4)

I think that would make sense. 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.

I think not generating a CSSAnimation is more consistent with how we handle @keyframes so I'd raise a spec issue for that and see if Blink agree (in which case it sounds like they would need to update their implementation).

Awesome. I will address this in Bug 1773149. Don't generate the CSSAnimation when animation-timeline is none or unknown-timeline. So I will design the hash table based on this assumption in this bug.+

Comment 6

[Boris Chiou [:boris]]

Just filed https://github.com/w3c/csswg-drafts/issues/7364.