1211810 - Lock highlighter on animated elements in the animation-inspector

Status: RESOLVED FIXED

(DevTools :: Inspector: Animations, defect)

Description

[Patrick Brosset <:pbro>]

The animated elements that are shown in the animation-inspector panel allow 2 mouse interactions:
- hovering over them highlights them in the page (the highlighter stays visible only as long as the mouse is over the element in the panel),
- clicking on the inspector icon next to them selects them in the markup-view

We'd like to add a third type of interaction: locking the highlighter on an element. The idea is that it might be useful to see the highlighter on an element while you're using the timeline.
For this to work, we'd need a way to show the highlighter, and then hide it.

Comment 1

[Patrick Brosset <:pbro>]

Attachment: Bug_1211810_-_Add_a_way_to_lock_the_highlighter_on.diff

Comment 2

[Patrick Brosset <:pbro>]

Attachment: Bug_1211810_-_Add_a_way_to_lock_the_highlighter_on.diff

This patch modifies the way DOM nodes are previewed in the animation-inspector.
Now, clicking on the DOM node output itself will select it in the inspector (and there's a tooltip for this), and clicking on the inspector icon *next* to the DOM node output will "lock" the highlighter on the node, in the page.

This proves to be very useful when you want to track an element that's being transformed in an animation. You can use the tool to highlight the element, and then scrub back and forth to see it being transformed easily.

This uses a new instance of the highlighter so that it doesn't interfere with the other highlighter that is used on node hover in the markup-view, breadcrumbs, etc.

This patch also:
- removes some old unused code in animation-panel.js that I had forgotten to remove before,
- adds a new test
- modifies a couple of existing tests so they pass.

Pending try build: https://treeherder.mozilla.org/#/jobs?repo=try&revision=1075094fc413

Comment 3

[Tom Tromey :tromey]

Comment on attachment 8671398 [details] [diff] [review]
Bug_1211810_-_Add_a_way_to_lock_the_highlighter_on.diff

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

Looks good to me.

::: devtools/client/animationinspector/utils.js
@@ +146,5 @@
> + * that is used to lock the highlighter on animated nodes in the page.
> + * It instantiates a new highlighter that is then shared amongst all instances
> + * of AnimationTargetNode. This is useful because that means showing the
> + * highlighter on one animated node will unhighlight the previously highlighted
> + * one, but will not interfer with the default inspector highlighter.

Typo, "interfere".

Comment 4

[Patrick Brosset <:pbro>]

https://hg.mozilla.org/integration/fx-team/rev/f7a73a8c22ce

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/f7a73a8c22ce

Comment 6

[Alfred Kayser]

Why the change from .node-selector to .node-highlighter?
This impacts themes unnecessarily.

Comment 7

[Patrick Brosset <:pbro>]

(In reply to Alfred Kayser from comment #6)
> Why the change from .node-selector to .node-highlighter?
> This impacts themes unnecessarily.
That's right, I guess I did not think of this. Which themes do you have in mind? To my knowledge, there's only the light/dark themes which we maintain ourselves and the firebug theme.
To answer your question, I changed the classname because node-highlighter is more meaningful in this case than node-selector.

Comment 8

[Alfred Kayser]

See: https://addons.mozilla.org/en-US/firefox/complete-themes/
I know that Mozilla is actively pushing these away, but there are still many people using and developing these themes.

Comment 9

[Will Bamberg [:wbamberg]]

I've added a section on Firefox 44: https://developer.mozilla.org/en-US/docs/Tools/Page_Inspector/How_to/Work_with_animations#Firefox_44

Also see bug 1205681, comment 15 for more on why I've chosen to present it like this.

Comment 10

[Patrick Brosset <:pbro>]

LGTM, thanks Will!