Closed
Bug 1244633
Opened 8 years ago
Closed 8 years ago
implement AnimationEffectTiming delay
Categories
(Core :: DOM: Animation, defect)
Core
DOM: Animation
Tracking
()
RESOLVED
FIXED
mozilla48
Tracking | Status | |
---|---|---|
firefox48 | --- | fixed |
People
(Reporter: motozawa, Assigned: daisuke)
References
()
Details
Attachments
(2 files)
No description provided.
Assignee | ||
Updated•8 years ago
|
Assignee: motozawa → daisuke
Assignee | ||
Comment 1•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=dae4b2986bc2
Assignee | ||
Comment 2•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/42705/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42705/
Attachment #8735324 -
Flags: review?(bbirtles)
Attachment #8735325 -
Flags: review?(bbirtles)
Assignee | ||
Comment 3•8 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/42707/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/42707/
Comment 4•8 years ago
|
||
Comment on attachment 8735324 [details] MozReview Request: Bug 1244633 - Part 1: implement AnimationEffectTiming delay. r=birtles https://reviewboard.mozilla.org/r/42705/#review39401
Attachment #8735324 -
Flags: review?(bbirtles) → review+
Comment 5•8 years ago
|
||
Comment on attachment 8735325 [details] MozReview Request: Bug 1244633 - Part 2: append tests for delay. r=birtles https://reviewboard.mozilla.org/r/42707/#review39403 This is good but needs animation observer tests that: * We create a change record when changing the delay * We *don't* create a change record when setting the delay to its current value (including when the current value is not zero--e.g. by specifying a different value in the constructor) Please add those tests before landing. ::: testing/web-platform/tests/web-animations/animation-effect-timing/delay.html:4 (Diff revision 1) > +<!DOCTYPE html> > +<meta charset=utf-8> > +<title>delay tests</title> > +<link rel="help" href="https://w3c.github.io/web-animations/#dom-animationeffecttimingreadonly-delay"> Wrong URL I think---this is not for the readonly interface, right? ::: testing/web-platform/tests/web-animations/animation-effect-timing/delay.html:33 (Diff revision 1) > +test(function(t) { > + var div = createDiv(t); > + var anim = div.animate({ opacity: [ 0, 1 ] }, 100); > + assert_throws({name: "TypeError"}, function() { > + anim.effect.timing.delay = Infinity; > + }, 'we can not assign Infinity to timing.delay'); > +}, 'set delay Infinity'); > + > +test(function(t) { > + var div = createDiv(t); > + var anim = div.animate({ opacity: [ 0, 1 ] }, 100); > + assert_throws({name: "TypeError"}, function() { > + anim.effect.timing.delay = -Infinity; > + }, 'we can not assign negative Infinity to timing.delay'); > +}, 'set delay negative Infinity'); > + > +test(function(t) { > + var div = createDiv(t); > + var anim = div.animate({ opacity: [ 0, 1 ] }, 100); > + assert_throws({name: "TypeError"}, function() { > + anim.effect.timing.delay = NaN; > + }, 'we can not assign NaN to timing.delay'); > +}, 'set delay NaN'); I don't think we need these three tests. WebIDL should do it for us. ::: testing/web-platform/tests/web-animations/animation-effect-timing/delay.html:63 (Diff revision 1) > + var div = createDiv(t); > + var anim = div.animate({ opacity: [ 0, 1 ] }, 100); > + anim.effect.timing.delay = 100; > + assert_equals(anim.effect.getComputedTiming().progress, null); > + assert_equals(anim.effect.getComputedTiming().currentIteration, null); > +}, 'Test that changing the delay to positive'); 'Test adding a positive delay to an animation without a backwards fill makes it no longer active' ::: testing/web-platform/tests/web-animations/animation-effect-timing/delay.html:72 (Diff revision 1) > + var anim = div.animate({ opacity: [ 0, 1 ] }, > + { fill: 'both', > + duration: 100 }); > + anim.effect.timing.delay = -50; > + assert_equals(anim.effect.getComputedTiming().progress, 0.5); > + assert_equals(anim.effect.getComputedTiming().currentIteration, 0); Not sure we need the check for currentIteration here? ::: testing/web-platform/tests/web-animations/animation-effect-timing/delay.html:74 (Diff revision 1) > + duration: 100 }); > + anim.effect.timing.delay = -50; > + assert_equals(anim.effect.getComputedTiming().progress, 0.5); > + assert_equals(anim.effect.getComputedTiming().currentIteration, 0); > +}, 'Test that changing the delay to ' + > + 'negative value that is half of duration'); 'Test seeking an animation by setting a negative delay' ::: testing/web-platform/tests/web-animations/animation-effect-timing/delay.html:84 (Diff revision 1) > + { fill: 'both', > + duration: 100 }); > + anim.effect.timing.delay = -100; > + assert_equals(anim.effect.getComputedTiming().progress, 1); > + assert_equals(anim.effect.getComputedTiming().currentIteration, 0); > +}, 'Test that changing the delay to ' + 'Test finishing an animation using a large negative delay'
Attachment #8735325 -
Flags: review?(bbirtles) → review+
Assignee | ||
Comment 6•8 years ago
|
||
Comment on attachment 8735324 [details] MozReview Request: Bug 1244633 - Part 1: implement AnimationEffectTiming delay. r=birtles Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42705/diff/1-2/
Attachment #8735324 -
Attachment description: MozReview Request: Bug 1244633 - Part 1: implement AnimationEffectTiming delay. r?birtles → MozReview Request: Bug 1244633 - Part 1: implement AnimationEffectTiming delay. r=birtles
Attachment #8735325 -
Attachment description: MozReview Request: Bug 1244633 - Part 2: append tests for delay. r?birtles → MozReview Request: Bug 1244633 - Part 2: append tests for delay. r=birtles
Assignee | ||
Comment 7•8 years ago
|
||
Comment on attachment 8735325 [details] MozReview Request: Bug 1244633 - Part 2: append tests for delay. r=birtles Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42707/diff/1-2/
Assignee | ||
Comment 8•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=ce30dc427a6a
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 9•8 years ago
|
||
hi part 2 failed to apply: 1 out of 1 hunks FAILED -- saving rejects to file dom/animation/test/chrome/test_animation_observers.html.rej
Flags: needinfo?(daisuke)
Keywords: checkin-needed
Assignee | ||
Comment 10•8 years ago
|
||
(In reply to Carsten Book [:Tomcat] from comment #9) > hi part 2 failed to apply: > > 1 out of 1 hunks FAILED -- saving rejects to file > dom/animation/test/chrome/test_animation_observers.html.rej OK! Thanks!
Flags: needinfo?(daisuke)
Assignee | ||
Comment 11•8 years ago
|
||
https://treeherder.mozilla.org/#/jobs?repo=try&revision=012cf638fb85
Assignee | ||
Comment 12•8 years ago
|
||
Comment on attachment 8735324 [details] MozReview Request: Bug 1244633 - Part 1: implement AnimationEffectTiming delay. r=birtles Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42705/diff/2-3/
Assignee | ||
Comment 13•8 years ago
|
||
Comment on attachment 8735325 [details] MozReview Request: Bug 1244633 - Part 2: append tests for delay. r=birtles Review request updated; see interdiff: https://reviewboard.mozilla.org/r/42707/diff/2-3/
Assignee | ||
Comment 14•8 years ago
|
||
Fix bitrot.
Comment 15•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b52399acb450 https://hg.mozilla.org/integration/mozilla-inbound/rev/2effa07d02e1
Comment 16•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/b52399acb450 https://hg.mozilla.org/mozilla-central/rev/2effa07d02e1
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox48:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla48
You need to log in
before you can comment on or make changes to this bug.
Description
•