Bug 222939 – [Web Animations] setKeyframes does not preserve animation's current offset

• Status: RESOLVED
• Resolution: FIXED
• Component: Animations

[Liam DeBeasi]

Created attachment 422613 [details]
Code reproduction

When using setKeyframes() on an animation, WebKit does not preserve the animation's current offset. For example, if an animation is at 100% progress and setKeyframes() is called, WebKit will reset the animation's progress to 0%.

Steps to reproduce:

1. Open attached code reproduction in either Safari for iOS or macOS.
2. Observe that a blue square translates from left to right and then stops.
3. Press "Update Keyframes". You should notice that the square jumps back to the left/starting position.

Expected Behavior:

I would expect that after pressing "Update Keyframes" that the square stays at its ending position.

Actual Behavior:

The square is reset back to its starting position.

Other Information:

- I can reproduce this in STP 121 and iOS 14.5, though I can reproduce this on stable releases as well.
- Both Chrome and Firefox preserve the animation's current offset as expected.
- This is only an issue if "fill: 'both'" is set on the animation.

[Radar WebKit Bug Importer]

<rdar://problem/75207793>

[Antoine Quint]

Created attachment 422677 [details]
Simplified testcase

Simplified the test case to not involve accelerated animations and not require interaction.

[Antoine Quint]

Interestingly, the output of animation.effect.getComputedTiming() shows the expected values, so I think this is purely about presentation and not the animation model.

[Antoine Quint]

What happens is that calling seyKeyframes() empties m_blendingKeyframes and we rely on that to have KeyframeEffect::animatedProperties() return the effect's properties inside of DocumentTimeline::animationCanBeRemoved(). Because we are not aware of any animated properties, the animation is marked as removable and retired from the keyframe effect stack, and thus it is no longer invalidated during the ensuing style update.

I think all this needs is for KeyframeEffect::animatedProperties() to return properties from the parsed keyframes until the next style update when m_blendingKeyframes will be reset.

[Antoine Quint]

Created attachment 422700 [details]
Patch

[EWS]

Committed r274165: <https://commits.webkit.org/r274165>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 422700 [details].

[Liam DeBeasi]

That was fast, thank you! I will keep an eye out for this fix whenever it lands in STP.

[Antoine Quint]

(In reply to Liam DeBeasi from comment #7)
> That was fast, thank you! I will keep an eye out for this fix whenever it
> lands in STP.

Thanks for filing with a great reduction, it really helps. It won't be in the immediate next STP release but in the one after that.