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

Fix Animation state after calling finish() #477

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix Animation state after calling finish() #477

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
914d7d6
Fix finish() play state issues
alancutter Jul 26, 2016
e3855d2
Rename to limit to match spec
alancutter Jul 27, 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
12 changes: 8 additions & 4 deletions src/animation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,11 +172,15 @@
this._paused = true;
},
finish: function() {
if (this._idle)
return;
this.currentTime = this._playbackRate > 0 ? this._totalDuration : 0;
this._startTime = this._totalDuration - this.currentTime;
var limit = this._playbackRate > 0 ? this._totalDuration : 0;
this.currentTime = limit;
if (this.startTime === null) {
this._startTime = this._timeline.currentTime - limit / this._playbackRate;
}
this._currentTimePending = false;
this._idle = false;
this._paused = false;
this._finished = true;
scope.invalidateEffects();
},
cancel: function() {
Expand Down
16 changes: 8 additions & 8 deletions test/js/animation.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,12 +318,12 @@ suite('animation', function() {
tick(1000);
var a = document.body.animate([], 2000);
a.finish();
assert.equal(a.startTime, 0);
assert.equal(a.startTime, -1000);
Copy link
Contributor

Choose a reason for hiding this comment

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

If I understand correctly, animate() should cause the animation to start playing, in which case startTime would be 1000, and endTime would be calculated from that instead. Is this a difference between animation time and document timeline time?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We called a.finish() which seeks currentTime to 2000 so startTime jumps to -1000 given that timeline.currentTime is 1000.

assert.equal(a.currentTime, 2000);
a.reverse();
a.finish();
assert.equal(a.currentTime, 0);
assert.equal(a.startTime, 2000);
assert.equal(a.startTime, 1000);
tick(2000);
});
test('cancelling clears all effects', function() {
Expand Down Expand Up @@ -494,13 +494,13 @@ suite('animation', function() {
assert.equal(a.startTime, null);
tick(1);
a.finish();
assert.equal(a.playState, 'idle');
assert.equal(a.currentTime, null);
assert.equal(a.startTime, null);
assert.equal(a.playState, 'finished');
assert.equal(a.currentTime, 300);
assert.equal(a.startTime, -299);
tick(2);
assert.equal(a.playState, 'idle');
assert.equal(a.currentTime, null);
assert.equal(a.startTime, null);
assert.equal(a.playState, 'finished');
assert.equal(a.currentTime, 300);
assert.equal(a.startTime, -299);
});
test('Pause after cancel', function() {
var a = document.body.animate([], 300);
Expand Down
9 changes: 3 additions & 6 deletions test/web-platform-tests-expectations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,16 +167,13 @@ module.exports = {
'KeyframeEffectReadOnly is not defined',

'Test finish() while pause-pending with negative playbackRate':
'assert_equals: The play state of a pause-pending animation should become "finished" after finish() is called expected "finished" but got "paused"',
'FLAKY_TEST_RESULT',

'Test finish() while pause-pending with positive playbackRate':
'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"',
'FLAKY_TEST_RESULT',

'Test finish() while play-pending':
'assert_approx_equals: The start time of a play-pending animation should be set after calling finish() expected NaN +/- 0.0005 but got 0',
'FLAKY_TEST_RESULT',

'Test finishing of animation with a current time past the effect end':
'animation.effect.getComputedTiming is not a function',
Expand Down