[highlighter] Inspect element doesn't work with the browser toolbox

RESOLVED FIXED in Firefox 29

Status

defect
RESOLVED FIXED
6 years ago
Last year

People

(Reporter: pbro, Assigned: pbro)

Tracking

unspecified
Firefox 29
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

Since bug 916443 is fixed, the highlighter doesn't seem to work correctly with the browser toolbox.
Highlighter on hover of markup-view nodes works fine, but using the "pick an element from the page" button doesn't work at all. 
The following JS error is thrown:

console.error: 
  Message: TypeError: browser is undefined
  Stack:
    HighlighterActor<._startPickerListeners@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/highlighter.js:188
HighlighterActor<.pick<@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/actors/highlighter.js:167
actorProto/</handler@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/protocol.js:906
DSC_onPacket@resource://gre/modules/commonjs/toolkit/loader.js -> resource://gre/modules/devtools/server/main.js:1025
DT__processIncoming/<@resource://gre/modules/devtools/dbg-client.jsm -> resource://gre/modules/devtools/server/transport.js:201
makeInfallible/<@resource://gre/modules/devtools/DevToolsUtils.jsm -> resource://gre/modules/devtools/DevToolsUtils.js:76
Paul, do you think we could use the BoxModelHighlighter here? I guess we could attach it to the top-most parent window.
Flags: needinfo?(paul)
Maybe :) Might need to tweak how the highlighter is attached.
Flags: needinfo?(paul)
Cool, I'll try that!
Assignee: nobody → pbrosset
The fix was an easy one. See it working in this video: http://www.youtube.com/watch?v=RxU07RBKABw
Will send a patch asap.
However, using the BoxModelHighlighter isn't going to be an option, we'll have to stick to the SimpleOutlineHighlighter (dashed red outline). There are several reasons to this: one is that the highlighter XUL markup would be inserted in the currently inspected tree, so would be visible, an other is that it's pretty complex to make it be in front of every other elements.
It might be feasible, but it'll need its own bug if we really want to do it.
Also, I'm partly convinced that at some stage we'll have to drop the XUL-based highlighter and come up with a different way of drawing the box-model that works the same across all devices.
The only problem that prevented the inspect button from working was that mouse events weren't listened to on the right object in case of the browser toolbox.
This is now fixed.
Attachment #8360357 - Flags: review?(fayearthur)
Attachment #8360357 - Flags: review?(fayearthur) → review+
Thanks Heather for the quick review.
btw, I forgot to mention the try build URL when attaching my patch earlier: https://tbpl.mozilla.org/?tree=Try&rev=f686f8bc834f

Fixed in fx-team: https://hg.mozilla.org/integration/fx-team/rev/bed829df1460
Whiteboard: [fixed-in-fx-team]
https://hg.mozilla.org/mozilla-central/rev/bed829df1460
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 29
Does this mean it won't be fixed til Firefox 29?? This used to be working Firefox 20-something, I used to inspect the web-developer toolbox, from the Browser Toolbox. Now in 33beta I can't inspect the inspector :(
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.