1149501 - Convert all DevTools code that refers to animation players to refer to animations instead

Status: NEW

(DevTools :: Inspector, defect, P3)

Description

[Jonathan Watt [:jwatt]]

The Web Animations spec was changed to merge the AnimationPlayer and Animation interfaces/objects. As a result there is no concept of a separate "player" now, only animations. Bug 1145246 has gotten rid of most player related names and comments, but devtools backwards compatibility requirements make its complete conversion more complicated. As requested in bug 1145246 comment 10, this is a follow-up bug to deal with expunging all references to "player" from the devtools code.

Comment 1

[Jonathan Watt [:jwatt]]

Attachment: working towards something like this

Comment 2

[Patrick Brosset <:pbro>]

Making this depend on bug 1155663 since I'm making some changes to the animations actors there that I'd like to avoid having to merge in. I'll start working on this now.

Comment 3

[Julian Descottes [:jdescottes]]

Bug 1155663 has now landed and we should be ok to start this cleanup bug. 

The goal is to rename animationplayer to animation in all the devtools code. 
After renaming, we should check this didn't create any conflict with the existing variables, property names etc... Running the test suite should take care of this.

This can be a good first bug, if you want to focus on setting up the development environment and following the contribution process. And cleanups are always welcome!

Inspector bug triage. Filter on CLIMBING SHOES

Comment 4

[Julian Descottes [:jdescottes]]

pbro: currently assigned to you, do you want to keep it?

Comment 5

[Patrick Brosset <:pbro>]

You're right, I'm not working on this at the moment, I will unassign myself.

Note however that most of it should be simple, but some parts might be harder/impossible.
Indeed, we have some backward compatibility [1] implications here. Changing an actor name (AnimationPlayerActor) might actually be impossible.
Changing actor method names (getAnimationPlayersForNode) is ok, but takes some steps: the new method should be defined on the actor, then the method should be wrapped on the front-side (with protocol.custom), and it should check if the actor has the new or the old method (with target.actorHasMethod), and based on the result, call the right one.

[1] https://wiki.mozilla.org/DevTools/Backwards_Compatibility

Comment 6

[Patrick Brosset <:pbro>]

[good first bug] whiteboard -> keyword mass change

Comment 7

[omomuro.m]

I would like to work on this bug

Comment 8

[omomuro.m]

never mind

Comment 9

[Patrick Brosset <:pbro>]

I don't think this is a good-first-bug and shouldn't be advertised as such. It's something that will require some backward compatibility code, that will not fix an issue or add a feature, and is likely to cause problems.
If we ever want to fix it (for consistency reasons), I wouldn't want a first-time contributor to pick this up.