Closed Bug 915448 Opened 8 years ago Closed 6 years ago

Error when last selected tool is not available for current toolbox target


(DevTools :: Framework, defect)

Not set


(firefox41 fixed)

Firefox 41
Tracking Status
firefox41 --- fixed


(Reporter: bbenvie, Assigned: past)



(1 file)

If you have a different set of tools in remote vs. local toolboxes, and the last selected tool isn't available when opening a new toolbox of the opposite target type (remote/local), an error is thrown.

* create a new panel that is only remote (returns `target.isRemote` from isTargetSupported)
* open a remote panel and select that panel
* open a new local toolbox

Because the last selected panel type is not compatible with the current toolbox's target, an error is thrown:

TypeError: tab is null @ resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/framework/toolbox.js:544
For step two, I meant "open a remote toolbox".
Easier STR:

1. Open the netmonitor on a page and then close the toolbox
2. Open WebIDE and connect to Chrome with Valence
3. Try to debug a page on Chrome.

A coding exception was thrown and uncaught in a Task.

Full message: TypeError: tab is null
Full stack: Toolbox.prototype.selectTool@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/framework/toolbox.js:1131:5<@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/framework/toolbox.js:373:13
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:867:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:746:7
this.PromiseWalker.scheduleWalkerLoop/<@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:688:37
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:688:5
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:709:7
this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:671:7</<@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/framework/toolbox.js:330:36
Assignee: nobody → past
Often a notification appears in WebIDE that the RDP connection has timed out (even though you can later select one of the available panels).
Simple fix for a bug that I always need to avoid hitting when I'm giving a talk.
Attachment #8604015 - Flags: review?(bgrinstead)
Comment on attachment 8604015 [details] [diff] [review]
The toolbox should expect the previously selected tool to be unavailable

Review of attachment 8604015 [details] [diff] [review]:

Looks good, and I think we can also remove some code from the constructor.  r=me with that change

::: browser/devtools/framework/toolbox.js
@@ +371,5 @@
>        this._pingTelemetry();
> +      // This check needs to happen after the target is remoted, otherwise we
> +      // could have done it in the toolbox constructor (bug 1072764).
> +      let toolDef = gDevTools.getToolDefinition(this._defaultToolId);

There is very similar code in the constructor (that's checking only if the toolDef exists and setting it to webconsole if not):

Looks like that condition can be removed and handled here with

`if (!toolDef || !toolDef.isTargetSupported(this._target)) {}`
Attachment #8604015 - Flags: review?(bgrinstead) → review+
That's what I tried and unfortunately it doesn't work without bug 1072764. This is why I put in the comment so we can remember to move it to the constructor when it's feasible to do so.
We discussed this on IRC and I realized what Brian suggested is different from what I initially thought and makes a lot of sense.
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.