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

Status: RESOLVED FIXED

(DevTools :: Inspector, defect)

Description

[Patrick Brosset <:pbro>]

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

Comment 1

[Patrick Brosset <:pbro>]

Paul, do you think we could use the BoxModelHighlighter here? I guess we could attach it to the top-most parent window.

Comment 2

[Paul Rouget [:paul]]

Maybe :) Might need to tweak how the highlighter is attached.

Comment 3

[Patrick Brosset <:pbro>]

Cool, I'll try that!

Comment 4

[Patrick Brosset <:pbro>]

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.

Comment 5

[Patrick Brosset <:pbro>]

Attachment: bug959076-browser-toolbox-can-inspect-browser-chrome.patch

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.

Comment 6

[Patrick Brosset <:pbro>]

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

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bed829df1460

Comment 8

[Luke]

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 :(