Closed Bug 1122466 Opened 5 years ago Closed 5 years ago

The AnimationPlayerActor sometimes doesn't retrieve the right duration/delay/iterationCount from the computed-styles

Categories

(DevTools :: Inspector: Animations, defect)

defect
Not set

Tracking

(firefox38 fixed)

RESOLVED FIXED
Firefox 38
Tracking Status
firefox38 --- fixed

People

(Reporter: pbro, Assigned: pbro)

References

Details

Attachments

(1 file, 2 obsolete files)

Because the WebAnimations API doesn't yet give us access to an animation delay, duration or iterationCount, the AnimationPlayerActor currently figures this out by looking at the current node's computed styles

It gets more complex when mutliple animations or transitions are defined on the node because then the computed delay may be a string like "1s, 5s, 2s" for instance, and so we have to split it by "," and figure out at which index of the split string array the right value is.

For now, what we've been doing is:
- when we retrieve the list of animations from the WebAnimations API, we get an array of AnimationPlayer objects,
- we just remember the index of each of these objects and use this index to parse the style strings

But, imagine a node with 3 animations, and one of them has ended.
Now you get the list of AnimationPlayers using WebAnimations API. It will give you an array with 2 entries only, since it does not return animations that have ended (note that this may change in the future).
So using the index of this array to parse the style string won't work anymore.

So the proposal here is to use the AnimationPlayer's name, then parse the computedStyle.animationName property, to find the name there, and get the index from this instead.

Of course this won't work with css transitions because they have no name, and the WebAnimations API doesn't give us the name of the transitionProperty (which we could have used instead in a similar way). But for transitions, we can continue on using the current solution.

This means that we partly have to live with this problem in some situations, but not all.

Having said this, I believe it's not a huge deal to live with this for now. The panel is in its alpha state for now, and so is the WebAnimations API implementation anyway. Things will get better in time.
Assignee: nobody → pbrosset
I still need to add a new test for this change.
Blocks: 1120900
Attachment #8550220 - Attachment is obsolete: true
Attachment #8555117 - Flags: review?(mratcliffe)
Comment on attachment 8555117 [details] [diff] [review]
bug1122466-retrieve-the-right-animation-index-in-computedstyles v2.patch

Review of attachment 8555117 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/devtools/server/actors/animation.js
@@ +106,5 @@
> +      return this.playerIndex;
> +    }
> +
> +    // If there's only one name.
> +    if (names.indexOf(",") === -1) {

You could use names.contains(",") here. It only gives a slight performance increase so use whatever you prefer.
Attachment #8555117 - Flags: review?(mratcliffe) → review+
Thanks Mike. Updated the patch as per your comment.
New try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=6c507f455c11
Attachment #8555117 - Attachment is obsolete: true
Attachment #8563438 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/266257775fc9
Status: NEW → RESOLVED
Closed: 5 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 38
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.