-
Notifications
You must be signed in to change notification settings - Fork 711
[css-view-transitions-1] Swap the order between setting the phase and calling the update callback #10826
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
Conversation
… calling the update callback. This fixes an issue where skipping the transition from within thet update callback would call the update callback twice. Closes w3c#10822
1. If |transition|'s [=ViewTransition/phase=] is not "`done`", then set |transition|'s [=ViewTransition/phase=] to "`update-callback-called`". | ||
|
||
1. Otherwise, set |callbackPromise| to the result of [=/invoking=] |transition|'s [=ViewTransition/update callback=]. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is "otherwise" here correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops, fixed
https://bugs.webkit.org/show_bug.cgi?id=279087 rdar://135226640 Reviewed by NOBODY (OOPS!). This is currently a spec bug, see: - w3c/csswg-drafts#10826 - w3c/csswg-drafts#10822 * LayoutTests/imported/w3c/web-platform-tests/css/css-view-transitions/update-callback-called-once-expected.xht: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-view-transitions/update-callback-called-once.html: Added. * Source/WebCore/dom/ViewTransition.cpp: (WebCore::ViewTransition::callUpdateCallback):
… times https://bugs.webkit.org/show_bug.cgi?id=279087 rdar://135226640 Reviewed by NOBODY (OOPS!). This is currently a spec bug, see: - w3c/csswg-drafts#10826 - w3c/csswg-drafts#10822 * LayoutTests/imported/w3c/web-platform-tests/css/css-view-transitions/update-callback-called-once-expected.xht: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-view-transitions/update-callback-called-once.html: Added. * Source/WebCore/dom/ViewTransition.cpp: (WebCore::ViewTransition::callUpdateCallback):
… times https://bugs.webkit.org/show_bug.cgi?id=279087 rdar://135226640 Reviewed by NOBODY (OOPS!). This is currently a spec bug, see: - w3c/csswg-drafts#10826 - w3c/csswg-drafts#10822 * LayoutTests/imported/w3c/web-platform-tests/css/css-view-transitions/update-callback-called-once-expected.xht: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-view-transitions/update-callback-called-once.html: Added. * Source/WebCore/dom/ViewTransition.cpp: (WebCore::ViewTransition::callUpdateCallback):
@@ -1600,14 +1600,14 @@ urlPrefix: https://wicg.github.io/navigation-api/; type: interface; | |||
|
|||
1. Let |callbackPromise| be null. | |||
|
|||
1. If |transition|'s [=ViewTransition/phase=] is not "`done`", then set |transition|'s [=ViewTransition/phase=] to "`update-callback-called`". |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May as well move it further up to keep the code using callbackPromise
together?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, done
… times https://bugs.webkit.org/show_bug.cgi?id=279087 rdar://135226640 Reviewed by NOBODY (OOPS!). This is currently a spec bug, see: - w3c/csswg-drafts#10826 - w3c/csswg-drafts#10822 * LayoutTests/imported/w3c/web-platform-tests/css/css-view-transitions/update-callback-called-once-expected.xht: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-view-transitions/update-callback-called-once.html: Added. * Source/WebCore/dom/ViewTransition.cpp: (WebCore::ViewTransition::callUpdateCallback):
… times https://bugs.webkit.org/show_bug.cgi?id=279087 rdar://135226640 Reviewed by NOBODY (OOPS!). This is currently a spec bug, see: - w3c/csswg-drafts#10826 - w3c/csswg-drafts#10822 * LayoutTests/imported/w3c/web-platform-tests/css/css-view-transitions/update-callback-called-once-expected.xht: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-view-transitions/update-callback-called-once.html: Added. * Source/WebCore/dom/ViewTransition.cpp: (WebCore::ViewTransition::callUpdateCallback):
… times https://bugs.webkit.org/show_bug.cgi?id=279087 rdar://135226640 Reviewed by Matt Woodrow. This is currently a spec bug, see: - w3c/csswg-drafts#10826 - w3c/csswg-drafts#10822 * LayoutTests/imported/w3c/web-platform-tests/css/css-view-transitions/update-callback-called-once-expected.xht: Added. * LayoutTests/imported/w3c/web-platform-tests/css/css-view-transitions/update-callback-called-once.html: Added. * Source/WebCore/dom/ViewTransition.cpp: (WebCore::ViewTransition::callUpdateCallback): Canonical link: https://commits.webkit.org/283167@main
This fixes an issue where skipping the transition from within thet update callback would call the update callback twice.
Closes #10822