Closed Bug 1211886 Opened 4 years ago Closed 4 years ago

Infinite animations that are past their first iterations can't be paused/played


(DevTools :: Inspector: Animations, defect)

Not set


(firefox45 fixed)

Firefox 45
Tracking Status
firefox45 --- fixed


(Reporter: pbro, Assigned: pbro)



(Whiteboard: [polish-backlog][difficulty=medium])


(1 file, 1 obsolete file)

- go to
- open the animation-inspector
- wait until all animations have completed their first iteration (i.e. the scrubber has gone beyond the timelines in the panel)
- try to use the pause button in the toolbar

Expected: the pause button still looks like a pause button and clicking it actually pauses the animations displayed in the timeline.

Actual: once the scrubber reaches the end of the timeline, the button turns to its play mode again, and clicking it has no effect on the animations.
I have a fix for this, build on top of a rather long commit series that ends with bug 1211801, so making this depend on bug 1211801.
Assignee: nobody → pbrosset
Depends on: 1211801
Blocks: de-44-polish
Blocks: 985861
No longer blocks: 1153271
No need to wait for bug 1211801 which I'll land later after all. This is small annoying bug that should be fixed asap. Will attach a patch later.
No longer depends on: 1211801
The code change itself is pretty simple: if there are infinite animations displayed in the timeline, we let the scrubber move even past the timeline bounds. This way there's no doubt possible when deciding if the button should be in paused or playing state.
I added more test cases for this.

@Mike: this is a relatively urgent review request, this bug has been noticed when working on our FF44 campaign demo material, so it'd be great to fix it before 44.
Attachment #8679968 - Flags: review?(mratcliffe)
Comment on attachment 8679968 [details] [diff] [review]

Wrong patch, sorry.
Attachment #8679968 - Attachment is obsolete: true
Attachment #8679968 - Flags: review?(mratcliffe)
Attached patch bug1211886.patchSplinter Review
Attachment #8679969 - Flags: review?(mratcliffe)
Attachment #8679969 - Flags: review?(mratcliffe) → review+
This try build is catching some weird, seemingly unrelated, leaks in other devtools tests.
Since they seem really unrelated and I don't see how this patch could cause them, I'm going to request check-in.
Keywords: checkin-needed
Got confirmation today that the leaks in the previous try builds are unrelated. Landing this now:
Keywords: checkin-needed
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Comment on attachment 8679969 [details] [diff] [review]

Approval Request Comment

[Feature/regressing bug #]: This is a bug from the original animation-panel timeline implementation in bug 1153271 and bug 1155661.

[User impact if declined]: If declined, the UI of the animation-panel (which is one of the tools demo'd heavily in FF44 marketing campaign) will be wrong for infinite animations. Indeed, the pause button will go back to play state after a while even if the animation is still playing.

[Describe test coverage new/current, TreeHerder]: Added a new browser devtools mochitest for this.

[Risks and why]: There are quite a few tests for the animation UI, so this is low risk, I don't think this introduced any new regressions. If it did, it will be something related to the play/pause button in the animation-inspector. So it's quite contained.

[String/UUID change made/needed]: None.
Attachment #8679969 - Flags: approval-mozilla-aurora?
Alright, it seems we have "blanket approval" for devtools polish bugs for dev-edition 44 (and this one is a pretty important one for the animation-inspector which is demo'd in the marketing campaign). So here it goes:
Attachment #8679969 - Flags: approval-mozilla-aurora?
removing the b2g 2.5 flag since this commit has been reverted due to an incorrect merge, sorry for the confusion
Component: Developer Tools: Inspector → Developer Tools: Animation Inspector
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.