Bug 257861 – REGRESSION (262875@main / iOS 17): fill: 'both' not respected with animation

• Component: Animations
• Status: RESOLVED FIXED

[Liam DeBeasi]

Created attachment 466639 [details]
Code reproduction

Using fill: 'both' causes elements to disappear after their animation ends when using iOS 17 beta 1.

Steps to reproduce:

1. Open code reproduction on an iOS 17 device.
2. Tap "Open Overlay". Observe that the overlay opens.
3. Tap "Close Overlay". Observe that the overlay closes.
4. Tap "Open Overlay" again. Observe that the overlay opens and then disappears.

Expected Behavior:

I expect the overlay to remain visible every time I present it.

Actual Behavior:

The overlay disappears after subsequent presentations.

Other Information:

- This does not reproduce on Chrome 113 or Firefox 113.
- This does not reproduce on iOS 16. This appears to be a regression on iOS 17.

[Radar WebKit Bug Importer]

<rdar://problem/110559121>

[Antoine Quint]

I'll look into this. In the meantime, do you know whether this only apply to opacity or transform, which are accelerated properties, or also other properties such as width/height/top/left which are not accelerated?

[Antoine Quint]

This regressed with 262875@main, the fix for bug 255338.

[Liam DeBeasi]

Created attachment 466664 [details]
Non-accelerated demo (working as expected)

This behavior does not reproduce with non-accelerated properties. I attached a reproduction that shows the expected behavior when animating height.

[zalan]

Created attachment 466694 [details]
Test reduction

[zalan]

Created attachment 466695 [details]
Test reduction

[Antoine Quint]

Not clear to me what is going wrong just yet, as far as I can tell at the GraphicsLayerCA level we are left with a single animation which is the last added animation in the test reduction – which is correct since its composite order is higher and the first animation is "replaced" per https://drafts.csswg.org/web-animations-1/#removing-replaced-animations. So I don't yet understand what the presentation value is the final value of the first animation.

[Antoine Quint]

Actually, we end up calling GraphicsLayerCA::removeAnimation() with the last animation and not the former, so the final stack for GraphicsLayerCA is incorrect. So the presentation value makes sense, it's the content of the GraphicsLayerCA animation stack which is incorrect.

[zalan]

<rdar://109221339>

[Antoine Quint]

The accelerated actions we issue are correct, the first animation is played and stopped when it stops producing an interpolating value, at which points the second animation is played and stopped when itself stops producing an interpolating value.

Somehow, we fail to remove the first animation, or add it back, it's unclear yet.

[Antoine Quint]

Oh, I think I see what's wrong, in KeyframeEffect::applyPendingAcceleratedActionsOrUpdateTimingProperties() we append AcceleratedAction::UpdateProperties but fail to record it as the last recorded action. This might just be it, there's a state disconnect here.

[zalan]

*** Bug 258134 has been marked as a duplicate of this bug. ***

[Antoine Quint]

Pull request: https://github.com/WebKit/WebKit/pull/15275

[Antoine Quint]

Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/40736

[EWS]

Committed 265498@main (c35a74273103): <https://commits.webkit.org/265498@main>

Reviewed commits have been landed. Closing PR #15275 and removing active labels.

[zalan]

While the fix addresses https://bug-257861-attachments.webkit.org/attachment.cgi?id=466695 it does not fix the original test case (and the duped bugzilla fails too)

[zalan]

Let's use <rdar://111407099> to track this issue.

[Antoine Quint]

Sorry for not being as thorough as I should have been. I'll take a look at the Code reproduction listed here.

Should we also de-dupe bug 258134 until we know for a fact that the complete fix for this bug actually addresses that bug as well?

[Antoine Quint]

I have a good understand of the issue here, we'll need to send down the relative sorting order of accelerated effects to GraphicsLayerCA which should be easy enough. I'm hoping to get this done in the coming week (starting July 3).

[Antoine Quint]

Pull request: https://github.com/WebKit/WebKit/pull/15693

[Antoine Quint]

Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/40947

[EWS]

Committed 265909@main (6f928cc543c3): <https://commits.webkit.org/265909@main>

Reviewed commits have been landed. Closing PR #15693 and removing active labels.

[Antoine Quint]

@Liam: does that regression break a website or application?

[Liam DeBeasi]

This breaks Ionic Framework's sheet modal component: https://docs-demo.ionic.io/component/modal

Open on an iOS 17 device and tap "Open Sheet Modal". The animation should break after you try to swipe.

Thanks for the fix! I'll give this a try when it lands in an upcoming beta.