| Summary: | REGRESSION (262875@main / iOS 17): fill: 'both' not respected with animation | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Liam DeBeasi <ldebeasi> | ||||||||||
| Component: | Animations | Assignee: | Antoine Quint <graouts> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | dino, graouts, graouts, webkit-bug-importer, zalan | ||||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||||
| Version: | Safari 17 | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| See Also: |
https://bugs.webkit.org/show_bug.cgi?id=255338 https://github.com/web-platform-tests/wpt/pull/40736 https://github.com/web-platform-tests/wpt/pull/40947 |
||||||||||||
| Attachments: |
|
||||||||||||
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? This regressed with 262875@main, the fix for bug 255338. 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.
Created attachment 466694 [details]
Test reduction
Created attachment 466695 [details]
Test reduction
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. 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. 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. 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. *** Bug 258134 has been marked as a duplicate of this bug. *** Pull request: https://github.com/WebKit/WebKit/pull/15275 Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/40736 Committed 265498@main (c35a74273103): <https://commits.webkit.org/265498@main> Reviewed commits have been landed. Closing PR #15275 and removing active labels. 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) Let's use <rdar://111407099> to track this issue. 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? 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). Pull request: https://github.com/WebKit/WebKit/pull/15693 Submitted web-platform-tests pull request: https://github.com/web-platform-tests/wpt/pull/40947 Committed 265909@main (6f928cc543c3): <https://commits.webkit.org/265909@main> Reviewed commits have been landed. Closing PR #15693 and removing active labels. @Liam: does that regression break a website or application? 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. |
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.