The inspector in the browser toolbox doesn't highlight and pick elements anymore

RESOLVED FIXED in Firefox 52

Status

()

Firefox
Developer Tools: Inspector
P1
normal
RESOLVED FIXED
a year ago
a year ago

People

(Reporter: pbro, Assigned: pbro)

Tracking

({regression})

Trunk
Firefox 52
regression
Points:
---

Firefox Tracking Flags

(firefox49 unaffected, firefox50 unaffected, firefox51 unaffected, firefox52 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

(Assignee)

Description

a year ago
STR:
- open Firefox
- open the Browser Toolbox
- in the inspector panel, move your mouse over elements
==> the corresponding elements aren't being highlighted in the main Firefox window as you do this, and exceptions are thrown in the browser console:
"Exception {  }"

- click on the "pick" button
- move your mouse over elements inside the main Firefox window
==> they don't get highlighted, and you can't select them, and exceptions are thrown in the browser console:
"NS_ERROR_XPC_BAD_CONVERT_JS: Could not convert JavaScript argument arg 0 [inIDOMUtils.removePseudoClassLock]"
(Assignee)

Comment 1

a year ago
Inside devtools\server\actors\highlighters\utils\markup.js if you replace

exports.removePseudoClassLock = (...args) => {
  lazyContainer.DOMUtils.removePseudoClassLock(...args);

with

exports.removePseudoClassLock = (...args) => {
  try {
    lazyContainer.DOMUtils.removePseudoClassLock(...args);
  } catch (e) {
    // ¯\_(ツ)_/¯
  }
};

the highlighting and selecting features work again, but the "Exception {  }" errors are still thrown.

So at least part of this is related to removePseudoClassLock, but I have no idea what changed in the recent past that could have broken this.
(Assignee)

Comment 2

a year ago
Regression introduced in bug 1304679.
Before that bug, the highlighter actor used to check if the node being highlighted was valid. After that bug, it doesn't anymore, but sub-highlighter classes do on their own.
I just forgot about the SimpleOutline highlighter, so it probably just needs to check if the node being passed can be highlighted or not.
Blocks: 1304679
(Assignee)

Updated

a year ago
Assignee: nobody → pbrosset
Status: NEW → ASSIGNED
Priority: -- → P1
Comment hidden (mozreview-request)

Comment 4

a year ago
mozreview-review
Comment on attachment 8794814 [details]
Bug 1305401 - SimpleOutlineHighlighter checks for node validity before highlighting;

https://reviewboard.mozilla.org/r/81116/#review79694

LGTM, thanks!

::: devtools/server/actors/highlighters/simple-outline.js:7
(Diff revision 1)
>   * License, v. 2.0. If a copy of the MPL was not distributed with this
>   * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  "use strict";
>  
> -const { installHelperSheet,
> +const { installHelperSheet, isNodeValid,

nit: one import per line? (same as in box-model.js)
Attachment #8794814 - Flags: review?(jdescottes) → review+
Comment hidden (mozreview-request)

Comment 6

a year ago
Pushed by pbrosset@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/99b2cfda036e
SimpleOutlineHighlighter checks for node validity before highlighting; r=jdescottes

Comment 7

a year ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/99b2cfda036e
Status: ASSIGNED → RESOLVED
Last Resolved: a year ago
status-firefox52: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 52

Updated

a year ago
status-firefox49: --- → unaffected
status-firefox50: --- → unaffected
status-firefox51: --- → unaffected
Duplicate of this bug: 1234713
Version: unspecified → Trunk
You need to log in before you can comment on or make changes to this bug.