Skip to content
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 setting the startTime of a play-pending animation to `null` have side effects? #2691

Closed
birtles opened this issue May 22, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@birtles
Copy link
Contributor

commented May 22, 2018

Typically setting the start time of an animation should cancel any pending tasks. This provides applications with a way to synchronously seek/sync animations. Furthermore, by setting the start time to null applications can perform a synchronous pause.

However, if an animation is play-pending its start time will already be unresolved (null).

If the animation is play-pending when script sets the start time to null what should happen?

a. The change is redundant and should be ignored. (Seems intuitive from a JS point of view, redundant changes have no side effects.), or
b. The pending play task is canceled. (Currently speced behavior. Allows applications to perform a synchronous pause even in the play-pending state.)

i.e.

const anim = elem.animate(...);
// anim.pending === true
// anim.startTime === null
// anim.currentTime === 0

anim.startTime = null;
// Is the animation still play-pending here?

(Firefox actually does (a) as an optimization but the spec says we should do (b) and I'm wondering which to fix.)

/cc @stephenmcgruer @flackr @graouts @dadaa

@stephenmcgruer

This comment has been minimized.

Copy link
Contributor

commented May 29, 2018

Apologies for the delay on this. @flackr and I talked it over this morning and option (a) - 'has no effect' makes most sense to us. We don't have a strong opinion on this however. Happy to discuss at the next monthly sync too.

@birtles

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2018

Interesting, I was actually leaning towards (b) since it gives author consistent behavior -- setting startTime to null always does a synchronous pause whether you're pending or not.

@stephenmcgruer

This comment has been minimized.

Copy link
Contributor

commented Jun 1, 2018

In which case something to discuss at our next sync I think! @graouts - any opinion?

@birtles

This comment has been minimized.

Copy link
Contributor Author

commented Jul 25, 2018

Discussed this yesterday with @graouts @stephenmcgruer @flackr and agreed that we should go with (b), i.e. no change to the spec.

Part of the reasoning is that this not only makes it easy to synchronously pause without having to check the pending state, but it is also consistent with the fact that setting the startTime always finishes a pending-play, regardless of whether or not the value you set is null.

Firefox will be updated in Mozilla bug 1478213 where I will also add a test for this.

@birtles birtles closed this Jul 25, 2018

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jul 26, 2018

Bug 1478213 - Don't ignore a redundant change to setting the animatio…
…n start time if we are pending; r=hiro

As discussed here:

  w3c/csswg-drafts#2691

We have a similar check in SetCurrentTime (with the exception that, according to
the spec, this behavior applies to either play OR pause pending, instead of just
pause-pending) so this patch tries to match the comment and format of that
check.

Differential Revision: https://phabricator.services.mozilla.com/D2410

--HG--
extra : rebase_source : 519a976424b021203a369926344c2174662e6156

xeonchen pushed a commit to xeonchen/gecko-cinnabar that referenced this issue Jul 26, 2018

Bug 1478213 - Don't ignore a redundant change to setting the animatio…
…n start time if we are pending; r=hiro

As discussed here:

  w3c/csswg-drafts#2691

We have a similar check in SetCurrentTime (with the exception that, according to
the spec, this behavior applies to either play OR pause pending, instead of just
pause-pending) so this patch tries to match the comment and format of that
check.

Differential Revision: https://phabricator.services.mozilla.com/D2410
@birtles

This comment has been minimized.

Copy link
Contributor Author

commented Jul 26, 2018

A test for this is added in: web-platform-tests/wpt#12195

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.