1150807 - Implement Animation.cancel()

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Comment 1

[Brian Birtles (:birtles)]

Attachment: WIP patch

Comment 2

[Brian Birtles (:birtles)]

This patch is still missing a few tests (as listed at the end of css-animations/test_animation-cancel.html).

Currently one of the tests in css-animations/test_animation-playstate.html is failing because it's checking that after we set the currentTime while idle we transition to the paused state. By the letter of the spec we should actually start playing from currentTime (i.e. transition to the running state) but I'm not sure if that's actually the desired behaviour in this case or if we should update the spec. It seems like it makes more sense only to update the start time if it's already set.

Comment 3

[Brian Birtles (:birtles)]

(In reply to Brian Birtles (:birtles) from comment #2)
> Currently one of the tests in css-animations/test_animation-playstate.html
> is failing because it's checking that after we set the currentTime while
> idle we transition to the paused state. By the letter of the spec we should
> actually start playing from currentTime (i.e. transition to the running
> state) but I'm not sure if that's actually the desired behaviour in this
> case or if we should update the spec. It seems like it makes more sense only
> to update the start time if it's already set.

I'm going to change the spec:

  https://lists.w3.org/Archives/Public/public-fx/2015AprJun/0027.html

Comment 4

[Brian Birtles (:birtles)]

The main problem I'm facing now is what to call the different Cancel methods. For each of these Animation methods we have four different ways of calling them:

 a. Call from script -- needs to reset a bunch of state in style so that we retrigger layer transactions etc.
 b. Call from script on a CSS animation/transition -- needs to reset state in style AND needs to flush style before doing anything
 c. Call from style -- doesn't need to reset any of that state (since we're in style already) but *may* need to behave differently (specifically for pausing/playing we behave differently when being called from style)
 d. Call from elsewhere in Gecko -- needs to reset a bunch of state in style

Typically we've named these as follows:

 a. XXXFromJS()
 b. XXXFromJS() (but on the CSSAnimation/CSSTransition-specific override)
 c. XXXFromStyle()
 d. XXX()

I wonder about (d). I wonder if XXX() should just do the operation but not reset state in style and make that the responsibility of the caller. Currently there *are* no other callers in Gecko at the moment other than style.

It might be that in bug 1151731 we find a better way of expressing this so I'll just leave it as-is for now.

Comment 5

[Brian Birtles (:birtles)]

Attachment: part 1 - Fix SilentlySetCurrentTime to not set the start time when it is unresolved

The reasoning for this change is described here:

  https://lists.w3.org/Archives/Public/public-fx/2015AprJun/0027.html

Furthermore, subsequent tests introduced in this bug for the animation playState
depend on this change.

Comment 6

[Brian Birtles (:birtles)]

Attachment: part 2 - Expose Animation::Cancel in WebIDL

Comment 7

[Brian Birtles (:birtles)]

Attachment: part 3 - Call PostUpdate from Cancel

This patch makes Cancel() call PostUpdate which clobbers certain state in style
so that animated style is correctly flushed when an animation is cancelled.

The main difficulty with this is that we *don't* want to call this when we're
cancelling an animation as a result of a style update or else we'll trigger
needless work. The pattern elsewhere has been to define a *FromStyle() method
for this case (e.g. CSSAnimation::PlayFromStyle, PauseFromStyle). This isn't
ideal because there's always the danger we will forget to call the appropriate
*FromStyle method. It is, however, consistent. Hopefully in bug 1151731 we'll
find a better way of expressing this.

Comment 8

[Brian Birtles (:birtles)]

Attachment: part 4 - Don't play/pause an idle animation when animation-play-state changes

This isn't spec'ed anywhere (since the whole Web Animations API <-> CSS
interaction isn't spec'ed yet) but it seems that changing animation-play-state
should not restart an idle animation.

If an author calls Cancel() on an animation then that animation should continue
to be idle until they call Play()/Pause() from the API. Cancelling an animation
and hanging on to it is a purely API-only feature and hence it's reasonable that
restoring it from this state is also an API-only feature.

One can imagine use-cases such as polyfilling where script wants to remove any
CSS Animations/Transitions run by the browser and replace them with something
else entirely. In that case, the script can call Cancel() on the animation and
be sure that the animation is going to stay out of the way even if something
else tweaks the animation-play-state.

Comment 9

[Brian Birtles (:birtles)]

Attachment: part 5 - Tests for Animation.cancel()

Comment 10

[Brian Birtles (:birtles)]

(In reply to Brian Birtles (:birtles) from comment #3)
> I'm going to change the spec:
> 
>   https://lists.w3.org/Archives/Public/public-fx/2015AprJun/0027.html

https://github.com/w3c/web-animations/commit/b90463ac5bd44adf3e5d42ae4895b41b99a17c7d

Comment 11

[Brian Birtles (:birtles)]

Try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=2a4c6438009b

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/076fabe36fd3
https://hg.mozilla.org/integration/mozilla-inbound/rev/72571965f94d
https://hg.mozilla.org/integration/mozilla-inbound/rev/12421d0f1bb3
https://hg.mozilla.org/integration/mozilla-inbound/rev/064e704dfa02
https://hg.mozilla.org/integration/mozilla-inbound/rev/f66f164d3160

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/076fabe36fd3
https://hg.mozilla.org/mozilla-central/rev/72571965f94d
https://hg.mozilla.org/mozilla-central/rev/12421d0f1bb3
https://hg.mozilla.org/mozilla-central/rev/064e704dfa02
https://hg.mozilla.org/mozilla-central/rev/f66f164d3160