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

NEW
Unassigned

Status

P3
minor
4 years ago
2 months ago

People

(Reporter: jwatt, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [btpp-backlog][good first bug])

Attachments

(1 attachment)

(Reporter)

Description

4 years ago
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.
(Reporter)

Comment 1

4 years ago
Created attachment 8586066 [details] [diff] [review]
working towards something like this
Component: Developer Tools → Developer Tools: Inspector
Status: NEW → ASSIGNED
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.
Depends on: 1155663
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
Mentor: jdescottes
Severity: normal → minor
Priority: -- → P3
Whiteboard: [btpp-backlog][good first bug]
pbro: currently assigned to you, do you want to keep it?
Flags: needinfo?(pbrosset)
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
Assignee: pbrosset → nobody
Status: ASSIGNED → NEW
Flags: needinfo?(pbrosset)
[good first bug] whiteboard -> keyword mass change
Keywords: good-first-bug

Comment 7

2 years ago
I would like to work on this bug

Comment 8

2 years ago
never mind
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.
Mentor: jdescottes
Keywords: good-first-bug

Updated

3 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.