We revived devtools.loader.srcdir and Loader.jsm recently. It allows to hot-reload a bunch of devtools resources, but not all. Also, it is very specific to devtools and this workflow can't be applied to any other Firefox resources. The big file moved :jryans did alllows us to simplify hot-reload feature by using an addon instead. We wouldn't bundled devtools as an addon. Instead we would just register xul content, skin and locales via an addon chrome.manifest. There wouldn't be any magic done in Loader.jsm. The addon is going to remap all root URL used by devtools to a local mozilla-central checkout transparently. This would have the benefit to not introduce a different codepath/loader/whatever between running firefox from a local checkout versus released package. The contribution workflow is done to be: * Install a Firefox Nightly * checkout mozilla-central with the same revision used to build this Nightly * Register the addon (that lives in m-c checkout) via Firefox UI, bug 1221141 * Play with devtools * Hit a keystroke (CTRL+ALT+R?) to hot-reload devtools * Play with devtools with sources being hot-reloaded * If you reload firefox, devtools are automatically going to be using local checkout sources. * If you want to stop mapping to local sources, disable the addon.

A wip patch to expose how simple this story is. This is just install.rdf and chrome.manifest files, with minor tweaks to hot reload from the addon.

Rebased after the last themes URL change.

I think it is time to proceed. Few things about this: - I tweaked gDevTools to figure out when a toolbox is opened in order to reopen it if we are hitting the reload shortcut on a window with an opened toolbox - The addon includes a omniscient key listener that can intercept key shortcut from any window, I would like to suggest moving this to about:debugging, so that we can have such trick to reload addons for all addon developers! There is still various followups to ensure everything reload, like: - bug 1183280, get rid of all preprocessed file, who appear to break stuff with this addon - bug 1232290, which break the editor - bug 1188405 or similar, to allow reloading gDevTools itself - bug 1185460 to allow keeping the devtools addon installed even after firefox reboot - bug 1217472 or similar, to cleanup Loader.jsm to use only one codepath and stop supporting loader path Note that I started writing a single page to guide contribution from nothing to check a patch in: https://public.etherpad-mozilla.org/p/devtools-contribution-workflow This includes the addon workflow that doesn't involve building firefox nor mach.

Also I plan to setup a small group to dogfood this new workflow and improve it based on feedback. I think you, brian and patrick are good candidates as well as new hires. But I need to checkin something first.

Comment on attachment 8698460 [details] [diff] [review] patch v3 Review of attachment 8698460 [details] [diff] [review]: ----------------------------------------------------------------- Well, I think the idea is interesting to explore. But, it did not seem to work for me. Hopefully we can figure out why! ::: devtools/bootstrap.js @@ +10,5 @@ > + > +// Helper to listen to a key on all windows > +function MultiWindowKeyListener({ key, ctrlKey, altKey, callback }) { > + let keyListener = function (event) { > + if (event.ctrlKey == !!ctrlKey && I wasn't able to press a key combo that actually met all these rules at once. I watched in Browser Toolbox, but `callback` was never called. I assume I am supposed to press Ctrl-Alt-R? @@ +71,5 @@ > + let reloadToolbox = false; > + if (isBrowser && top.gDevToolsBrowser.hasToolboxOpened) { > + reloadToolbox = top.gDevToolsBrowser.hasToolboxOpened(top); > + } > + dump("Reload devtools. (reload-toolbox:"+reloadToolbox+")\n"); Nit: devtools -> DevTools @@ +83,5 @@ > +} > + > +let listener; > +function startup() { > + dump("Devtools addon started.\n"); Nit: Devtools -> DevTools ::: devtools/install.rdf @@ +8,5 @@ > +<RDF xmlns="http://www.w3.org/1999/02/22-rdf-syntax-ns#" > + xmlns:em="http://www.mozilla.org/2004/em-rdf#"> > + <Description about="urn:mozilla:install-manifest" > + em:id="devtools@mozilla.org" > + em:name="Devtools" Well, this should at the very least be "DevTools". But, it might be better as a longer name. "Developer Tools (local version)"? Or some other phrase that means "not the regular tools"? It seems worthwhile to signify a special mode, as opposed to the built-in tools.

Comment on attachment 8698460 [details] [diff] [review] patch v3 Review of attachment 8698460 [details] [diff] [review]: ----------------------------------------------------------------- ::: devtools/install.rdf @@ +8,5 @@ > +<RDF xmlns="http://www.w3.org/1999/02/22-rdf-syntax-ns#" > + xmlns:em="http://www.mozilla.org/2004/em-rdf#"> > + <Description about="urn:mozilla:install-manifest" > + em:id="devtools@mozilla.org" > + em:name="Devtools" You should also add the `em:description` field with a longer description of what is going on.

What's the reasoning for this living in an addon as opposed to something built in but that's preffed off by maybe the chrome debugging pref or a new one?

That was an issue specific to Mac. event.key was the "Registered" character instead of just "r"... It should work better with using keyCode.

(In reply to Brian Grinstead [:bgrins] from comment #7) > What's the reasoning for this living in an addon as opposed to something > built in but that's preffed off by maybe the chrome debugging pref or a new > one? The reasoning was that this work wasn't really specific to devtools. We end up using a set of tricks that would benefit to other teams working on Firefox frontend as well as addon developers. * The loader trick, with the srcdir pref was really specific to devtools. It was a hack. Hard to maintain and very often broken as it includes very different codepaths. I would like to get rid of this completely as soon as the addon workflow is ready. * Instead, we are using a chrome.manifest file to redirect all devtools URIs (chrome://devtools/xxx, resource://devtools/) to the local checkout. So that we don't have different codepaths in the loader. There is just this chrome.manifest file that redirects everything. Then we just have to ensure destroying the old loader and spawn a new one. * We could have done something in devtools to register this chrome.manifest, but we would have been working around the fact that working on addons sucks... * It would have been unfair to all addon developers who are using the same feature but have to do many tricks to get their code working in Firefox. So instead I'm trying to improve the workflow for everyone. * With this chrome.manifest, devtools looks exactly like pdf.js and shumway. We could all reuse the same workflow to work on Firefox features. And anytime we improve something, all teams win. I hope system addons are going to also follow this track. * One example is the magic shortcut. I would like to include this in about:debugging to allow doing this for any add-on. * So far, I'm not trying to package devtools as an addon, but that work would certainly help us doing it if we want to go that way (DevTools as system add-on??)

Something about the fonts or styles used forces CodeMirror to use serif fonts instead of monospace.

Comment on attachment 8700564 [details] [diff] [review] patch v4 Review of attachment 8700564 [details] [diff] [review]: ----------------------------------------------------------------- Check attachment 8701245 [details], there seems to be some kind of font issue. Have you seen this? If you don't want to solve it now, we should at least have a follow up bug. I think the double reload issue should be resolved before landing, it seems to impede actually using this much at all. Anyway, the reloading does work now, at least the first time! ::: devtools/bootstrap.js @@ +69,5 @@ > + let top = getTopLevelWindow(event.view) > + let isBrowser = top.location.href.includes("/browser.xul") && top.gDevToolsBrowser; > + let reloadToolbox = false; > + if (isBrowser && top.gDevToolsBrowser.hasToolboxOpened) { > + reloadToolbox = top.gDevToolsBrowser.hasToolboxOpened(top); Something about this seems to fail if you reload twice in a row: 1. Open toolbox 2. Press Ctrl-Alt-R 3. Toolbox reloads, log says "Reload DevTools. (reload-toolbox:true)" 4. Press Ctrl-Alt-R again 5. Toolbox disappears, log says "Reload DevTools. (reload-toolbox:false)" ::: devtools/install.rdf @@ +8,5 @@ > +<RDF xmlns="http://www.w3.org/1999/02/22-rdf-syntax-ns#" > + xmlns:em="http://www.mozilla.org/2004/em-rdf#"> > + <Description about="urn:mozilla:install-manifest" > + em:id="devtools@mozilla.org" > + em:name="Developer Tools (local version)" Please add a longer-form em:description field as well.

Comment on attachment 8702536 [details] [diff] [review] Introduce an addon to easily work on devtools codebase. r=jryans Review of attachment 8702536 [details] [diff] [review]: ----------------------------------------------------------------- - with the long description - fixed the double reload. I had to tweak hasToolboxOpened. There is some state mixup between the multiple loader instances living around gDevTools singleton. (hopefully that is going to be fixed by bug 1188405) - the font issue is related to bug 1183280, I'll try to move forward on that. Waiting for Jan 4 to toggle to r? flag ;)

Comment on attachment 8702536 [details] [diff] [review] Introduce an addon to easily work on devtools codebase. r=jryans Review of attachment 8702536 [details] [diff] [review]: ----------------------------------------------------------------- Great, double reload does seem fixed here as well. This seems ready to land so you can iterate further with some more testers.

https://hg.mozilla.org/integration/fx-team/rev/1850e2691a107ed4cd99cae0950fe8af0ce48433 Bug 1225108 - Introduce an addon to easily work on devtools codebase. r=jryans