-
Notifications
You must be signed in to change notification settings - Fork 658
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[web-animations-1] should an animation be ready if its associated target's document lacks a browsing context? #8439
Comments
@graouts How does the The second condition for resolving the ready promise is: If I do: const anim = new Animation(new KeyframeEffect(document.body, { opacity: [0, 1] }, 1000), null);
anim.play();
requestAnimationFrame(() => {
console.log(anim.pending);
}); The animation (and it's |
…sing-context.html is a unique failure https://bugs.webkit.org/show_bug.cgi?id=186494 rdar://problem/41000163 Reviewed by Dean Jackson. The Web Animations spec defines several factors for an animation to be considered "ready" [0], one being that the "user agent has completed any setup required to begin the playback of the animation's associated effect including rendering the first frame of any keyframe effect". While it does not specifically calls out the lack of a browsing context for the associated effect's target's document, there certainly won't be any frames rendered in that specific case. So we do not run pending tasks, the condition for the "ready" promise to resolve, if the associated effect is associated with a document lacking a browsing context. I filed a spec issue [1] to clarify this in the spec, but I believe the spirit of the spec already goes in the direction of this test. [0] https://drafts.csswg.org/web-animations-1/#ready [1] w3c/csswg-drafts#8439 * LayoutTests/imported/w3c/web-platform-tests/web-animations/interfaces/Animatable/animate-no-browsing-context-expected.txt: * Source/WebCore/animation/AnimationEffect.h: (WebCore::AnimationEffect::preventsAnimationReadiness const): * Source/WebCore/animation/KeyframeEffect.cpp: (WebCore::KeyframeEffect::preventsAnimationReadiness const): * Source/WebCore/animation/KeyframeEffect.h: * Source/WebCore/animation/WebAnimation.cpp: (WebCore::WebAnimation::tick): Canonical link: https://commits.webkit.org/260101@main
Thanks! Sorry, I overlooked that. Yes, you're right. I'll have another look next week. Somewhat relatedly, for the code sample I gave, I noticed Chrome gives the wrong result but I don't have a Mac in order to check Safari. Do you know what it does there? |
That code sample produces the same result in Chrome Canary (112.0.5568.0), Firefox Nightly (111.0a1, 2023-02-11) and Safari Technology Preview 163 with the animation marked pending (as expected). |
Oh, you're right. I don't know what the test was I ran but last week I encountered one case where Chrome was resolving the ready Promise while Firefox wasn't. I had a look at the original issue and I think it comes back to this comment from the original commit: https://hg.mozilla.org/mozilla-central/rev/a9805d5b89d741e7532aa469e15354533204fffa#l2.11 That is, no-one seems to resolve style for elements in documents without a browsing context. This comes from bug 1370123. |
There is a WPT test added in web-platform-tests/wpt@90f237a which does the following:
At the time of wring, only Firefox passes that test because both Safari and Chrome resolve the
ready
promise for this element, likely because it is associated with a timeline that is associated with a document with a browsing context.However, the Web Animations spec does not mention the notion of a browsing context once. This comment in the test:
… very likely refers to this spec text about being ready:
But I think this definition is too vague to infer that an animation targeting an element that is in a document without a browsing context should not be in the ready state. For instance, is an element with
display: none
able to render its first frame?I think we need to clarify what impact the availability of a browsing context has on an animation's ready state.
Assigning this to @birtles as the author of this test, also cc'ing @flackr.
The text was updated successfully, but these errors were encountered: