817385 - [toolbox] select tool when used from commands and inspect context menu

Status: RESOLVED WORKSFORME

(DevTools :: General, defect, P1)

Description

[Girish Sharma [:Optimizer]]

or otherwise, everywhere where this function is used, should not assume that this function will also select the tool.
For example in 

http://mxr.mozilla.org/mozilla-central/source/browser/base/content/nsContextMenu.js#420

STR :
1. Right click on page ans select inspect element.
2. Toolbox appears with the Inspect tool selected.
3. Switch to Style Editor tool
4. Rigth click on the page again and select inspect.
5. The highlighter appears, but inspect tool is not selected.

Comment 1

[Paul Rouget [:paul]]

API-wise, `getPanelForTarget` is not supposed to handle any UI operation.

So I think we should just select the inspector in nsContextMenu.

Comment 2

[Girish Sharma [:Optimizer]]

Okay, and I think more places also, let me find out.

Comment 3

[Girish Sharma [:Optimizer]]

So I found a few more places too :

http://mxr.mozilla.org/mozilla-central/source/browser/devtools/tilt/TiltVisualizer.jsm#178

http://mxr.mozilla.org/mozilla-central/source/browser/devtools/commandline/CmdBreak.jsm multiple places

http://mxr.mozilla.org/mozilla-central/source/browser/devtools/commandline/CmdDbg.jsm multiple places

http://mxr.mozilla.org/mozilla-central/source/browser/devtools/inspector/CmdInspect.jsm#37

http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/webconsole/WebConsoleUtils.jsm#1575

and many more tests, but I am not sure if tests need them actually or not.

Comment 4

[Paul Rouget [:paul]]

(In reply to Girish Sharma [:Optimizer] from comment #3)
> So I found a few more places too :
> 
> http://mxr.mozilla.org/mozilla-central/source/browser/devtools/tilt/
> TiltVisualizer.jsm#178

No need to call select here.

> http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/webconsole/
> WebConsoleUtils.jsm#1575

No need to call select here.

> http://mxr.mozilla.org/mozilla-central/source/browser/devtools/commandline/
> CmdBreak.jsm multiple places

I don't know about this one.

> http://mxr.mozilla.org/mozilla-central/source/browser/devtools/commandline/
> CmdDbg.jsm multiple places

I don't know about this one.

> http://mxr.mozilla.org/mozilla-central/source/browser/devtools/inspector/
> CmdInspect.jsm#37

Needed here.

We should not change the role of getPanel, but just add a call to select.

Comment 5

[Girish Sharma [:Optimizer]]

yes, I agree that getPanelForTarget should not select, as sometimes it might be needed to just refer and not select.

as for cmd Break and dbg, it is needed to focus the debugger onl yin case of dbg command, and I guess break command only adds a breakpoint, and does not opens the debugger.

Comment 6

[Girish Sharma [:Optimizer]]

I can no longer reproduce this. I think this was fixed by bug 813031