Like say SeaMonkey....

Try server job: https://treeherder.mozilla.org/#/jobs?repo=try&revision=549ce7592e25

Comment on attachment 8700334 [details] [diff] [review] Patch v1.0 Move the about page to /docshell and wrap with an #ifdef MOZ_DEVTOOLS Review of attachment 8700334 [details] [diff] [review]: ----------------------------------------------------------------- The concept seems fine, just want to see the define be less confusing. Also, I believe a review from a build peer is needed, perhaps :glandium since he reviewed other recent DevTools build things. ::: docshell/base/moz.build @@ +80,5 @@ > > if CONFIG['MOZ_TOOLKIT_SEARCH']: > DEFINES['MOZ_TOOLKIT_SEARCH'] = True > + > +if CONFIG['MOZ_DEVTOOLS'] == 'all': It seems simpler to follow if the DEFINE is set to the same value as the CONFIG, instead of becoming a boolean. ::: docshell/base/nsAboutRedirector.cpp @@ +51,5 @@ > { > "credits", "https://www.mozilla.org/credits/", > nsIAboutModule::URI_SAFE_FOR_UNTRUSTED_CONTENT > }, > +#ifdef MOZ_DEVTOOLS We should test that this set to the value "all", not just defined in general, once the moz.build is changed. That is the logic used to include the client files: https://dxr.mozilla.org/mozilla-central/source/devtools/moz.build#10

Changes since the last patch: 1. Set DEFINE to the same value as CONFIG > +if CONFIG['MOZ_DEVTOOLS'] == 'all': > + DEFINES['MOZ_DEVTOOLS'] = CONFIG['MOZ_DEVTOOLS'] 2. Test that MOZ_DEVTOOLS is set to the value "all" Also, Needs review by a build peer for the build-config changes.

Try server job looks green-ish. https://treeherder.mozilla.org/#/jobs?repo=try&revision=8be02a983ec8&group_state=expanded

Comment on attachment 8701783 [details] [diff] [review] Patch v2 Use Review of attachment 8701783 [details] [diff] [review]: ----------------------------------------------------------------- ::: docshell/base/moz.build @@ +80,5 @@ > > if CONFIG['MOZ_TOOLKIT_SEARCH']: > DEFINES['MOZ_TOOLKIT_SEARCH'] = True > + > +if CONFIG['MOZ_DEVTOOLS'] == 'all': It would make more sense to me to unconditionally copy the config value to the define, but let's see what :glandium thinks. ::: docshell/build/moz.build @@ +29,5 @@ > > if CONFIG['GNU_CXX']: > CXXFLAGS += ['-Wshadow'] > + > +if CONFIG['MOZ_DEVTOOLS'] == 'all': Same note as other moz.build.

Comment on attachment 8701783 [details] [diff] [review] Patch v2 Use Review of attachment 8701783 [details] [diff] [review]: ----------------------------------------------------------------- Why hardcode this in C++, when you could register the about:debugging url with a simple nsIAboutModule component. For example this is what I have for about:tabs in the tabstats addon: function AboutTabs() {} AboutTabs.prototype = { QueryInterface: XPCOMUtils.generateQI([Ci.nsIAboutModule]), classDescription: 'about:tabs', classID: Components.ID('{21dfe065-bb20-4f02-a969-e4bf3cf73e90}'), contractID: '@mozilla.org/network/protocol/about;1?what=tabs', newChannel: function(uri) { var channel = Services.io.newChannel('resource://tabstats/abouttabs.html', null, null); var securityManager = Cc['@mozilla.org/scriptsecuritymanager;1'].getService(Ci.nsIScriptSecurityManager); var principal = securityManager.getSystemPrincipal(uri); channel.originalURI = uri; channel.owner = principal; return channel; }, getURIFlags: function(uri) { return Ci.nsIAboutModule.ALLOW_SCRIPT; } }; const AboutTabsFactory = XPCOMUtils.generateNSGetFactory([AboutTabs])(AboutTabs.prototype.classID); function startup(aData, aReason) { Cm.registerFactory(AboutTabs.prototype.classID, AboutTabs.prototype.classDescription, AboutTabs.prototype.contractID, AboutTabsFactory); /* (...) */ }

(In reply to Mike Hommey [:glandium] (VAC: 31 Dec - 11 Jan) from comment #6) > Why hardcode this in C++, when you could register the about:debugging url > with a simple nsIAboutModule component. For example this is what I have for > about:tabs in the tabstats addon: 1. I have a version that does that too (see attachment). However this method requires applications to explicitly buy in to this by including the component in the relevant package-manifest.in 2. We are not an addon; we are part of Devtools. 3. nsAboutRedirector.cpp already exists for this very purpose. 4. The nsIAboutModule boilerplate is unnecessary bloat. The C++ patch is smaller and more compact.

Comment on attachment 8702015 [details] Alternative standalone nsAbout component. >browser/installer/package-manifest.in: > >#if MOZ_DEVTOOLS == all >@RESPATH@/components/nsAboutDebug.js >@RESPATH@/components/aboutDebug.manifest >#endif > >-------------------------------------------------- >moz.build: > >if CONFIG['MOZ_DEVTOOLS'] == 'all': > EXTRA_COMPONENTS += [ > nsAboutDebug.js, > aboutDebug.manifest, > ] > >-------------------------------------------------- >aboutDebug.manifest: > >component {5797adeb-1e00-4187-9c81-ed716aec2313} nsAboutDebug.js >contract @mozilla.org/network/protocol/about;1?what=debugging {5797adeb-1e00-4187-9c81-ed716aec2313} You can, in fact, abuse the system: you can actually register a component in a chrome directory. So you can, in fact put the following in a jar.mn in devtools: devtools.jar: % component {5797adeb-1e00-4187-9c81-ed716aec2313} devtools/nsAboutDebug.js % contract @mozilla.org/network/protocol/about;1?what=debugging {5797adeb-1e00-4187-9c81-ed716aec2313} nsAboutDebug.js And that will work (except if you package as jar instead of omni or flat) That said, I do agree that the boilerplate for nsIAboutModule is rather big, and it seems to me we could have something like the 'resource' registration in chrome manifests, but dedicated to about:, but that would be a followup.

Try results: https://treeherder.mozilla.org/#/jobs?repo=try&revision=d7eafd52d6e5&group_state=expanded > <RattyAway> So something like: > if CONFIG['MOZ_DEVTOOLS'] == 'all': > DEFINES['FOOBAR'] = True > <glandium> s/FOOBAR/MOZ_DEVTOOLS_ALL/

Comment on attachment 8702568 [details] [diff] [review] Patch v3 Minimalist patch usind existing nsAboutRedirector.cpp and MOZ_DEVTOOLS_ALL Review of attachment 8702568 [details] [diff] [review]: ----------------------------------------------------------------- Assuming you discussed this with :glandium already, then it works for me.

[bugday-20160323] Status: RESOLVED,FIXED -> UNVERIFIED Comments: STR: Not clear. Developer specific testing Component: Name Firefox Version 46.0b9 Build ID 20160322075646 Update Channel beta User Agent Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0 OS Windows 7 SP1 x86_64 Expected Results: Developer specific testing Actual Results: As expected

Why are you going through tons of fixed bugs and adding unnecessary information? What system are you going through?