Closed
Bug 1478213
Opened 6 years ago
Closed 6 years ago
Setting `startTime = null` on a play-pending Animation should do a synchronous pause
Categories
(Core :: DOM: Animation, enhancement, P3)
Core
DOM: Animation
Tracking
()
RESOLVED
FIXED
mozilla63
| Tracking | Status | |
|---|---|---|
| firefox63 | --- | fixed |
People
(Reporter: birtles, Assigned: birtles)
References
Details
Attachments
(3 files)
i.e. we should drop the optimization where we skip the change if the value to set matches the current value: https://searchfox.org/mozilla-central/rev/bdfd20ef30d521b57d5b6feeda71325e8b4cad66/dom/animation/Animation.cpp#237-239 See: https://github.com/w3c/csswg-drafts/issues/2691
|
Assignee | |
Updated•6 years ago
|
Priority: -- → P3
|
|
Assignee | |
Comment 1•6 years ago
|
||
|
|
Assignee | |
Comment 2•6 years ago
|
||
As discussed here: https://github.com/w3c/csswg-drafts/issues/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.
|
|
Assignee | |
Updated•6 years ago
|
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
|
||
Comment 3•6 years ago
|
||
Comment on attachment 8995068 [details] Bug 1478213 - Add test that fails if we ignore a seemingly redundant setting of startTime to null; r=hiro Hiroyuki Ikezoe (:hiro) has approved the revision. https://phabricator.services.mozilla.com/D2409
Attachment #8995068 -
Flags: review+
|
|
Assignee | |
Comment 4•6 years ago
|
||
The optimization this code was working around has now been fixed.
|
|
||
Comment 5•6 years ago
|
||
Comment on attachment 8995069 [details] Bug 1478213 - Don't ignore a redundant change to setting the animation start time if we are pending; r=hiro Hiroyuki Ikezoe (:hiro) has approved the revision. https://phabricator.services.mozilla.com/D2410
Attachment #8995069 -
Flags: review+
|
|
||
Comment 6•6 years ago
|
||
Comment on attachment 8995070 [details] Bug 1478213 - Drop the workaround in DevTools for setting an animation's startTime to something non-null before setting it to null; r=daisuke Daisuke Akatsuka (:daisuke) has approved the revision. https://phabricator.services.mozilla.com/D2419
Attachment #8995070 -
Flags: review+
|
|
Assignee | |
Comment 7•6 years ago
|
||
Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=319c5baee88d73ad84d5acd1b37bd9cd184dd368
|
|
Assignee | |
Comment 8•6 years ago
|
||
I discussed with glob and it turns out there is a bug in arc-review where it was failing to set the base revision correctly. As a result, each of the patches in Phabricator is actually a squashed diff between that patch and the base (central). Nevertheless, I think the reviews are still valid so I'll just land this on inbound.
Pushed by bbirtles@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/420ef12d7971 Add test that fails if we ignore a seemingly redundant setting of startTime to null; r=hiro https://hg.mozilla.org/integration/mozilla-inbound/rev/e9fd70739814 Don't ignore a redundant change to setting the animation start time if we are pending; r=hiro https://hg.mozilla.org/integration/mozilla-inbound/rev/943638903f10 Drop the workaround in DevTools for setting an animation's startTime to something non-null before setting it to null; r=daisuke
Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/12195 for changes under testing/web-platform/tests
Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.
|
||
Comment 12•6 years ago
|
||
| bugherder | ||
https://hg.mozilla.org/mozilla-central/rev/420ef12d7971 https://hg.mozilla.org/mozilla-central/rev/e9fd70739814 https://hg.mozilla.org/mozilla-central/rev/943638903f10
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
Upstream PR merged
You need to log in
before you can comment on or make changes to this bug.
Description
•