Closed Bug 1122414 Opened 5 years ago Closed 5 years ago

When a waapi AnimationEffect corresponds to a CSS transition, its name property should be the transition property

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: pbro, Assigned: birtles)

References

Details

Attachments

(3 files)

This request comes out of working on the new animation inspector panel in the devtools.

It's useful for users to know which css animation or transition a specific widget in the devtools corresponds to.
The WebAnimations API defines that an AnimationEffect has a name property (https://w3c.github.io/web-animations/#animationeffect). It turns out that this name is the name of the @keyframes rule when the owner AnimationPlayer corresponds to a CSS animation.

It's empty for CSS transitions, because transitions don't have names we can use.

My suggestion is to have the AnimationEffect.name return the transitionProperty in that case. It's better than returning nothing and would be useful in the devtools animation inspector to help users relate whatever they are seeing to their CSS.

I talked to Brian Birtles about this and he said it sounded reasonable. Of course this would need to be spec'd too at some stage.
It turns out having a name for transitions would be more helpful than just for displaying it to users. When transitions are added and removed from a node, new Animation objects are created everytime. And so if we want the devtools animation inspector to show these as coming from the same transition, we need an identifier. This name would help.
Blocks: 1149999
What sort of name do you have in mind? For example, if it's a transition of the 'transform' property should the name be 'transform'?
Flags: needinfo?(pbrosset)
Actually, I just checked the summary and it says just that. Once we expose keyframes DevTools could fill in this information automatically but I guess until then we need to fill in the name.
Flags: needinfo?(pbrosset)
(In reply to Brian Birtles (:birtles) from comment #2)
> What sort of name do you have in mind? For example, if it's a transition of
> the 'transform' property should the name be 'transform'?
Exactly (bonus points if it doesn't return 'all' for Animation objects created by a 'transition:all' property).
Assignee: nobody → bbirtles
Status: NEW → ASSIGNED
This is a bit awkward. We lazily set mName to the transition property and then
return it. The reasons for this approach are:

* We don't really want to eagerly fill in mName for all transitions since in
  99% of cases we'll never use it and this will lead to wasted allocations.

* The signature of Name() returns a const nsString reference. This is because
  Name() is used when building CSS Animations (to compare different copies of
  the same animation when updating). For that case we don't really want to
  generate unnecessary copies of nsString objects so we return a reference.
  However, that means for transitions as well we need to return a reference so
  we can't just generate a temporary string on-demand.

  As a result we also have to const-cast ourselves so we can update the mName
  member. We could make mName mutable but seeing as it's only set once, the
  const_cast seems more appropriate.
Attachment #8587794 - Flags: review?(jwatt)
Comment on attachment 8588499 [details] [diff] [review]
part 3 - Update DevTools tests to expect a name for transitions

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

Thanks Brian for working on this!
Attachment #8588499 - Flags: review?(pbrosset) → review+
Attachment #8587793 - Flags: review?(jwatt) → review+
Attachment #8587794 - Flags: review?(jwatt) → review+
Comment on attachment 8587794 [details] [diff] [review]
part 2 - Make return the transitionProperty from Animation.name for CSS transitions

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

Oh, and can you fix the first line of the review comment? ("Make return the transitionProperty from Animation.name for CSS transitions")
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.