When contribution to devtools/client/json-viewer and hitting the reload shortcut, nothing happens. At least visually, the devtools actually reloads but you have to reload the tab to see your changes. Ideally, the document would just reload to see the changes. At least we should have a notification somehow that it got reloaded to then manually reload the tab.

This patch moves the reload logic to the addon. I don't think this code was very well hosted in Loader.jsm. This has nothing to do with the loader and is more about more highlevel devtools tricks. May be that's something to be put in devtools-browser?

Comment on attachment 8717949 [details] [diff] [review] patch v1 Review of attachment 8717949 [details] [diff] [review]: ----------------------------------------------------------------- This patch feel like surgery, but it provides a good experience while using the addon. I haven't had time to see how we could possibly convert gDevToolsBrowser to use Addon APIs. But I imagine this could would (almost) go away if I manage to do that. I'm piling up on top of that patch in bug 1241050, first attachment, to be able to reload gcli.

s/first attachment/second attachment.

Comment on attachment 8717949 [details] [diff] [review] patch v1 Review of attachment 8717949 [details] [diff] [review]: ----------------------------------------------------------------- Overall makes sense, but looks like still something to fix. ::: devtools/bootstrap.js @@ +105,5 @@ > + } > + }, false); > + } > + } else if (windowtype === "devtools:webide") { > + dump("reload window: "+windowtype+"\n"); Remove dump @@ +111,5 @@ > + } else if (windowtype === "devtools:webconsole") { > + // Browser console document can't just be reloaded. > + // HUDService is going to close it on unload. > + // Instead we have to manually toggle it. > + dump("reload browser console\n"); Remove dump @@ +127,5 @@ > + // Wait for a second before opening the toolbox to avoid races > + // between the old and the new one. > + let {setTimeout} = Cu.import("resource://gre/modules/Timer.jsm", {}); > + setTimeout(() => { > + let { gBrowser } = window; I get `window is not defined` here when I try this, and the toolbox never reopens.

Hum... there was even more issues. I ended up focusing on jsonview, about:debugging and gcli reload and forgot to test simple toolbox contribution. I wish I could write a test for this, but that looks even more challenging than testing the browser toolbox :/ The main issue is that tests are run without the source tree available. So. No reload addon. And no source to modify. I could somehow ship the addon as support-files, but what about the sources? It will break if the addon doesn't live within the whole sources. There is so many things that would be so much easier if tests were run with the source tree!!!

Comment on attachment 8718578 [details] [diff] [review] patch v2 Review of attachment 8718578 [details] [diff] [review]: ----------------------------------------------------------------- Great, this version works well! Unfortunately, I am not sure about testing it. I also noticed that with the add-on applied, the debugger show no sources, and many errors in Browser Console: TypeError: 'get window' called on an object that does not implement interface Window. react-dev.js:19569:0 ReferenceError: Task is not defined event-listeners.js:52:0 TypeError: DebuggerView is undefined workers-view.js:52:24 TypeError: DebuggerController is undefined variable-bubble-view.js:18:3 TypeError: DebuggerController is undefined watch-expressions-view.js:18:3 TypeError: DebuggerController is undefined global-search-view.js:18:3 TypeError: DebuggerController is undefined toolbar-view.js:19:3 TypeError: DebuggerView is undefined options-view.js:215:24 TypeError: DebuggerController is undefined stack-frames-view.js:18:3 TypeError: DebuggerView is undefined stack-frames-classic-view.js:140:39 TypeError: DebuggerController is undefined filter-view.js:19:3 TypeError: this._view is undefined I think this is case from before this patch as well. Is it a known issue? devtools.loader.hotreload is set to false, but maybe it interferes with the base BrowserLoader? The memory tool appears to work though, and also uses BrowserLoader, so maybe it's specific to debugger.

(In reply to J. Ryan Stinnett [:jryans] (use ni?) from comment #8) > Comment on attachment 8718578 [details] [diff] [review] > patch v2 > > Review of attachment 8718578 [details] [diff] [review]: > ----------------------------------------------------------------- > > Great, this version works well! Unfortunately, I am not sure about testing > it. You should see stuff being automatically reloaded *with* your change applied when hitting the magic key stroke. for example, if you modify webconsole.xul, you should see the browser toolbox being reloaded with your change. Now it isn't just the regular web toolbox that is reloaded, but any devtools related UI (web toolbox, browser console, json views, about:debugging, webide, developer toolbar) > I also noticed that with the add-on applied, the debugger show no sources, > and many errors in Browser Console: Yes, it looks unrelated. I need to take a look at that in bug 1248609.

https://hg.mozilla.org/integration/fx-team/rev/4db039bf7d4e2fca6981ff414e87c96fa7d2e1ec Bug 1247270 - Ensure reloading every devtools-related documents when hitting the reload shortcut. r=jryans