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)
DevTools
Inspector
Tracking
(firefox51 wontfix, firefox52 wontfix, firefox53 wontfix, firefox54 wontfix, firefox55 verified)
RESOLVED
FIXED
Firefox 55
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.
Comment 1•8 years ago
|
||
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).
status-firefox51:
--- → affected
status-firefox52:
--- → affected
status-firefox53:
--- → affected
status-firefox54:
--- → affected
Component: Untriaged → Developer Tools: Inspector
Assignee | ||
Comment 2•8 years ago
|
||
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
Blocks: devtools/highlighters
Comment hidden (mozreview-request) |
Assignee | ||
Comment 5•8 years ago
|
||
Note: this patch is built on top of bug 1312103's patch.
Assignee | ||
Updated•8 years ago
|
Comment 6•8 years ago
|
||
mozreview-review |
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 hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Assignee | ||
Comment 9•8 years ago
|
||
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 11•8 years ago
|
||
mozreview-review |
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+
Comment hidden (mozreview-request) |
Comment hidden (mozreview-request) |
Comment 14•8 years ago
|
||
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
Assignee | ||
Comment 15•8 years ago
|
||
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.
Comment 16•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Assignee | ||
Comment 17•8 years ago
|
||
This likely fixes also few intermittents that happens in bug 1321389; we should keep on eye on that.
Assignee | ||
Updated•8 years ago
|
Comment 18•8 years ago
|
||
[BugDay-20170329]
The issue is no longer to be reproduced in Nightly 55.0a1
Comment 19•8 years ago
|
||
Doesn't look like an uplift candidate in late beta, wontfix for 54.
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•