Closed Bug 1327725 Opened 8 years ago Closed 8 years ago

Element picker stays active in previous page when I navigate to another page

Categories

(DevTools :: Inspector, defect, P2)

defect

Tracking

(firefox51 wontfix, firefox52 wontfix, firefox53 wontfix, firefox54 wontfix, firefox55 verified)

RESOLVED FIXED
Firefox 55
Tracking Status
firefox51 --- wontfix
firefox52 --- wontfix
firefox53 --- wontfix
firefox54 --- wontfix
firefox55 --- verified

People

(Reporter: arni2033, Assigned: zer0)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

>>> My Info: Win7_64, Nightly 49, 32bit, ID 20160526082509 STR_1: 1. Open new tab. Focus urlbar, type "data:,a", press Enter. Focus urlbar, type "data:,b", press Enter. 2. Press Ctrl+Shift+C to open inspector 3. Hover mouse over letter "b" on the page 4. Press Alt+Left to navigate to "data:,a" 5. Press F12 to close devtools 6. Press Alt+Right to navigate to "data:,b" AR: Element <pre> on the page is still highlighted ER: No highlight at all, because I closed devtools STR_2: (further experiments) 1. Open new tab. Focus urlbar, type "data:,a", press Enter. Focus urlbar, type "data:,b", press Enter 2. Press Ctrl+Shift+C to open inspector 3. Hover mouse over letter "b" on the page 4. Press Alt+Left to navigate to "data:,a" 5. Move mouse by ~5px, then hover mouse over letter "a" on the page 6. Press Alt+Right to navigate to "data:,b" 7. Move mouse to the center of the page 8. Press Alt+Left to navigate to "data:,a" 9. Move mouse by ~5px 10. Press Alt+Right to navigate to "data:,b" 11. Press F12 to close devtools 12. Press Alt+Left to navigate to "data:,a" AR: Steps 11, 12 - two "highlights" are displayed on the page at the same time. Devtools are closed. ER: Steps 6-10 - only one highlight should be displayed on the page. Steps 11,12 - no highlight.
No longer blocks: 1277113
User Agent Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0 Build ID 20170130030205 I was able to reproduce the issue on the latest Firefox Release (51.0.1) and on the latest Nightly (54.0a1).
Component: Untriaged → Developer Tools: Inspector
This bug is similar to other bugs we've fixed in the past – and currently, to bug 1333707 for example. We should probably hide / destroy the box model highlighter during the `pagehide` event here as well.
Assignee: nobody → zer0
Inspector bug triage (filter on CLIMBING SHOES).
Priority: -- → P2
Note: this patch is built on top of bug 1312103's patch.
Comment on attachment 8837583 [details] Bug 1327725 - ensure the pagehide event we received are for the window's highlighter; https://reviewboard.mozilla.org/r/112708/#review114432 ::: devtools/server/actors/highlighters/box-model.js:105 (Diff revision 1) > * regionFill property: `highlighter.regionFill.margin = "red"; > */ > this.regionFill = {}; > > this.onWillNavigate = this.onWillNavigate.bind(this); > + this.onPageHide = this.hide.bind(this); Move this above this.onWillNavigate ::: devtools/server/actors/highlighters/box-model.js:263 (Diff revision 1) > > /** > * Destroy the nodes. Remove listeners. > */ > destroy: function () { > + let { pageListenerTarget } = this.highlighterEnv; Move this after this.highlighterEnv.off("will-navigate", this.onWillNavigate). I also like to keep the consistent ordering of on/off of the event listeners.
Attachment #8837583 - Flags: review?(gl) → review+
Comment on attachment 8837583 [details] Bug 1327725 - ensure the pagehide event we received are for the window's highlighter; Gabriel, I need another round of review, since the code has changed; could you take a look? Thanks!
Attachment #8837583 - Flags: review+ → review?(gl)
Comment on attachment 8837583 [details] Bug 1327725 - ensure the pagehide event we received are for the window's highlighter; https://reviewboard.mozilla.org/r/112708/#review121264 ::: devtools/server/actors/highlighters/box-model.js:727 (Diff revision 3) > if (isTopLevel) { > this.hide(); > } > + }, > + > + onPageHide: function ({ target }) { Move this above onWillNavigate ::: devtools/server/actors/highlighters/box-model.js:728 (Diff revision 3) > this.hide(); > } > + }, > + > + onPageHide: function ({ target }) { > + // Ensure we're getting this event from the window the highlighter is handling. We should fully explain what is happening: If a page hide event is triggered for current window's highlighter, hide the highlighter. ::: devtools/server/actors/highlighters/css-grid.js:420 (Diff revision 3) > if (isTopLevel) { > this.hide(); > } > }, > > + onPageHide: function ({ target }) { Move above onWillNavigate ::: devtools/server/actors/highlighters/css-grid.js:421 (Diff revision 3) > this.hide(); > } > }, > > + onPageHide: function ({ target }) { > + // Ensure we're getting this event from the window the highlighter is handling. Rephrase the comment as above. ::: devtools/server/actors/highlighters/geometry-editor.js:392 (Diff revision 3) > > - const { type, pageX, pageY } = event; > + const { target, type, pageX, pageY } = event; > > switch (type) { > case "pagehide": > + // Ensure we're getting this event from the window the highlighter is handling. Rephrase the comment. ::: devtools/server/actors/highlighters/measuring-tool.js:538 (Diff revision 3) > break; > case "scroll": > this.hideLabel("position"); > break; > case "pagehide": > + // Ensure we're getting this event from the window the highlighter is handling. Rephrase the comment. ::: devtools/server/actors/highlighters/rulers.js:204 (Diff revision 3) > switch (event.type) { > case "scroll": > this._onScroll(event); > break; > case "pagehide": > + // Ensure we're getting this event from the window the highlighter is handling. Rephrase the comment.
Attachment #8837583 - Flags: review?(gl) → review+
Pushed by mferretti@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/b1b91f7861ec ensure the pagehide event we received are for the window's highlighter; r=gl
The code is changed since the review, since the patches made some intermittent more frequent. I shown the changes to Gabriel directly and we discussed about that.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
This likely fixes also few intermittents that happens in bug 1321389; we should keep on eye on that.
[BugDay-20170329] The issue is no longer to be reproduced in Nightly 55.0a1
Doesn't look like an uplift candidate in late beta, wontfix for 54.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: