Bug 201048 – Accelerated animations freeze on render tree rebuild

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

[Liam DeBeasi]

When animating an element within a shadow root in a custom element, layout invalidations via `Node.insertBefore` cause the animation to freeze. This does not happen when using a custom element without a shadow root. I am able to reproduce this in Safari 12 on both iOS and macOS.

Code reproduction: https://codepen.io/liamdebeasi/pen/YzKNGzW

Steps to reproduce:
1. Go to the link above.
2. You should see a box that is rotating, as well as a button.
3. Click the "Click Me" button.
4. Notice that the box stops rotating.
5. Subsequent clicks of the "Click Me" button will cause the animation to render for a frame and then stop again.

Hovering over the ".square" element in Web Inspector will reveal that the animation is still playing. The "highlight" that appears over the element in the DOM tree should update every time you hover over it. Switching to another tab and then back to the original tab somehow fixes the issue.

This bug does not happen when using a custom element without a shadow root, as seen in this code example: https://codepen.io/liamdebeasi/pen/xxKgErw (follow the same steps as before)

As an additional reference, Angular can sometimes call `Node.insertBefore` when running change detection. This can cause animations in a user's application to break when using the shadow DOM. See https://github.com/ionic-team/ionic/issues/17202 for more info)


Web Animations API Note:

The same bug affects Safari's implementation of the Web Animations API (tested using Safari Technology Preview 90). With just "Web Animations" enabled in "Experiments", the tab switching trick mentioned above does not work. When also enabling "CSS Animations via Web Animations" the tab switching trick works.

Web Animations Code Reproduction: https://codepen.io/liamdebeasi/pen/LYPxbed

[Radar WebKit Bug Importer]

<rdar://problem/54612621>

[Niklas Merz]

I can add that this seems to be a regression. The same transitions work more smoothly on iOS 12.4 than on iOS 13.

[Niklas Merz]

Created attachment 386084 [details]
Sample app shown on iOS 12

[Niklas Merz]

Created attachment 386085 [details]
Sample app on iOS 13

I just built a small sample with Ionic that shows why this bug is really bad for the UX. The same app runs on iOS 12 better and the animation is less stuttering. 

That's why I think this might be a regression.

[Niklas Merz]

Sorry for the ping. This is really a huge UX flaw for page transitions in Ionic apps with certain pages.

Can we do anything to get this fixed? Thanks

[Ryosuke Niwa]

(In reply to Niklas Merz from comment #5)
> Sorry for the ping. This is really a huge UX flaw for page transitions in
> Ionic apps with certain pages.

I appreciate that you provided this detail. It allows us to priorities this bug better.

> Can we do anything to get this fixed?

It would be ideal if we had a minimum reproduction that doesn't rely on custom elements instead of https://codepen.io/liamdebeasi/pen/YzKNGzW but it's not a big deal. I can easily make a reduction myself.

FWIW, I don't think I can work on it anytime soon due to other commitments. Having said that, it's probably not that hard to diagnose this issue even as the first WebKit bug for someone so if you can find someone who can work on this that might work too.

[Liam DeBeasi]

(In reply to Ryosuke Niwa from comment #6)
> It would be ideal if we had a minimum reproduction that doesn't rely on
> custom elements instead of https://codepen.io/liamdebeasi/pen/YzKNGzW but
> it's not a big deal. I can easily make a reduction myself.
> 
> FWIW, I don't think I can work on it anytime soon due to other commitments.
> Having said that, it's probably not that hard to diagnose this issue even as
> the first WebKit bug for someone so if you can find someone who can work on
> this that might work too.

I can try and diagnose it a bit, though I am not very familiar with the WebKit code base.

What kind of reproduction would be helpful here? The issue only happens with custom elements/Shadow DOM, so I am not sure that a reproduction without custom elements is possible here.

[Tim Guan-tin Chien [:timdream]]

Maybe the test case in bug 204805 can be helpful.

[Antti Koivisto]

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

[Antti Koivisto]

204805 has working WIP patch and test case without shadow trees.

[Antti Koivisto]

Created attachment 389514 [details]
patch

[Antoine Quint]

Comment on attachment 389514 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=389514&action=review

> LayoutTests/webanimations/accelerated-animation-renderer-change.html:48
> +    await new Promise(resolve => setTimeout(resolve, 50));

It would be preferable to get a reference to the animation using element.getAnimations() and then wait for its ready promise to resolve and then wait one frame using requestAnimationFrame(), that way there is no setTimeout() involved.

> LayoutTests/webanimations/accelerated-animation-renderer-change.html:54
> +    await new Promise(resolve => setTimeout(resolve, 500));

Ideally we could use requestAnimationFrame here too, possibly with 2 frames to guarantee the animation would have ticked a couple of times.

[Antti Koivisto]

Created attachment 389525 [details]
patch

[Antti Koivisto]

Created attachment 389527 [details]
patch

[WebKit Commit Bot]

Comment on attachment 389527 [details]
patch

Clearing flags on attachment: 389527

Committed r255663: <https://trac.webkit.org/changeset/255663>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Antti Koivisto]

Adding slot based test in https://bugs.webkit.org/show_bug.cgi?id=207359

[Niklas Merz]

I just did a quick test with Safari Technology Preview on macOS and the latest iOS 13.4 Developer Beta 2. Looks like the issue is fixed and the patch has landed in the beta versions!

The codepen works now and the page transitions in our app are much smoother!

Thank you very much for fixing this.

[Ryosuke Niwa]

(In reply to Niklas Merz from comment #18)
> I just did a quick test with Safari Technology Preview on macOS and the
> latest iOS 13.4 Developer Beta 2. Looks like the issue is fixed and the
> patch has landed in the beta versions!
> 
> The codepen works now and the page transitions in our app are much smoother!

Thanks for verifying that the bug is fixed!