Closed
Bug 1453010
Opened 7 years ago
Closed 7 years ago
Clicking the "target" icon doesn't lock the highlighter on the element
Categories
(DevTools :: Inspector: Animations, defect, P1)
Tracking
(firefox-esr52 unaffected, firefox60 unaffected, firefox61 verified)
VERIFIED
FIXED
Firefox 61
Tracking | Status | |
---|---|---|
firefox-esr52 | --- | unaffected |
firefox60 | --- | unaffected |
firefox61 | --- | verified |
People
(Reporter: gpalko, Assigned: daisuke)
References
(Blocks 2 open bugs)
Details
(Keywords: regression)
Attachments
(4 files)
[Environment:]
Windows 8.1, Windows 10, Mac OSX 10.13.2
Nightly 61.0a1 2018-04-04
[Steps:]
1. Open Firefox Nightly and load https://rawgit.com/dadaa/3b73f847427025b51ba1ab7333013d0c/raw/77f3f0bb884875a179c3407f73bf8a8dd54751c9/doc_simple_animation.html.
2. Press F12.
3. Select Inspector.
4. Select Animation tab.
5. Click the "target" button near an element locator(e.g. div.ball.multi) and move the mouse cursor away from the button
[Actual Result:]
The element is not highlighted.
[Expected Result:]
5. Animation elements should be highlighted if their "target" button was clicked
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•7 years ago
|
||
This try includes Reps change:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=b2843c2a2549e0c3a5a2a949994e58e169d678d4
Comment 6•7 years ago
|
||
mozreview-review |
Comment on attachment 8966900 [details]
Bug 1453010 - Part 1: Lock highlighted element by clicking on the inspect icon in AnimationTarget component.
https://reviewboard.mozilla.org/r/235558/#review241296
Code analysis found 1 defect in this patch:
- 1 defect found by mozlint
You can run this analysis locally with:
- `./mach lint path/to/file` (JS/Python)
If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx
::: devtools/client/inspector/animation/components/AnimationTarget.js:83
(Diff revision 1)
> const {
> onHideBoxModelHighlighter,
> onShowBoxModelHighlighterForNode,
> + highlightingNodeActorID,
> + lockHighlighting,
> setSelectedNode,
Error: 'setselectednode' is assigned a value but never used. [eslint: no-unused-vars]
Comment 7•7 years ago
|
||
mozreview-review |
Comment on attachment 8966901 [details]
Bug 1453010 - Part 2: Change the title of inspect icon.
https://reviewboard.mozilla.org/r/235560/#review241298
Code analysis found 1 defect in this patch:
- 1 defect found by mozlint
You can run this analysis locally with:
- `./mach lint path/to/file` (JS/Python)
If you see a problem in this automated review, please report it here: http://bit.ly/2y9N9Vx
::: devtools/client/inspector/animation/components/AnimationTarget.js:85
(Diff revision 1)
> const {
> onHideBoxModelHighlighter,
> onShowBoxModelHighlighterForNode,
> highlightingNodeActorID,
> lockHighlighting,
> setSelectedNode,
Error: 'setselectednode' is assigned a value but never used. [eslint: no-unused-vars]
Assignee | ||
Comment 8•7 years ago
|
||
These patches depend on following PR:
https://github.com/devtools-html/devtools-core/pull/1028
Assignee | ||
Updated•7 years ago
|
Assignee: nobody → dakatsuka
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Updated•7 years ago
|
tracking-firefox61:
--- → +
Updated•7 years ago
|
Priority: -- → P1
Comment 13•7 years ago
|
||
Friendly review ping!
Comment 14•7 years ago
|
||
I am wondering if this should actually be a feature. The target icon is actually overused in a number of different places and for different reasons. In the rules, the target icon acts as a selector highlighter toggle and in the grid panel it acts as a select element and a brief box model highlighter only on hover. This bug would make it behave like the former.
If we wanted to make the changes so it is highlighted briefly, we can just do what is seen here and simplify the entire solution. https://searchfox.org/mozilla-central/source/devtools/client/inspector/grids/components/GridItem.js#117
Comment 15•7 years ago
|
||
Disregard what I said, I see it is a current feature in the old animation inspector.
Comment 16•7 years ago
|
||
mozreview-review |
Comment on attachment 8966900 [details]
Bug 1453010 - Part 1: Lock highlighted element by clicking on the inspect icon in AnimationTarget component.
https://reviewboard.mozilla.org/r/235558/#review243730
::: devtools/client/inspector/animation/actions/animations.js:48
(Diff revision 2)
> elementPickerEnabled,
> };
> },
>
> /**
> + * Update the highlighting node.
s/highlighting/highlighted
::: devtools/client/inspector/animation/actions/animations.js:50
(Diff revision 2)
> },
>
> /**
> + * Update the highlighting node.
> + */
> + updateHighlightingNode(nodeFront) {
updateHighlightedNode
::: devtools/client/inspector/animation/actions/index.js:20
(Diff revision 2)
> "UPDATE_DETAIL_VISIBILITY",
>
> // Update state of the picker enabled.
> "UPDATE_ELEMENT_PICKER_ENABLED",
>
> + // Update highlighting node.
s/highlighting/highlighted
::: devtools/client/inspector/animation/actions/index.js:21
(Diff revision 2)
>
> // Update state of the picker enabled.
> "UPDATE_ELEMENT_PICKER_ENABLED",
>
> + // Update highlighting node.
> + "UPDATE_HIGHLIGHTING_NODE",
s/HIGHLIGHTING/HIGHLIGHTED
::: devtools/client/inspector/animation/animation.js:41
(Diff revision 2)
> this.addAnimationsCurrentTimeListener.bind(this);
> this.getAnimatedPropertyMap = this.getAnimatedPropertyMap.bind(this);
> this.getAnimationsCurrentTime = this.getAnimationsCurrentTime.bind(this);
> this.getComputedStyle = this.getComputedStyle.bind(this);
> this.getNodeFromActor = this.getNodeFromActor.bind(this);
> + this.lockHighlighting = this.lockHighlighting.bind(this);
s/lockHighlighting/setHighlightedNode
::: devtools/client/inspector/animation/animation.js:175
(Diff revision 2)
> this.simulatedAnimationForKeyframesProgressBar = null;
> }
>
> this.stopAnimationsCurrentTimeTimer();
>
> + if (this.highlighter) {
I think we can remove this and just call onHideBoxModelHighlighter
::: devtools/client/inspector/animation/animation.js:269
(Diff revision 2)
> + *
> + * @param {NodeFront} nodeFront
> + */
> + async lockHighlighting(nodeFront) {
> + if (!this.highlighter) {
> + this.highlighter = await this.inspector.toolbox.highlighterUtils
This should be calling onShowBoxModelHighlighterForNode
::: devtools/client/inspector/animation/animation.js:273
(Diff revision 2)
> + if (!this.highlighter) {
> + this.highlighter = await this.inspector.toolbox.highlighterUtils
> + .getHighlighterByType("BoxModelHighlighter");
> + }
> +
> + await this.highlighter.hide();
onHideBoxModelHighlighter
::: devtools/client/inspector/animation/components/AnimationTarget.js:99
(Diff revision 2)
> + const isHighlighting = nodeFront.actorID === highlightingNodeActorID;
> +
> return dom.div(
> {
> - className: "animation-target"
> + className: "animation-target" +
> + (isHighlighting ? " highlighting" : ""),
s/isHighlighting/isHighlighted
::: devtools/client/inspector/animation/reducers/animations.js:22
(Diff revision 2)
>
> const INITIAL_STATE = {
> animations: [],
> detailVisibility: false,
> elementPickerEnabled: false,
> + highlightingNodeActorID: null,
s/highlightingNodeActorID/highlightedNode
::: devtools/client/inspector/animation/reducers/animations.js:62
(Diff revision 2)
> return Object.assign({}, state, {
> elementPickerEnabled
> });
> },
>
> + [UPDATE_HIGHLIGHTING_NODE](state, { highlightingNodeActorID }) {
UPDATE_HIGHLIGHTED_NODE
Attachment #8966900 -
Flags: review?(gl) → review+
Comment 17•7 years ago
|
||
mozreview-review |
Comment on attachment 8966901 [details]
Bug 1453010 - Part 2: Change the title of inspect icon.
https://reviewboard.mozilla.org/r/235560/#review243742
Attachment #8966901 -
Flags: review?(gl) → review+
Comment 18•7 years ago
|
||
mozreview-review |
Comment on attachment 8966902 [details]
Bug 1453010 - Part 3: Select a node by clicking on dom node element in AnimationTargetCompositor.
https://reviewboard.mozilla.org/r/235562/#review243744
::: devtools/client/inspector/animation/components/AnimationTarget.js:111
(Diff revision 2)
> {
> defaultRep: ElementNode,
> mode: MODE.TINY,
> inspectIconTitle: getInspectorStr("inspector.nodePreview.highlightNodeLabel"),
> object: translateNodeFrontToGrip(nodeFront),
> + onDOMNodeClick: () => setSelectedNode(nodeFront, { reason: "animation-panel" }),
Consider also scrolling into view like https://searchfox.org/mozilla-central/source/devtools/client/inspector/grids/components/GridItem.js#86
Attachment #8966902 -
Flags: review?(gl) → review+
Comment 19•7 years ago
|
||
mozreview-review |
Comment on attachment 8966902 [details]
Bug 1453010 - Part 3: Select a node by clicking on dom node element in AnimationTargetCompositor.
https://reviewboard.mozilla.org/r/235562/#review243774
::: devtools/client/inspector/animation/components/AnimationTarget.js:111
(Diff revision 2)
> {
> defaultRep: ElementNode,
> mode: MODE.TINY,
> inspectIconTitle: getInspectorStr("inspector.nodePreview.highlightNodeLabel"),
> object: translateNodeFrontToGrip(nodeFront),
> + onDOMNodeClick: () => setSelectedNode(nodeFront, { reason: "animation-panel" }),
Actually, I don't know if onDOMNodeClick should select the node. I expected this to be onInspectIconClick
Attachment #8966902 -
Flags: review+
Comment 20•7 years ago
|
||
mozreview-review |
Comment on attachment 8966902 [details]
Bug 1453010 - Part 3: Select a node by clicking on dom node element in AnimationTargetCompositor.
https://reviewboard.mozilla.org/r/235562/#review243782
::: devtools/client/inspector/animation/components/AnimationTarget.js:111
(Diff revision 2)
> {
> defaultRep: ElementNode,
> mode: MODE.TINY,
> inspectIconTitle: getInspectorStr("inspector.nodePreview.highlightNodeLabel"),
> object: translateNodeFrontToGrip(nodeFront),
> + onDOMNodeClick: () => setSelectedNode(nodeFront, { reason: "animation-panel" }),
Ah disregard this as well, I see this is what it does in the current animation inspector. I would still consider adding scrollIntoView
Attachment #8966902 -
Flags: review+
Comment 21•7 years ago
|
||
mozreview-review |
Comment on attachment 8966903 [details]
Bug 1453010 - Part 4: Add test for locking highlighting.
https://reviewboard.mozilla.org/r/235564/#review243904
::: devtools/client/inspector/animation/test/browser_animation_animation-target_highlight.js:19
(Diff revision 2)
> +// the class will add to those animation target as well
> +
> +add_task(async function() {
> + await addTab(URL_ROOT + "doc_simple_animation.html");
> + await removeAnimatedElementsExcept([".animated", ".multi"]);
> + const { animationInspector, panel } = await openAnimationInspector();
We can simplify this by either getting the inspector or toolbox from openAnimationInspector
and do
toolbox.once("node-highlight")
::: devtools/client/inspector/animation/test/browser_animation_animation-target_highlight.js:21
(Diff revision 2)
> +add_task(async function() {
> + await addTab(URL_ROOT + "doc_simple_animation.html");
> + await removeAnimatedElementsExcept([".animated", ".multi"]);
> + const { animationInspector, panel } = await openAnimationInspector();
> +
> + info("Checking highlighting when mouse over on a target node");
s/Checking/Check/g
::: devtools/client/inspector/animation/test/browser_animation_animation-target_highlight.js:33
(Diff revision 2)
> + let onUnhighlight = animationInspector.inspector.toolbox.once("node-unhighlight");
> + mouseOutOnTargetNode(animationInspector, panel, 0);
> + await onUnhighlight;
> + ok(true, "Unhighlighted the targe node");
> +
> + info("Checking locking highlighting when click on the inspect icon");
Check node is highlighted when the inspect icon is clicked.
::: devtools/client/inspector/animation/test/browser_animation_animation-target_highlight.js:39
(Diff revision 2)
> + onHighlight = animationInspector.inspector.toolbox.once("node-highlight");
> + await clickOnInspectIcon(animationInspector, panel, 0);
> + nodeFront = await onHighlight;
> + assertNodeFront(nodeFront, "DIV", "ball animated");
> + ok(panel.querySelectorAll(".animation-target")[0].classList.contains("highlighting"),
> + "The highlighting animation target element should have 'highlighting' class");
The highlighted animation target
::: devtools/client/inspector/animation/test/browser_animation_animation-target_highlight.js:41
(Diff revision 2)
> + nodeFront = await onHighlight;
> + assertNodeFront(nodeFront, "DIV", "ball animated");
> + ok(panel.querySelectorAll(".animation-target")[0].classList.contains("highlighting"),
> + "The highlighting animation target element should have 'highlighting' class");
> +
> + info("Checking keeping to lock even if mouse is out from the animation target");
Check if the animation target is still highlighted on mouse out
::: devtools/client/inspector/animation/test/browser_animation_animation-target_highlight.js:45
(Diff revision 2)
> +
> + info("Checking keeping to lock even if mouse is out from the animation target");
> + mouseOutOnTargetNode(animationInspector, panel, 0);
> + await wait(500);
> + ok(panel.querySelectorAll(".animation-target")[0].classList.contains("highlighting"),
> + "The highlighting element still should have 'highlighting' class");
The highlighted element
::: devtools/client/inspector/animation/test/browser_animation_animation-target_highlight.js:47
(Diff revision 2)
> + mouseOutOnTargetNode(animationInspector, panel, 0);
> + await wait(500);
> + ok(panel.querySelectorAll(".animation-target")[0].classList.contains("highlighting"),
> + "The highlighting element still should have 'highlighting' class");
> +
> + info("Checking locking element after changing to the other");
info("Highlighting another animation target");
::: devtools/client/inspector/animation/test/browser_animation_animation-target_highlight.js:53
(Diff revision 2)
> + onHighlight = animationInspector.inspector.toolbox.once("node-highlight");
> + await clickOnInspectIcon(animationInspector, panel, 1);
> + nodeFront = await onHighlight;
> + assertNodeFront(nodeFront, "DIV", "ball multi");
> +
> + info("Checking 'highlighting' class");
info("Check the highlighted state of the animation targets")
::: devtools/client/inspector/animation/test/head.js:202
(Diff revision 2)
> EventUtils.synthesizeMouse(controllerEl, mousedonwX, 0, {}, controllerEl.ownerGlobal);
> await waitForSummaryAndDetail(animationInspector);
> };
>
> /**
> + * Click on an inspect icon in AnimationTargetComponent.
Reword this all to be:
Click on the inspect icon for the given AnimationTargetComponent
::: devtools/client/inspector/animation/test/head.js:205
(Diff revision 2)
>
> /**
> + * Click on an inspect icon in AnimationTargetComponent.
> + *
> + * @param {AnimationInspector} animationInspector.
> + * @param {AnimationsPanel} panel
I am really hesistant on keeping this. There's technically no object called AnimationsPanel anymore and really this is an DOMElement.
I would change all of this @param {AnimationsPanel} to @param {DOMElement} in this file or get rid of panel and do animationInspector.doc.querySelector everywhere.
::: devtools/client/inspector/animation/test/head.js:264
(Diff revision 2)
> // Restore the scrubber style.
> scrubberEl.style.pointerEvents = "unset";
> };
>
> /**
> + * Click on a target node in AnimationTargetComponent.
Click on the target node for the given AnimationTargetComponent index.
::: devtools/client/inspector/animation/test/head.js:372
(Diff revision 2)
> const rate = 1 / bounds.width * pixels;
> return { duration, rate };
> };
>
> /**
> + * Mouse over on a target node in AnimationTargetComponent.
Mouse over the target node for the given AnimationTargetComponent index.
::: devtools/client/inspector/animation/test/head.js:385
(Diff revision 2)
> +const mouseOverOnTargetNode = function(animationInspector, panel, index) {
> + info(`Mouse over on a target node in animation target component[${ index }]`);
> + const targetEl = panel.querySelectorAll(".animation-target .objectBox")[index];
> + targetEl.scrollIntoView(false);
> + EventUtils.synthesizeMouse(targetEl, 10, 5,
> + { type: "mouseover" }, targetEl.ownerGlobal);
Just a single tab is enough.
::: devtools/client/inspector/animation/test/head.js:389
(Diff revision 2)
> + EventUtils.synthesizeMouse(targetEl, 10, 5,
> + { type: "mouseover" }, targetEl.ownerGlobal);
> +};
> +
> +/**
> + * Mouse out on a target node in AnimationTargetComponent.
Mouse out of the target node for the given AnimationTargetComponent index.
::: devtools/client/inspector/animation/test/head.js:402
(Diff revision 2)
> +const mouseOutOnTargetNode = function(animationInspector, panel, index) {
> + info(`Mouse out on a target node in animation target component[${ index }]`);
> + const targetEl = panel.querySelectorAll(".animation-target .objectBox")[index];
> + targetEl.scrollIntoView(false);
> + EventUtils.synthesizeMouse(targetEl, -1, -1,
> + { type: "mouseout" }, targetEl.ownerGlobal);
Just a single tab. We can even make this one line if you did const el instead of targetEl
Attachment #8966903 -
Flags: review?(gl) → review+
Assignee | ||
Comment 22•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8966900 [details]
Bug 1453010 - Part 1: Lock highlighted element by clicking on the inspect icon in AnimationTarget component.
https://reviewboard.mozilla.org/r/235558/#review243730
> This should be calling onShowBoxModelHighlighterForNode
Thank you for the review, Gabriel.
Unfortunately, although I have tried this way, onShowBoxModelHighlighterForNode did not work at here. Because, even if set the highlighted node by onShowBoxModelHighlighterForNode at here, when mouse was out of Rep component, onHideBoxModelHighlighter will be called. Naturally, the highlight was hidden.
Am I wrong something how to use?
I think we need an independent highlighter for locking as same as previous animation inspector[1].
[1] https://searchfox.org/mozilla-central/source/devtools/client/inspector/shared/dom-node-preview.js#325
Comment 23•7 years ago
|
||
(In reply to Daisuke Akatsuka (:daisuke) from comment #22)
> Comment on attachment 8966900 [details]
> Bug 1453010 - Part 1: Lock highlighted element by clicking on the inspect
> icon in AnimationTarget component.
>
> https://reviewboard.mozilla.org/r/235558/#review243730
>
> > This should be calling onShowBoxModelHighlighterForNode
>
> Thank you for the review, Gabriel.
>
> Unfortunately, although I have tried this way,
> onShowBoxModelHighlighterForNode did not work at here. Because, even if set
> the highlighted node by onShowBoxModelHighlighterForNode at here, when mouse
> was out of Rep component, onHideBoxModelHighlighter will be called.
> Naturally, the highlight was hidden.
> Am I wrong something how to use?
>
> I think we need an independent highlighter for locking as same as previous
> animation inspector[1].
>
> [1]
> https://searchfox.org/mozilla-central/source/devtools/client/inspector/
> shared/dom-node-preview.js#325
Maybe it's just weird that the target would toggle the box model highlighter in this case. We use highlighters-overlay.js to manage all our highlighter states because you can imagine if you navigate out of a page we need to clear this highlighter by hiding it and when it goes back to the page, it doesn't re-highlight it.
Comment 24•7 years ago
|
||
If we still want to proceed, I think you should implement the right methods in highlighters-overlay and send another review.
Comment 25•7 years ago
|
||
Alternatively, remove the onDOMNodeMouseOut and onDOMNodeMouseOver methods for the rep.
Assignee | ||
Comment 26•7 years ago
|
||
We can't remove onDOMNodeMouseOut, onDOMNodeMouseOver from the rep, because we want same feature for previous animation inspector.
Okay, I'll try to implement in highlighters-overlay.
Assignee | ||
Comment 27•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8966902 [details]
Bug 1453010 - Part 3: Select a node by clicking on dom node element in AnimationTargetCompositor.
https://reviewboard.mozilla.org/r/235562/#review243744
> Consider also scrolling into view like https://searchfox.org/mozilla-central/source/devtools/client/inspector/grids/components/GridItem.js#86
Indeed, I'll do this. Thanks!
Assignee | ||
Comment 28•7 years ago
|
||
Let me cancel the r+ for patch 1, because I modified highlight-overlay.
Assignee | ||
Updated•7 years ago
|
Attachment #8966900 -
Flags: review+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 33•7 years ago
|
||
Assignee | ||
Updated•7 years ago
|
Attachment #8966900 -
Flags: review?(gl)
Comment 34•7 years ago
|
||
mozreview-review |
Comment on attachment 8966900 [details]
Bug 1453010 - Part 1: Lock highlighted element by clicking on the inspect icon in AnimationTarget component.
https://reviewboard.mozilla.org/r/235558/#review246036
::: devtools/client/inspector/animation/animation.js:147
(Diff revision 3)
> this.inspector.toolbox.on("picker-started", this.onElementPickerStarted);
> this.inspector.toolbox.on("picker-stopped", this.onElementPickerStopped);
> this.inspector.toolbox.on("select", this.onSidebarSelectionChanged);
> }
>
> - destroy() {
> + async destroy() {
I don't see any await inside of this destroy(). Do we need this async here?
::: devtools/client/inspector/animation/animation.js:449
(Diff revision 3)
> setDetailVisibility(isVisible) {
> this.inspector.store.dispatch(updateDetailVisibility(isVisible));
> }
>
> /**
> + * Set highlighted node. If set null, unhighlight.
Highlight the given node with the box model highlighter. If no node is provided, hide the box model highlighter.
::: devtools/client/inspector/animation/components/AnimationTarget.js:49
(Diff revision 3)
> this.updateNodeFront(nextProps.animation);
> }
> }
>
> shouldComponentUpdate(nextProps, nextState) {
> - return this.state.nodeFront !== nextState.nodeFront;
> + return (this.state.nodeFront !== nextState.nodeFront) ||
I think this will be just as readable without the brackets. I would remove them.
::: devtools/client/inspector/shared/highlighters-overlay.js:411
(Diff revision 3)
> + if (!isShown) {
> + return;
> + }
> +
> + this.boxModelHighlighterShown = node;
> + this.emit("box-model-highlighter-shown", node, options);
Same as abelow
::: devtools/client/inspector/shared/highlighters-overlay.js:424
(Diff revision 3)
> + return;
> + }
> +
> + await this.highlighters.BoxModelHighlighter.hide();
> + this.boxModelHighlighterShown = null;
> + this.emit("box-model-highlighter-hidden");
I don't see us using this emitted event. So, maybe remove it for now.
Attachment #8966900 -
Flags: review?(gl) → review+
Assignee | ||
Comment 35•7 years ago
|
||
mozreview-review-reply |
Comment on attachment 8966900 [details]
Bug 1453010 - Part 1: Lock highlighted element by clicking on the inspect icon in AnimationTarget component.
https://reviewboard.mozilla.org/r/235558/#review246036
> I don't see any await inside of this destroy(). Do we need this async here?
Thanks, Gabriel.
Ah, that's the remains of previous changes. Thanks.
> I don't see us using this emitted event. So, maybe remove it for now.
Okay, thanks!
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 40•7 years ago
|
||
I'll make these patches to land after checking the try.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3c5e0892ab07a912a98f14e1944b7c605f460abb
Comment hidden (mozreview-request) |
Assignee | ||
Comment 42•7 years ago
|
||
I changed the test a bit so as to wait for highlighting when the animation target clicking.
And try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=586881a1d384235eecbf39862f093cf56b7e53cb
Comment 43•7 years ago
|
||
Pushed by dakatsuka@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/bc816979fee8
Part 1: Lock highlighted element by clicking on the inspect icon in AnimationTarget component. r=gl
https://hg.mozilla.org/integration/autoland/rev/407a33054ca6
Part 2: Change the title of inspect icon. r=gl
https://hg.mozilla.org/integration/autoland/rev/a1666927ab16
Part 3: Select a node by clicking on dom node element in AnimationTargetCompositor. r=gl
https://hg.mozilla.org/integration/autoland/rev/cd392b05865e
Part 4: Add test for locking highlighting. r=gl
Comment 44•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/bc816979fee8
https://hg.mozilla.org/mozilla-central/rev/407a33054ca6
https://hg.mozilla.org/mozilla-central/rev/a1666927ab16
https://hg.mozilla.org/mozilla-central/rev/cd392b05865e
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 61
Reporter | ||
Comment 45•7 years ago
|
||
Verified on Win10 and MacOS 10.12 on Nightly 61.0a1(BuildID 20180502100112), the issue is fixed.
Reporter | ||
Updated•7 years ago
|
Status: RESOLVED → VERIFIED
Comment 46•6 years ago
|
||
Thanks for verifying this, also marking the status of 61 as fixed based on your testing.
Reporter | ||
Comment 47•6 years ago
|
||
Removing tracking flag since the feature is pref'ed off in FX61 and has been moved to Fx62
tracking-firefox61:
+ → ---
Updated•6 years ago
|
Product: Firefox → DevTools
Updated•6 years ago
|
status-firefox59:
unaffected → ---
Flags: in-qa-testsuite+
You need to log in
before you can comment on or make changes to this bug.
Description
•