Toolbox no longer opens after devtools.reload()

RESOLVED FIXED in Firefox 42

Status

defect
RESOLVED FIXED
4 years ago
a year ago

People

(Reporter: past, Assigned: ochameau)

Tracking

(Blocks 2 bugs)

Trunk
Firefox 42
Dependency tree / graph

Firefox Tracking Flags

(firefox41 affected, firefox42 fixed)

Details

Attachments

(3 attachments)

STR:

1. Open GCLI and enter "tools builtin"
2. Open the toolbox.
3. The toolbox opens an empty iframe and the following errors appear in the console:

JavaScript error: chrome://browser/content/devtools/theme-switching.js, line 45: TypeError: newThemeDef is null
*************************
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:1136:5
Toolbox.prototype.open/<@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/framework/toolbox.js:378:13
TaskImpl_run@resource://gre/modules/Task.jsm:314:40
Handler.prototype.process@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:887:23
this.PromiseWalker.walkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:766:7
this.PromiseWalker.scheduleWalkerLoop/<@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:39
Promise*this.PromiseWalker.scheduleWalkerLoop@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:707:7
this.PromiseWalker.schedulePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:729:7
this.PromiseWalker.completePromise@resource://gre/modules/Promise.jsm -> resource://gre/modules/Promise-backend.js:687:7
Toolbox.prototype.open/</<@resource://gre/modules/commonjs/toolkit/loader.js -> resource:///modules/devtools/framework/toolbox.js:327:36

*************************
Actually the same thing happens when using |tools srcdir| and |tools reload|, so it's the devtools.reload() call that is messing things up.
Summary: Toolbox no longer opens after |tools builtin| → Toolbox no longer opens after devtools.reload()
Assignee

Comment 2

4 years ago
Posted patch patch v1Splinter Review
Might be regressed from bug 986841.
But may be not just that one.

There is two issues:
1) Unload process work fine and unregister module.
   main.js correctly unregister tools and they disappear from the menu/shortcuts.
   But during reload we prevent reloading main.js so that tools aren't re-registered.
   That's because of the _mainid check:
   http://mxr.mozilla.org/mozilla-central/source/toolkit/devtools/Loader.jsm#363
2) We are not calling main() again.
   I don't know if we used to do it, but we should.

Ideally we would move most of gDevtools.jsm to a regular module,
into main.js for ex, or new modules as dependency to main.js.
Then, I think it would be up to Loader.jsm to call main()
and Loader.jsm may be the only one JSM we end up with!
Also, this is quite too magical to merge main.js exports onto Loader.jsm `devtools`.
It may be clearer and more explicit to just expose main.js exports on `devtools`
and may be expose `require` as new Loader.jsm symbol.

So this patch isn't perfect but at least it make it work again,
so that we can start working on various bugs related to live coding on devtools codebase!
Assignee

Comment 3

4 years ago
We also need to fix promise require path...
Here is a first alternative. As the overall plan is to only make devtools codebase hackable,
I think it makes more sense to make everyhing else being loaded from firefox ressources.
So here, we load promise module from firefox.
Assignee

Comment 4

4 years ago
Otherwise, we can just fix the path in the meantime.
This is just a typo...
Assignee

Updated

4 years ago
Attachment #8639728 - Flags: review?(jwalker)
Attachment #8639728 - Flags: review?(jwalker) → review+
Assignee

Comment 7

4 years ago
Comment on attachment 8639731 [details] [diff] [review]
fix promises - Always load builtin Promise module

Ryan, I landed just the typo for now, but what do you think about that?
Attachment #8639731 - Flags: feedback?
Assignee

Comment 8

4 years ago
About comment 2, I filled bug 1188405 and bug 1188401.
Assignee

Updated

4 years ago
Assignee: nobody → poirot.alex
https://hg.mozilla.org/mozilla-central/rev/524519fbf4c7
https://hg.mozilla.org/mozilla-central/rev/7b878026755d
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Comment on attachment 8639731 [details] [diff] [review]
fix promises - Always load builtin Promise module

I think the alternative seems good, since we're only focused on reloading DevTools modules anyway, and promise is not one of those.
Attachment #8639731 - Flags: feedback? → feedback+

Updated

a year ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.