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.

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

Throw InvalidStateError when calling play() on a finished, reversed, infinite duration animation #475

Merged
merged 2 commits into from
Jul 27, 2016
Merged

Throw InvalidStateError when calling play() on a finished, reversed, infinite duration animation #475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb9e59d
Do the fixings
alancutter Jul 26, 2016
667eba2
Update expectations for finish.html
alancutter Jul 26, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/animation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,10 @@
this._playbackRate = value;
this._startTime = null;
if (this.playState != 'paused' && this.playState != 'idle') {
this.play();
this._finishedFlag = false;
this._idle = false;
this._ensureAlive();
scope.invalidateEffects();
}
if (oldCurrentTime != null) {
this.currentTime = oldCurrentTime;
Expand All @@ -141,7 +144,15 @@
play: function() {
this._paused = false;
if (this._isFinished || this._idle) {
this._currentTime = this._playbackRate > 0 ? 0 : this._totalDuration;
if (this._playbackRate >= 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it significant that this has gone from > 0 to >= 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It kind of emulates the part in the spec that says "Set animation’s hold time to zero." when we don't really have the concept of a hold time here.

this._currentTime = 0;
} else if (this._totalDuration < Infinity) {
this._currentTime = this._totalDuration;
} else {
throw new DOMException(
'Unable to rewind negative playback rate animation with infinite duration',
'InvalidStateError');
}
this._startTime = null;
}
this._finishedFlag = false;
Expand Down
3 changes: 0 additions & 3 deletions src/web-animations-next-animation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,9 +276,6 @@
this._forEachChild(function(childAnimation) {
childAnimation.playbackRate = value;
});
if (this.playState != 'paused' && this.playState != 'idle') {
this.play();
}
if (oldCurrentTime !== null) {
this.currentTime = oldCurrentTime;
}
Expand Down
12 changes: 2 additions & 10 deletions test/web-platform-tests-expectations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,10 +176,10 @@ module.exports = {
'KeyframeEffectReadOnly is not defined',

'Test finish() while pause-pending with negative playbackRate':
'assert_equals: The start time of a pause-pending animation should be set after calling finish() expected (undefined) undefined but got (number) 100000',
'assert_equals: The play state of a pause-pending animation should become "finished" after finish() is called expected "finished" but got "paused"',

'Test finish() while pause-pending with positive playbackRate':
'assert_approx_equals: The start time of a pause-pending animation should be set after calling finish() expected NaN +/- 0.0005 but got 0',
'assert_equals: The play state of a pause-pending animation should become "finished" after finish() is called expected "finished" but got "paused"',

'Test finish() while paused':
'assert_equals: The play state of a paused animation should become "finished" after finish() is called expected "finished" but got "paused"',
Expand Down Expand Up @@ -232,11 +232,6 @@ module.exports = {
'assert_throws: Expect InvalidStateError exception on calling pause() from idle with a negative playbackRate and infinite-duration animation function "function () {\n"use strict";\n animation.pause(); }" did not throw',
},

'test/web-platform-tests/web-animations/interfaces/Animation/play.html': {
'play() throws when seeking an infinite-duration animation played in reverse':
'assert_throws: Expected InvalidStateError exception on calling play() with a negative playbackRate and infinite-duration animation function "function () {\n"use strict";\n animation.play(); }" did not throw',
},

'test/web-platform-tests/web-animations/interfaces/Animation/playState.html': {
'Animation.playState is \'paused\' after cancelling an animation, seeking it makes it paused':
'assert_equals: After seeking an idle animation, it is effectively paused expected "paused" but got "idle"',
Expand All @@ -260,9 +255,6 @@ module.exports = {
'reverse() when playbackRate > 0 and currentTime < 0':
'assert_equals: reverse() should start playing from the animation effect end if the playbackRate > 0 and the currentTime < 0 expected 100000 but got -200000',

'reverse() when playbackRate > 0 and currentTime < 0 and the target effect end is positive infinity':
'assert_throws: reverse() should throw InvalidStateError if the playbackRate > 0 and the currentTime < 0 and the target effect is positive infinity function "function () {\n"use strict";\n animation.reverse(); }" did not throw',

'reverse() when playbackRate > 0 and currentTime > effect end':
'assert_equals: reverse() should start playing from the animation effect end if the playbackRate > 0 and the currentTime > effect end expected 100000 but got 200000',
},
Expand Down