Skip to content

[css-view-transitions] Behavior of calling document.startViewTransition synchronously more than once #11292

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.

Sign up for GitHub

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

Jump to bottom
Closed
bramus opened this issue Nov 28, 2024 · 18 comments · Fixed by #11693
Closed

[css-view-transitions] Behavior of calling document.startViewTransition synchronously more than once #11292

bramus opened this issue Nov 28, 2024 · 18 comments · Fixed by #11693
Labels
css-view-transitions-1 View Transitions; Bugs only Needs Edits

Comments

@bramus
Copy link
Contributor

bramus commented Nov 28, 2024 •
edited
Loading

Demo: https://codepen.io/bramus/pen/gOVyzjL

This demo has two successive calls to document.startViewTransition:

document.querySelectorAll('.item label').forEach(($label) => {
	// @note: we listen on click because that happens *before* a change event
	$label.addEventListener('click', async (e) => {
		if (!document.startViewTransition) return;

		e.preventDefault();
		const $input = $label.querySelector('input');
		
		document.startViewTransition(() => {
			$input.checked = !$input.checked;
		});

		document.startViewTransition(() => {
			document.body.style.padding = `${Math.random() * 10}rem`;
		});
		
	});
});

When you check this in Chrome, the code from both callbacks get executed as part of the View Transition: the faux <details> opens/closes using an animation + it also moves around because the padding of the <body> was changed.

When you check this in Safari, only the code from the second callback gets executed as part of the View Transition: the faux <details> is immediately opened/closed and only moves around because the padding of the <body> was changed.

As per spec, the second call to document.startViewTransition should skip any active view transition, which led me to believe Safari is doing the right thing here.

Checking things with @noamr, he says it’s racy in the spec:

This is actually racy in the spec:

  1. const transition1 = document.startViewTransition(updateCallback1);
  2. document.startViewTransition(updateCallback2):
    2.1 calls transition1.skipTransition() internally
    2.1.1 queue a task to call updateCallback1
    2.2 wait till the next rendering steps
    2.2.1 capture the old state
    2.2.2 queue a task to call updateCallback2

There is no guarantee that 2.2.1 would be after/before 2.1.1. It's a race between the rendering steps and the queued update callback.

I’m not sure if an how we can solve, but I surely would love to see all implementations behave the same by having intstructions for what to do in this case baked into the spec.
@bramus bramus added the css-view-transitions-1 View Transitions; Bugs only label Nov 28, 2024
@noamr
Copy link
Collaborator

noamr commented Nov 28, 2024

The best practice here would be to use updateCallbackDone:

const $input = $label.querySelector('input');

await document.startViewTransition(() => {
	$input.checked = !$input.checked;
}).updateCallbackDone;

document.startViewTransition(() => {
	document.body.style.padding = `${Math.random() * 10}rem`;
});

@bramus
Copy link
Contributor Author

bramus commented Nov 28, 2024 •
edited
Loading

The best practice here would be to use updateCallbackDone

Sure, that would fix that demo so that it’s consistent across browsers. But I also would love to see it work interoperably without that fix.

I’m fine with whatever the outcome is (altough I expected Safari’s behavior to occur; but also see that Chrome’s behavior seems interesting). My main concern is interop here.

@noamr
Copy link
Collaborator

noamr commented Nov 28, 2024

An important detail here is that update callbacks can also be asynchronous and have multiple steps; we can queue them in such a way that they would start in the order of their respective view transitions, but without awaiting it would be constrained to a subset of those callbacks.

@noamr
Copy link
Collaborator

noamr commented Nov 28, 2024

@emilio @jakearchibald @nt1m any takes on this?
I think relying on updateCallbackDone is the right thing to do here, but I'm open to exploring queuing the update callbacks in order received rather than race between the successful one and failed ones

@jakearchibald
Copy link
Contributor

The Safari implementation is definitely wrong, as it violates the intent that transitions are an enhancement.

That means a failure to create a visual transition, which can happen due to misconfiguration or device constraints, will not prevent the developer’s UpdateCallback being called, even if it’s known in advance that the transition animations cannot happen.

For example, if the developer calls skipTransition() at the start of the view transition lifecycle, the steps relating to the animated transition, such as creating the view transition tree, will not happen. However, the UpdateCallback will still be called. It’s only the visual transition that’s skipped, not the underlying state change.

It'd be nice to remove the race condition somehow though.

@noamr
Copy link
Collaborator

noamr commented Dec 3, 2024

The Safari implementation is definitely wrong, as it violates the intent that transitions are an enhancement.

Ah right, I was reading it as Safari was executing them in order. Not calling the update callback at all is definitely wrong and a spec violation. Surprised that we don't have WPTs for this.

It'd be nice to remove the race condition somehow though.

Happy to resolve on something and spec it. It's not complex to fix.

@noamr noamr added the Agenda+ label Dec 3, 2024
@noamr
Copy link
Collaborator

noamr commented Dec 3, 2024

Proposing the following resolution: the update callbacks would be maintained in an internal queue rather than separately on the event loop, and when "calling the update callback" all of them would be invoked.

@bramus
Copy link
Contributor Author

bramus commented Dec 3, 2024

The Safari implementation is definitely wrong, as it violates the intent that transitions are an enhancement.

That means a failure to create a visual transition, which can happen due to misconfiguration or device constraints, will not prevent the developer’s UpdateCallback being called, even if it’s known in advance that the transition animations cannot happen.
For example, if the developer calls skipTransition() at the start of the view transition lifecycle, the steps relating to the animated transition, such as creating the view transition tree, will not happen. However, the UpdateCallback will still be called. It’s only the visual transition that’s skipped, not the underlying state change.

But Safari is executing the first callback (the one that changes .checked) just fine. What is different between Chrome and Safari is that Safari seems to flush the queue of previously registered callbacks before taking the snapshots, whereas Chrome executes all previously registered callbacks as part of the resulting transition.

One could say Chrome is in the wrong here, because it changes the .checked state as part of the View Transition, which goes against “It’s only the visual transition that’s skipped, not the underlying state change.” – Chrome does not skip the visual transition.

@jakearchibald
Copy link
Contributor

When you check this in Safari, only the code from the second callback gets executed as part of the View Transition

Ah, my bad, I interpreted the above incorrectly.

What is different between Chrome and Safari is that Safari seems to flush the queue of previously registered callbacks before taking the snapshots, whereas Chrome executes all previously registered callbacks as part of the resulting transition.

Yeah, I agree that's nice.

@bramus
Copy link
Contributor Author

bramus commented Dec 3, 2024

What is different between Chrome and Safari is that Safari seems to flush the queue of previously registered callbacks before taking the snapshots, whereas Chrome executes all previously registered callbacks as part of the resulting transition.

Yeah, I agree that's nice.

The Safari behavior?

@jakearchibald
Copy link
Contributor

jakearchibald commented Dec 3, 2024 •
edited
Loading

Yes. Sorry, that wasn't clear. The Safari behaviour where it makes the previous transition do all its cancelation behaviours before the new transition does any of its capturing sounds good.

@noamr
Copy link
Collaborator

noamr commented Jan 21, 2025

Let's tag this for async resolution? @astearns

@bramus
Copy link
Contributor Author

bramus commented Jan 21, 2025

PROPOSED RESOLUTION: When starting a new view transition, have the previous active transition do all its cancelation behaviours before the new transition does any of its capturing.

@astearns astearns added the Async Resolution: Proposed Candidate for auto-resolve with stated time limit label Jan 21, 2025
@astearns
Copy link
Member

I‘d like to see some WebKit confirmation on this proposed resolution.

@noamr
Copy link
Collaborator

noamr commented Jan 21, 2025 •
edited
Loading

I‘d like to see some WebKit confirmation on this proposed resolution.

The resolution is to adopt the current webkit behavior
Happy to get confirmation from @nt1m

@nt1m
Copy link
Member

nt1m commented Jan 21, 2025

Sounds good

@astearns astearns moved this to FTF agenda items in CSSWG January 2025 meeting Jan 22, 2025
@astearns astearns moved this from FTF agenda items to Regular agenda items in CSSWG January 2025 meeting Jan 22, 2025
@astearns
Copy link
Member

astearns commented Feb 4, 2025

The CSSWG will automatically accept this resolution one week from now if no objections are raised here. Anyone can add an emoji to this comment to express support. If you do not support this resolution, please add a new comment.

Proposed Resolution: When starting a new view transition, have the previous active transition do all its cancelation behaviours before the new transition does any of its capturing.

@astearns astearns added Async Resolution: Call For Consensus Resolution will be called after time limit expires and removed Async Resolution: Proposed Candidate for auto-resolve with stated time limit labels Feb 4, 2025
@astearns
Copy link
Member

RESOLVED: When starting a new view transition, have the previous active transition do all its cancelation behaviours before the new transition does any of its capturing.

@astearns astearns added Needs Edits and removed Agenda+ Async Resolution: Call For Consensus Resolution will be called after time limit expires labels Feb 10, 2025
@mozilla-apprentice mozilla-apprentice mentioned this issue Feb 10, 2025
Closed
@chromium-helper chromium-helper mentioned this issue Feb 11, 2025
Closed
noamr added a commit to noamr/csswg-drafts that referenced this issue Feb 11, 2025
@noamr
[css-view-transitions-1] Always perform all the update callbacks befo…
18d50d7 
…re capturing the new state.

Closes w3c#11292
@noamr noamr mentioned this issue Feb 11, 2025
Merged
@noamr noamr closed this as completed in cc56543 Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
css-view-transitions-1 View Transitions; Bugs only Needs Edits
Projects
CSSWG January 2025 meeting
Status: Regular agenda items
Milestone
No milestone
5 participants
@noamr @jakearchibald @bramus @astearns @nt1m