Closed Bug 959076 Opened 11 years ago Closed 11 years ago

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


(DevTools :: Inspector, defect)

Not set


(Not tracked)

Firefox 29


(Reporter: pbro, Assigned: pbro)



(1 file)

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:

  Message: TypeError: browser is undefined
    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:
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:

Fixed in fx-team:
Whiteboard: [fixed-in-fx-team]
Closed: 11 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.