Closed Bug 576900 Opened 10 years ago Closed 10 years ago

Make suite XPCOM components use new manifests and data tables

Categories

(SeaMonkey :: Build Config, defect)

defect
Not set
blocker

Tracking

(blocking-seamonkey2.1 a3+)

RESOLVED FIXED
Tracking Status
blocking-seamonkey2.1 --- a3+

People

(Reporter: neil, Assigned: neil)

References

()

Details

Attachments

(8 files, 1 obsolete file)

+++ This bug was initially created as a clone of Bug #575740 +++

Bug 568691 is changing XPCOM registration a lot, we should prepare suite patches to deal with that so we don't break.
Attachment #455979 - Flags: review?(stefanh)
Attachment #455979 - Flags: review?(mnyromyr)
Attachment #455979 - Flags: review?(kairo)
Attachment #455979 - Flags: review?(iann_bugzilla)
Attachment #455979 - Flags: review?(bugspam.Callek)
Attachment #455979 - Flags: review?(kairo)
Attachment #455979 - Flags: review?(stefanh)
Attachment #455979 - Flags: review?(mnyromyr)
Attachment #455979 - Flags: review?(iann_bugzilla)
Attachment #455979 - Flags: review?(bugspam.Callek)
Attachment #455979 - Flags: review+
Comment on attachment 455979 [details] [diff] [review]
C++ fixes [Checkin: Comment 4]

>+static const mozilla::Module::CIDEntry kSuiteCIDs[] = 

Note that in all cases here, the second argument you pass is "false". While this argument is currently unused it is intended to be forced in the future to be true|false for getService() and createInstance() sanity-checking. Not blocking on that though.

>+  { "@mozilla.org/embeddor.implemented/bookmark-charset-resolver;1", &kNS_BOOKMARKS_SERVICE_CID },

nit: #define this magic contract.
(In reply to comment #2)
>(From update of attachment 455979 [details] [diff] [review])
>>+  { "@mozilla.org/embeddor.implemented/bookmark-charset-resolver;1", &kNS_BOOKMARKS_SERVICE_CID },
>nit: #define this magic contract.
KaiRo will probably get rid of it anyway...
Comment on attachment 455979 [details] [diff] [review]
C++ fixes [Checkin: Comment 4]

Pushed changeset 7ab1997b0fc5 to comm-central.
* Tested that -help output includes the -setDefault parameters
* Tested that Application is available in the Error console
* Tested that previewing a feed works

Note that if you have managed to complete a build then you may need to use -jsconsole to get anywhere and use Tools/Add-on Manager to open a browser. (Isn't it convenient that it does!)
Assignee: nobody → neil
Status: NEW → ASSIGNED
Attachment #456110 - Flags: review?(iann_bugzilla)
Attachment #456110 - Flags: review?(bugspam.Callek)
The nsBrowserContentHandler one looks weird until you realise that the uriloader tries to create an instance instead of a service. Firefox has to override the XPCOMUtils factory method, but it was easier to keep not using XPCOMUtils. I don't know why nsAboutAbout has the same service protection, none of the other abouts bother.
Attachment #456118 - Flags: review?(iann_bugzilla)
Attachment #456118 - Flags: review?(bugspam.Callek)
Attachment #456110 - Flags: review+
Comment on attachment 456110 [details] [diff] [review]
feed shell smile JS component fixes [Checkin: Comment 26]

>   newURI: function newURI(spec, originalCharset, baseURI) {
>+dump("FeedConverter.newURI(" + spec + ", ...)\n");

Whops?

>-EXTRA_PP_COMPONENTS = \
>+EXTRA_COMPONENTS = \

Did not (yet) check if these can truely be dropped from being preprocessed.

>diff --git a/suite/feeds/src/SuiteFeeds.manifest 

Did not yet check for typo's or mistakes in this file.

>-  classDescription: WCCR_CLASSNAME,

Kill the const for this one

Other than those I listed above, (and my personal todo's) this looks good. (not clearing my review request though yet; since there is still stuff to review here). IanN if you beat me to the rest you can clear my remaining review.
Requesting r/a for CLOSED TREE
Attachment #456344 - Flags: review?
Comment on attachment 456344 [details] [diff] [review]
Fix typo GNOME not Gnome patch [Checkin: Comment 10]

request for r/a= for CLOSED TREE (only one person should be allowed to have kairo in their name!)
Attachment #456344 - Flags: review? → review?(kairo)
Comment on attachment 456344 [details] [diff] [review]
Fix typo GNOME not Gnome patch [Checkin: Comment 10]

http://hg.mozilla.org/comm-central/rev/cdc80e438f90
r/a=KaiRo via IRC
Attachment #456344 - Attachment description: Fix typo GNOME not Gnome patch → Fix typo GNOME not Gnome patch [Checkin: Comment 10]
Attachment #456344 - Flags: review?(kairo) → review+
Attachment #456110 - Flags: review?(iann_bugzilla) → review+
Attachment #456118 - Flags: review?(iann_bugzilla) → review+
Comment on attachment 456118 [details] [diff] [review]
browser JS component fixes [Checkin: Comment 26]

>-  lockFactory: function lockFactory(lock) {
>-    /* no-op */
>   }
Don't we want to keep lockFactory as we are in nsBrowserContentHandler.js

r=me with that addressed
Fix nsSuiteDownloadManagerUI.

Tested with  Mozilla/5.0 (Windows; Windows NT 6.1; WOW64; en-US; rv:2.0b2pre) Gecko/20100708 Lightning/1.1a1pre SeaMonkey/2.1a3pre
Attachment #456423 - Flags: review?(neil)
Attachment #456423 - Flags: feedback?(kairo)
Comment on attachment 456110 [details] [diff] [review]
feed shell smile JS component fixes [Checkin: Comment 26]

r+ pending my nits in c#7
Attachment #456110 - Flags: review?(bugspam.Callek)
Comment on attachment 456118 [details] [diff] [review]
browser JS component fixes [Checkin: Comment 26]

We can just use IanN's review here I think
Attachment #456118 - Flags: review?(bugspam.Callek)
Comment on attachment 456423 [details] [diff] [review]
Fix nsSuiteDownloadManagerUI [Checkin: Comment 26]

Can't we just do nsSuiteCommon.manifest and fold all these components into it?
Comment on attachment 456423 [details] [diff] [review]
Fix nsSuiteDownloadManagerUI [Checkin: Comment 26]

That said, the DownloadManagerUI changes here look good, but I still would love to see it all done.
Attachment #456423 - Flags: feedback+
Comment on attachment 456423 [details] [diff] [review]
Fix nsSuiteDownloadManagerUI [Checkin: Comment 26]

I don't yet understand this new stuff, Callek's feedback and Neil's review are surely enough here.
Attachment #456423 - Flags: feedback?(kairo)
(In reply to comment #16)
> (From update of attachment 456423 [details] [diff] [review])
> That said, the DownloadManagerUI changes here look good, but I still would love
> to see it all done.

Ratty, do you intend to update this patch; or should I take up work building on top of this?
Callek, I'd rather Neil's patches (and maybe mine) land first otherwise you'd be doing extra work. Moving into nsSuiteCommon.manifest would probably require another review and I'd rather get the builds working again first.
Comment on attachment 456423 [details] [diff] [review]
Fix nsSuiteDownloadManagerUI [Checkin: Comment 26]

Lets see who can get to review this first.
Attachment #456423 - Flags: review?(iann_bugzilla)
Comment on attachment 456423 [details] [diff] [review]
Fix nsSuiteDownloadManagerUI [Checkin: Comment 26]

Callek says over IRC that he'll be working on an updated patch.
Attachment #456423 - Flags: review?(neil)
Attachment #456423 - Flags: review?(iann_bugzilla)
Comment on attachment 456423 [details] [diff] [review]
Fix nsSuiteDownloadManagerUI [Checkin: Comment 26]

Sorry for the bugspam. I misunderstood Callek.
Attachment #456423 - Flags: review?(iann_bugzilla)
Comment on attachment 456423 [details] [diff] [review]
Fix nsSuiteDownloadManagerUI [Checkin: Comment 26]

my turn for bugspam; I think we can use my feedback as a review here. a=Callek for whomever wants to land this
Attachment #456423 - Flags: review?(iann_bugzilla)
Attachment #456423 - Flags: review+
Attachment #456423 - Flags: feedback+
This builds on the "Fix nsSuiteDownloadManagerUI" patch, and still has a todo of the nsSiderbar.js component.

Which I hope to move to common/src and add to the component there really. But it needs XPCOMUtil love anyway, so leaving as a todo (for when I'm more awake)
Attachment #456663 - Flags: review?(neil)
Comment on attachment 456663 [details] [diff] [review]
SuiteCommon.manifest [Checkin: comment 31]

Whoever gets to it first...
Attachment #456663 - Flags: review?(iann_bugzilla)
Attachment #456110 - Attachment description: feed shell smile JS component fixes → feed shell smile JS component fixes [Checkin: Comment 26]
Attachment #456118 - Attachment description: browser JS component fixes → browser JS component fixes [Checkin: Comment 26]
Attachment #456423 - Attachment description: Fix nsSuiteDownloadManagerUI → Fix nsSuiteDownloadManagerUI [Checkin: Comment 26]
Attachment #455979 - Attachment description: C++ fixes → C++ fixes [Checkin: Comment 4]
> +  classID:          Components.ID("{4e6c1112-57b6-44ba-adf9-99fb573b0a30}")
I don't think we need all these spaces now.

> diff --git a/suite/common/src/nsSuiteDownloadManagerUI.manifest b/suite/common/src/nsSuiteCommon.manifest
> rename from suite/common/src/nsSuiteDownloadManagerUI.manifest
> rename to suite/common/src/nsSuiteCommon.manifest

I misunderstood what Callek meant. Sorry about that.
Comment on attachment 456663 [details] [diff] [review]
SuiteCommon.manifest [Checkin: comment 31]

># HG changeset patch
># Parent 7438e5a5f01a44cdcc11b9fe4312ed29532ea567
>
>diff --git a/suite/common/src/nsAboutCertError.js b/suite/common/src/nsAboutCertError.js
>--- a/suite/common/src/nsAboutCertError.js
>+++ b/suite/common/src/nsAboutCertError.js
>@@ -39,8 +39,6 @@
> 
> function AboutCertError() {}
> AboutCertError.prototype = {
>-  classDescription: "About Cert Error",
>-  contractID: "@mozilla.org/network/protocol/about;1?what=certerror",
>   classID: Components.ID("{b24861fb-e91a-40dd-886d-68d26a9586c7}"),
>   QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsIAboutModule]),
> 
>@@ -67,6 +65,4 @@
>   }
> };
> 
>-function NSGetModule(compMgr, fileSpec) {
>-  return XPCOMUtils.generateModule([AboutCertError]);
>-}
>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([AboutCertError]);
>diff --git a/suite/common/src/nsAboutSessionRestore.js b/suite/common/src/nsAboutSessionRestore.js
>--- a/suite/common/src/nsAboutSessionRestore.js
>+++ b/suite/common/src/nsAboutSessionRestore.js
>@@ -38,8 +38,6 @@
> 
> function AboutSessionRestore() { }
> AboutSessionRestore.prototype = {
>-  classDescription: "about:sessionrestore",
>-  contractID: "@mozilla.org/network/protocol/about;1?what=sessionrestore",
>   classID: Components.ID("{a03c813e-abe8-41de-8d0c-5aa85f877696}"),
>   QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsIAboutModule]),
> 
>@@ -56,6 +54,4 @@
>   }
> };
> 
>-function NSGetModule(compMgr, fileSpec) {
>-  return XPCOMUtils.generateModule([AboutSessionRestore]);
>-}
>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([AboutSessionRestore[);
>diff --git a/suite/common/src/nsSessionStartup.js b/suite/common/src/nsSessionStartup.js
>--- a/suite/common/src/nsSessionStartup.js
>+++ b/suite/common/src/nsSessionStartup.js
>@@ -246,17 +246,8 @@
>   QueryInterface : XPCOMUtils.generateQI([Components.interfaces.nsIObserver,
>                                           Components.interfaces.nsISupportsWeakReference,
>                                           Components.interfaces.nsISessionStartup]),
>-  classDescription: "Suite Session Startup Service",
>-  classID:          Components.ID("{4e6c1112-57b6-44ba-adf9-99fb573b0a30}"),
>-  contractID:       "@mozilla.org/suite/sessionstartup;1",
>-
>-  // get this contractID registered for certain categories via XPCOMUtils
>-  _xpcom_categories: [
>-    // make ourselves a startup observer
>-    { category: "app-startup", service: true }
>-  ]
>+  classID:          Components.ID("{4e6c1112-57b6-44ba-adf9-99fb573b0a30}")
> 
> };
> 
>-function NSGetModule(aCompMgr, aFileSpec)
>-  XPCOMUtils.generateModule([SessionStartup]);
>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([SessionStartup]);
>diff --git a/suite/common/src/nsSessionStore.js b/suite/common/src/nsSessionStore.js
>--- a/suite/common/src/nsSessionStore.js
>+++ b/suite/common/src/nsSessionStore.js
>@@ -139,8 +139,6 @@
> }
> 
> SessionStoreService.prototype = {
>-  classDescription: "Suite Session Store Service",
>-  contractID: "@mozilla.org/suite/sessionstore;1",
>   classID: Components.ID("{d37ccdf1-496f-4135-9575-037180af010d}"),
>   QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsISessionStore,
>                                          Components.interfaces.nsIDOMEventListener,
>@@ -2880,5 +2878,4 @@
>   }
> };
> 
>-function NSGetModule(aComMgr, aFileSpec)
>-  XPCOMUtils.generateModule([SessionStoreService]);
>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([SessionStoreService]);
>diff --git a/suite/common/src/nsSuiteDownloadManagerUI.manifest b/suite/common/src/nsSuiteCommon.manifest
>rename from suite/common/src/nsSuiteDownloadManagerUI.manifest
>rename to suite/common/src/nsSuiteCommon.manifest
>--- a/suite/common/src/nsSuiteDownloadManagerUI.manifest
>+++ b/suite/common/src/nsSuiteCommon.manifest
>@@ -1,2 +1,16 @@
> component {08bbb4af-7bff-4b16-8ff7-d62f3ec5aa0c} nsSuiteDownloadManagerUI.js
> contract @mozilla.org/download-manager-ui;1 {08bbb4af-7bff-4b16-8ff7-d62f3ec5aa0c}
>+component {b24861fb-e91a-40dd-886d-68d26a9586c7} nsAboutCertError.js
>+contract @mozilla.org/network/protocol/about;1?what=certerror {b24861fb-e91a-40dd-886d-68d26a9586c7}
>+component {a03c813e-abe8-41de-8d0c-5aa85f877696} nsAboutSessionRestore.js
>+contract @mozilla.org/network/protocol/about;1?what=sessionrestore {a03c813e-abe8-41de-8d0c-5aa85f877696}
>+component {4e6c1112-57b6-44ba-adf9-99fb573b0a30} nsSessionStartup.js
>+contract @mozilla.org/suite/sessionstartup;1 {4e6c1112-57b6-44ba-adf9-99fb573b0a30}
>+category app-startup SessionStartup service,@mozilla.org/suite/sessionstartup;1
>+component {d37ccdf1-496f-4135-9575-037180af010d} nsSessionStore.js
>+contract @mozilla.org/suite/sessionstore;1 {d37ccdf1-496f-4135-9575-037180af010d}
>+component {bbbbe845-5a1b-40ee-813c-f84b8faaa07c} nsSuiteGlue.js
>+contract @mozilla.org/suite/suiteglue;1 {bbbbe845-5a1b-40ee-813c-f84b8faaa07c}
>+category app-startup nsSuiteGlue service,@mozilla.org/suite/suiteglue;1
>+component {450a13bd-0d07-4e5d-a9f0-448c201728b1} nsSuiteGlue.js
>+contract @mozilla.org/geolocation/prompt;1 {450a13bd-0d07-4e5d-a9f0-448c201728b1}
>diff --git a/suite/common/src/nsSuiteGlue.js b/suite/common/src/nsSuiteGlue.js
>--- a/suite/common/src/nsSuiteGlue.js
>+++ b/suite/common/src/nsSuiteGlue.js
>@@ -461,27 +461,18 @@
> 
> 
>   // for XPCOM
>-  classDescription: "SeaMonkey Suite Glue Service",
>   classID:          Components.ID("{bbbbe845-5a1b-40ee-813c-f84b8faaa07c}"),
>-  contractID:       "@mozilla.org/suite/suiteglue;1",
> 
>   QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsIObserver,
>                                          Components.interfaces.nsISupportsWeakReference,
>-                                         Components.interfaces.nsISuiteGlue]),
>+                                         Components.interfaces.nsISuiteGlue])
> 
>-  // get this contractID registered for certain categories via XPCOMUtils
>-  _xpcom_categories: [
>-    // make SuiteGlue a startup observer
>-    { category: "app-startup", service: true }
>-  ]
> }
> 
> function GeolocationPrompt() {}
> 
> GeolocationPrompt.prototype = {
>-  classDescription: "Geolocation Prompting Component",
>   classID:          Components.ID("{450a13bd-0d07-4e5d-a9f0-448c201728b1}"),
>-  contractID:       "@mozilla.org/geolocation/prompt;1",
> 
>   QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsIGeolocationPrompt]),
> 
>@@ -570,6 +561,4 @@
> };
> 
> //module initialization
>-function NSGetModule(aCompMgr, aFileSpec) {
>-  return XPCOMUtils.generateModule([SuiteGlue, GeolocationPrompt]);
>-}
>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([SuiteGlue, GeolocationPrompt]);
Attachment #456663 - Flags: review?(neil)
Attachment #456663 - Flags: review?(iann_bugzilla)
Attachment #456663 - Flags: review+
Comment on attachment 456663 [details] [diff] [review]
SuiteCommon.manifest [Checkin: comment 31]

># HG changeset patch
># Parent 7438e5a5f01a44cdcc11b9fe4312ed29532ea567
>
>diff --git a/suite/common/src/nsAboutCertError.js b/suite/common/src/nsAboutCertError.js
>--- a/suite/common/src/nsAboutCertError.js
>+++ b/suite/common/src/nsAboutCertError.js
>@@ -39,8 +39,6 @@
> 
> function AboutCertError() {}
> AboutCertError.prototype = {
>-  classDescription: "About Cert Error",
>-  contractID: "@mozilla.org/network/protocol/about;1?what=certerror",
>   classID: Components.ID("{b24861fb-e91a-40dd-886d-68d26a9586c7}"),
>   QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsIAboutModule]),
> 
>@@ -67,6 +65,4 @@
>   }
> };
> 
>-function NSGetModule(compMgr, fileSpec) {
>-  return XPCOMUtils.generateModule([AboutCertError]);
>-}
>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([AboutCertError]);
>diff --git a/suite/common/src/nsAboutSessionRestore.js b/suite/common/src/nsAboutSessionRestore.js
>--- a/suite/common/src/nsAboutSessionRestore.js
>+++ b/suite/common/src/nsAboutSessionRestore.js
>@@ -38,8 +38,6 @@
> 
> function AboutSessionRestore() { }
> AboutSessionRestore.prototype = {
>-  classDescription: "about:sessionrestore",
>-  contractID: "@mozilla.org/network/protocol/about;1?what=sessionrestore",
>   classID: Components.ID("{a03c813e-abe8-41de-8d0c-5aa85f877696}"),
>   QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsIAboutModule]),
> 
>@@ -56,6 +54,4 @@
>   }
> };
> 
>-function NSGetModule(compMgr, fileSpec) {
>-  return XPCOMUtils.generateModule([AboutSessionRestore]);
>-}
>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([AboutSessionRestore[);
>diff --git a/suite/common/src/nsSessionStartup.js b/suite/common/src/nsSessionStartup.js
>--- a/suite/common/src/nsSessionStartup.js
>+++ b/suite/common/src/nsSessionStartup.js
>@@ -246,17 +246,8 @@
>   QueryInterface : XPCOMUtils.generateQI([Components.interfaces.nsIObserver,
>                                           Components.interfaces.nsISupportsWeakReference,
>                                           Components.interfaces.nsISessionStartup]),
>-  classDescription: "Suite Session Startup Service",
>-  classID:          Components.ID("{4e6c1112-57b6-44ba-adf9-99fb573b0a30}"),
>-  contractID:       "@mozilla.org/suite/sessionstartup;1",
>-
>-  // get this contractID registered for certain categories via XPCOMUtils
>-  _xpcom_categories: [
>-    // make ourselves a startup observer
>-    { category: "app-startup", service: true }
>-  ]
>+  classID:          Components.ID("{4e6c1112-57b6-44ba-adf9-99fb573b0a30}")
> 
> };
> 
>-function NSGetModule(aCompMgr, aFileSpec)
>-  XPCOMUtils.generateModule([SessionStartup]);
>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([SessionStartup]);
>diff --git a/suite/common/src/nsSessionStore.js b/suite/common/src/nsSessionStore.js
>--- a/suite/common/src/nsSessionStore.js
>+++ b/suite/common/src/nsSessionStore.js
>@@ -139,8 +139,6 @@
> }
> 
> SessionStoreService.prototype = {
>-  classDescription: "Suite Session Store Service",
>-  contractID: "@mozilla.org/suite/sessionstore;1",
>   classID: Components.ID("{d37ccdf1-496f-4135-9575-037180af010d}"),
>   QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsISessionStore,
>                                          Components.interfaces.nsIDOMEventListener,
>@@ -2880,5 +2878,4 @@
>   }
> };
> 
>-function NSGetModule(aComMgr, aFileSpec)
>-  XPCOMUtils.generateModule([SessionStoreService]);
>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([SessionStoreService]);
>diff --git a/suite/common/src/nsSuiteDownloadManagerUI.manifest b/suite/common/src/nsSuiteCommon.manifest
>rename from suite/common/src/nsSuiteDownloadManagerUI.manifest
>rename to suite/common/src/nsSuiteCommon.manifest
>--- a/suite/common/src/nsSuiteDownloadManagerUI.manifest
>+++ b/suite/common/src/nsSuiteCommon.manifest
>@@ -1,2 +1,16 @@
> component {08bbb4af-7bff-4b16-8ff7-d62f3ec5aa0c} nsSuiteDownloadManagerUI.js
> contract @mozilla.org/download-manager-ui;1 {08bbb4af-7bff-4b16-8ff7-d62f3ec5aa0c}
>+component {b24861fb-e91a-40dd-886d-68d26a9586c7} nsAboutCertError.js
>+contract @mozilla.org/network/protocol/about;1?what=certerror {b24861fb-e91a-40dd-886d-68d26a9586c7}
>+component {a03c813e-abe8-41de-8d0c-5aa85f877696} nsAboutSessionRestore.js
>+contract @mozilla.org/network/protocol/about;1?what=sessionrestore {a03c813e-abe8-41de-8d0c-5aa85f877696}
>+component {4e6c1112-57b6-44ba-adf9-99fb573b0a30} nsSessionStartup.js
>+contract @mozilla.org/suite/sessionstartup;1 {4e6c1112-57b6-44ba-adf9-99fb573b0a30}
>+category app-startup SessionStartup service,@mozilla.org/suite/sessionstartup;1
>+component {d37ccdf1-496f-4135-9575-037180af010d} nsSessionStore.js
>+contract @mozilla.org/suite/sessionstore;1 {d37ccdf1-496f-4135-9575-037180af010d}
>+component {bbbbe845-5a1b-40ee-813c-f84b8faaa07c} nsSuiteGlue.js
>+contract @mozilla.org/suite/suiteglue;1 {bbbbe845-5a1b-40ee-813c-f84b8faaa07c}
>+category app-startup nsSuiteGlue service,@mozilla.org/suite/suiteglue;1
>+component {450a13bd-0d07-4e5d-a9f0-448c201728b1} nsSuiteGlue.js
>+contract @mozilla.org/geolocation/prompt;1 {450a13bd-0d07-4e5d-a9f0-448c201728b1}
>diff --git a/suite/common/src/nsSuiteGlue.js b/suite/common/src/nsSuiteGlue.js
>--- a/suite/common/src/nsSuiteGlue.js
>+++ b/suite/common/src/nsSuiteGlue.js
>@@ -461,27 +461,18 @@
> 
> 
>   // for XPCOM
>-  classDescription: "SeaMonkey Suite Glue Service",
>   classID:          Components.ID("{bbbbe845-5a1b-40ee-813c-f84b8faaa07c}"),
>-  contractID:       "@mozilla.org/suite/suiteglue;1",
> 
>   QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsIObserver,
>                                          Components.interfaces.nsISupportsWeakReference,
>-                                         Components.interfaces.nsISuiteGlue]),
>+                                         Components.interfaces.nsISuiteGlue])
> 
>-  // get this contractID registered for certain categories via XPCOMUtils
>-  _xpcom_categories: [
>-    // make SuiteGlue a startup observer
>-    { category: "app-startup", service: true }
>-  ]
> }
> 
> function GeolocationPrompt() {}
> 
> GeolocationPrompt.prototype = {
>-  classDescription: "Geolocation Prompting Component",
>   classID:          Components.ID("{450a13bd-0d07-4e5d-a9f0-448c201728b1}"),
>-  contractID:       "@mozilla.org/geolocation/prompt;1",
> 
>   QueryInterface: XPCOMUtils.generateQI([Components.interfaces.nsIGeolocationPrompt]),
> 
>@@ -570,6 +561,4 @@
> };
> 
> //module initialization
>-function NSGetModule(aCompMgr, aFileSpec) {
>-  return XPCOMUtils.generateModule([SuiteGlue, GeolocationPrompt]);
>-}
>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([SuiteGlue, GeolocationPrompt]);
Comment on attachment 456663 [details] [diff] [review]
SuiteCommon.manifest [Checkin: comment 31]

(3rd time lucky)
As per Ratty's comment, no need for the extra whitespace between classID: and Components.ID
r/a=me with that addressed.
Comment on attachment 456663 [details] [diff] [review]
SuiteCommon.manifest [Checkin: comment 31]

http://hg.mozilla.org/comm-central/rev/6b4c6913a871
Attachment #456663 - Attachment description: SuiteCommon.manifest → SuiteCommon.manifest [Checkin: comment 31]
Comment on attachment 456663 [details] [diff] [review]
SuiteCommon.manifest [Checkin: comment 31]

>rename from suite/common/src/nsSuiteDownloadManagerUI.manifest
>rename to suite/common/src/nsSuiteCommon.manifest

You forgot to change the Makefile.in for this change, I'll check in a "bustage" fix for that (even though it doesn't fix tinderbox bustage, it fixes private build bustage for people building without ChatZilla).
This updates nsSidebar.js I added XPCOMUtils useage, but did not do much more cleanup than necessary for this.
Attachment #456686 - Flags: review?(iann_bugzilla)
Comment on attachment 456686 [details] [diff] [review]
nsSidebar.js [Checkin: Comment 36]

># HG changeset patch
># User Justin Wood <Callek@gmail.com>
># Parent 2ded29909aeb323a15c621d456fba9f89cf3e664
>Bug 576900 - Update the nsSiderbar.js component to also work with the new manifests. (Moves it to the common components dir too).
>
>diff --git a/suite/common/Makefile.in b/suite/common/Makefile.in
>--- a/suite/common/Makefile.in
>+++ b/suite/common/Makefile.in
>@@ -48,10 +48,6 @@
> PARALLEL_DIRS += tests downloads/tests
> endif
> 
>-EXTRA_COMPONENTS = \
>-	sidebar/nsSidebar.js \
>-	$(NULL)
>-
> include $(topsrcdir)/config/rules.mk
> 
> abs_srcdir = $(call core_abspath,$(srcdir))
>diff --git a/suite/common/src/Makefile.in b/suite/common/src/Makefile.in
>--- a/suite/common/src/Makefile.in
>+++ b/suite/common/src/Makefile.in
>@@ -48,6 +48,7 @@
> 	nsAboutCertError.js \
> 	nsAboutSessionRestore.js \
> 	nsSessionStartup.js \
>+	nsSiderbar.js \
> 	nsSuiteCommon.manifest \
> 	nsSuiteDownloadManagerUI.js \
> 	nsSuiteGlue.js \
>diff --git a/suite/common/sidebar/nsSidebar.js b/suite/common/src/nsSiderbar.js
>rename from suite/common/sidebar/nsSidebar.js
>rename to suite/common/src/nsSiderbar.js
>--- a/suite/common/sidebar/nsSidebar.js
>+++ b/suite/common/src/nsSiderbar.js
>@@ -22,6 +22,7 @@
>  * Contributor(s):
>  *   Stephen Lamm            <slamm@netscape.com>
>  *   Robert John Churchill   <rjc@netscape.com>
>+ *   Justin Wood             <Callek@gmail.com>
>  *
>  * Alternatively, the contents of this file may be used under the terms of
>  * either of the GNU General Public License Version 2 or later (the "GPL"),
>@@ -54,14 +55,12 @@
> const DEBUG = false; /* set to false to suppress debug messages */
> const PANELS_RDF_FILE  = "UPnls"; /* directory services property to find panels.rdf */
> 
>-const SIDEBAR_CONTRACTID   = "@mozilla.org/sidebar;1";
> const SIDEBAR_CID      = Components.ID("{22117140-9c6e-11d3-aaf1-00805f8a4905}");
> const CONTAINER_CONTRACTID = "@mozilla.org/rdf/container;1";
> const DIR_SERV_CONTRACTID  = "@mozilla.org/file/directory_service;1"
> const NETSEARCH_CONTRACTID = "@mozilla.org/rdf/datasource;1?name=internetsearch"
> const IO_SERV_CONTRACTID   = "@mozilla.org/network/io-service;1";
> const nsISupports      = Components.interfaces.nsISupports;
>-const nsIFactory       = Components.interfaces.nsIFactory;
> const nsISidebar       = Components.interfaces.nsISidebar;
> const nsIRDFContainer  = Components.interfaces.nsIRDFContainer;
> const nsIProperties    = Components.interfaces.nsIProperties;
>@@ -70,6 +69,8 @@
> const nsIInternetSearchService = Components.interfaces.nsIInternetSearchService;
> const nsIClassInfo = Components.interfaces.nsIClassInfo;
> 
>+Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
>+
> function nsSidebar()
> {
>     const RDF_CONTRACTID = "@mozilla.org/rdf/rdf-service;1";
>@@ -305,9 +306,6 @@
> // property of nsIClassInfo
> nsSidebar.prototype.flags = nsIClassInfo.DOM_OBJECT;
> 
>-// property of nsIClassInfo
>-nsSidebar.prototype.classDescription = "Sidebar";
>-
> // method of nsIClassInfo
> nsSidebar.prototype.getInterfaces = function(count) {
>     var interfaceList = [nsISidebar, nsIClassInfo];
>@@ -318,78 +316,12 @@
> // method of nsIClassInfo
> nsSidebar.prototype.getHelperForLanguage = function(count) {return null;}
> 
>-nsSidebar.prototype.QueryInterface =
>-function (iid) {
>-    if (iid.equals(nsISidebar) ||
>-        iid.equals(nsIClassInfo) ||
>-        iid.equals(nsISupports))
>-        return this;
>+nsSidebar.prototype.QueryInterface = 
>+XPCOMUtils.generateQI([nsISidebar, nsIClassInfo, nsISupports]);
> 
>-    throw Components.results.NS_ERROR_NO_INTERFACE;
>-}
>+nsSidebar.prototype.classID = SIDEBAR_CID;
> 
>-var sidebarModule = new Object();
>-
>-sidebarModule.registerSelf =
>-function (compMgr, fileSpec, location, type)
>-{
>-    debug("registering (all right -- a JavaScript module!)");
>-    compMgr = compMgr.QueryInterface(Components.interfaces.nsIComponentRegistrar);
>-
>-    compMgr.registerFactoryLocation(SIDEBAR_CID,
>-                                    "Sidebar JS Component",
>-                                    SIDEBAR_CONTRACTID,
>-                                    fileSpec,
>-                                    location,
>-                                    type);
>-
>-    const CATMAN_CONTRACTID = "@mozilla.org/categorymanager;1";
>-    const nsICategoryManager = Components.interfaces.nsICategoryManager;
>-    var catman = Components.classes[CATMAN_CONTRACTID].
>-                            getService(nsICategoryManager);
>-
>-    const JAVASCRIPT_GLOBAL_PROPERTY_CATEGORY = "JavaScript global property";
>-    catman.addCategoryEntry(JAVASCRIPT_GLOBAL_PROPERTY_CATEGORY,
>-                            "sidebar",
>-                            SIDEBAR_CONTRACTID,
>-                            true,
>-                            true);
>-}
>-
>-sidebarModule.getClassObject =
>-function (compMgr, cid, iid) {
>-    if (!cid.equals(SIDEBAR_CID))
>-        throw Components.results.NS_ERROR_NO_INTERFACE;
>-
>-    if (!iid.equals(Components.interfaces.nsIFactory))
>-        throw Components.results.NS_ERROR_NOT_IMPLEMENTED;
>-
>-    return sidebarFactory;
>-}
>-
>-sidebarModule.canUnload =
>-function(compMgr)
>-{
>-    debug("Unloading component.");
>-    return true;
>-}
>-
>-/* factory object */
>-var sidebarFactory = new Object();
>-
>-sidebarFactory.createInstance =
>-function (outer, iid) {
>-    debug("CI: " + iid);
>-    if (outer != null)
>-        throw Components.results.NS_ERROR_NO_AGGREGATION;
>-
>-    return (new nsSidebar()).QueryInterface(iid);
>-}
>-
>-/* entrypoint */
>-function NSGetModule(compMgr, fileSpec) {
>-    return sidebarModule;
>-}
>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([nsSidebar]);
> 
> /* static functions */
> if (DEBUG)
>diff --git a/suite/common/src/nsSuiteCommon.manifest b/suite/common/src/nsSuiteCommon.manifest
>--- a/suite/common/src/nsSuiteCommon.manifest
>+++ b/suite/common/src/nsSuiteCommon.manifest
>@@ -14,3 +14,6 @@
> category app-startup nsSuiteGlue service,@mozilla.org/suite/suiteglue;1
> component {450a13bd-0d07-4e5d-a9f0-448c201728b1} nsSuiteGlue.js
> contract @mozilla.org/geolocation/prompt;1 {450a13bd-0d07-4e5d-a9f0-448c201728b1}
>+component {22117140-9c6e-11d3-aaf1-00805f8a4905} nsSidebar.js
>+contract @mozilla.org/sidebar;1 {22117140-9c6e-11d3-aaf1-00805f8a4905}
>+category JavaScript-global-property sidebar @mozilla.org/sidebar;1
Attachment #456686 - Flags: review?(neil)
Comment on attachment 456686 [details] [diff] [review]
nsSidebar.js [Checkin: Comment 36]

>+nsSidebar.prototype.QueryInterface = 
>+XPCOMUtils.generateQI([nsISidebar, nsIClassInfo, nsISupports]);
This line needs indenting
r/a=me with this change
Attachment #456686 - Flags: review?(iann_bugzilla) → review+
Comment on attachment 456686 [details] [diff] [review]
nsSidebar.js [Checkin: Comment 36]

http://hg.mozilla.org/comm-central/rev/7fed1ff742c4
Attachment #456686 - Attachment description: nsSidebar.js → nsSidebar.js [Checkin: Comment 36]
Attachment #456686 - Flags: review?(neil)
(In reply to comment #36)
> Comment on attachment 456686 [details] [diff] [review]
> nsSidebar.js [Checkin: Comment 36]
> 
> http://hg.mozilla.org/comm-central/rev/7fed1ff742c4

And as you noted on IRC, you messed up my committee-name but this time I did the patch with -U so I wonder how you went about importing it that this got messed up?
Attached patch -editor fixup (obsolete) — Splinter Review
This fixes the -edit command line handler. And is tested ;-)

I also pushed a followup to my nsSidebar.js changes: http://hg.mozilla.org/comm-central/rev/5f567f48c811
Attachment #456713 - Flags: review?(iann_bugzilla)
Comment on attachment 456713 [details] [diff] [review]
-editor fixup

># HG changeset patch
># User Justin Wood <Callek@gmail.com>
># Date 1278825692 14400
># Node ID a53d246401516c623d6d4d6832d7a5dc3a576252
># Parent 5f567f48c811ee5da40fafbbcbd77fdacf29ea71
>Bug 576900 - Do the XPCOM manifest magic for the editor CLH too.
>(Also cleanup a small bit)
>
>diff --git a/editor/ui/Makefile.in b/editor/ui/Makefile.in
>--- a/editor/ui/Makefile.in
>+++ b/editor/ui/Makefile.in
>@@ -49,7 +49,7 @@
> ifndef MOZ_STANDALONE_COMPOSER
> PREF_JS_EXPORTS = $(srcdir)/composer.js
> ifdef MOZ_SUITE
>-EXTRA_COMPONENTS = nsComposerCmdLineHandler.js
>+EXTRA_COMPONENTS = nsComposerCLH.manifest nsComposerCLH.js
> endif
> endif
> 
>diff --git a/editor/ui/nsComposerCmdLineHandler.js b/editor/ui/nsComposerCLH.js
>rename from editor/ui/nsComposerCmdLineHandler.js
>rename to editor/ui/nsComposerCLH.js
>--- a/editor/ui/nsComposerCmdLineHandler.js
>+++ b/editor/ui/nsComposerCLH.js
>@@ -34,19 +34,12 @@
>  *
>  * ***** END LICENSE BLOCK ***** */
> 
>-const nsICmdLineHandler     = Components.interfaces.nsICmdLineHandler;
> const nsICommandLineHandler = Components.interfaces.nsICommandLineHandler;
>-const nsIFactory            = Components.interfaces.nsIFactory;
> const nsISupports           = Components.interfaces.nsISupports;
>-const nsIModule             = Components.interfaces.nsIModule;
>-const nsIComponentRegistrar = Components.interfaces.nsIComponentRegistrar;
>-const nsICategoryManager    = Components.interfaces.nsICategoryManager;
> const nsISupportsString     = Components.interfaces.nsISupportsString;
> const nsIWindowWatcher      = Components.interfaces.nsIWindowWatcher;
> 
>-const NS_ERROR_FAILURE        = Components.results.NS_ERROR_FAILURE;
>-const NS_ERROR_NO_AGGREGATION = Components.results.NS_ERROR_NO_AGGREGATION;
>-const NS_ERROR_NO_INTERFACE   = Components.results.NS_ERROR_NO_INTERFACE;
>+Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
> 
> function nsComposerCmdLineHandler() {}
> nsComposerCmdLineHandler.prototype = {
>@@ -55,28 +48,7 @@
>   },
> 
>   /* nsISupports */
>-
>-  QueryInterface: function(iid) {
>-    if (iid.equals(nsISupports))
>-      return this;
>-
>-    if (nsICmdLineHandler && iid.equals(nsICmdLineHandler))
>-      return this;
>-
>-    if (nsICommandLineHandler && iid.equals(nsICommandLineHandler))
>-      return this;
>-
>-    throw NS_ERROR_NO_INTERFACE;
>-  },
>-
>-  /* nsICmdLineHandler */
>-  commandLineArgument : "-edit",
>-  prefNameForStartup : "general.startup.editor",
>-  chromeUrlForTask : "chrome://editor/content/editor.xul",
>-  helpText : "Start with editor.",
>-  handlesArgs : true,
>-  defaultArgs : "about:blank",
>-  openWindowWithArgs : true,
>+  QueryInterface: XPCOMUtils.generateQI([nsICommandLineHandler, nsISupports]),
> 
>   /* nsICommandLineHandler */
>   handle : function handle(cmdLine) {
>@@ -110,106 +82,10 @@
>     cmdLine.preventDefault = true;
>   },
> 
>-  helpInfo : "  -edit <url>        Open Composer.\n"
>+  helpInfo : "  -edit <url>        Open Composer.\n",
>+  
>+  /* XPCOMUtils */
>+  classID: Components.ID("{f7d8db95-ab5d-4393-a796-9112fe758cfa}")
> };
> 
>-function nsComposerCmdLineHandlerFactory() {
>-}
>-
>-nsComposerCmdLineHandlerFactory.prototype = {
>-  /* nsISupports */
>-
>-  QueryInterface: function(iid) {
>-    if (!iid.equals(nsIFactory) &&
>-        !iid.equals(nsISupports)) {
>-          throw Components.results.NS_ERROR_NO_INTERFACE;
>-    }
>-    return this;
>-  },
>-
>-  /* nsIFactory */
>-  createInstance: function(outer, iid) {
>-    if (outer != null) {
>-      throw NS_ERROR_NO_AGGREGATION;
>-    }
>-
>-    return new nsComposerCmdLineHandler().QueryInterface(iid);
>-  },
>-
>-  lockFactory: function(lock) {
>-  }
>-};
>-
>-const nsComposerCmdLineHandler_CID =
>-  Components.ID("{f7d8db95-ab5d-4393-a796-9112fe758cfa}");
>-
>-const ContractIDPrefix =
>-  "@mozilla.org/commandlinehandler/general-startup;1?type=";
>-
>-var thisModule = {
>-  /* nsISupports */
>-
>-  QueryInterface: function(iid) {
>-    if (!iid.equals(nsIModule) &&
>-        !iid.equals(nsISupports)) {
>-          throw Components.results.NS_ERROR_NO_INTERFACE;
>-    }
>-    return this;
>-  },
>-
>-  /* nsIModule */
>-
>-  getClassObject: function (compMgr, cid, iid) {
>-    if (!cid.equals(nsComposerCmdLineHandler_CID)) {
>-      throw NS_ERROR_FAILURE;
>-    }
>-
>-    if (!iid.equals(nsIFactory)) {
>-      throw NS_ERROR_NO_INTERFACE;
>-    }
>-
>-    return new nsComposerCmdLineHandlerFactory();
>-  },
>-
>-  registerSelf: function (compMgr, fileSpec, location, type) {
>-    var compReg = compMgr.QueryInterface(nsIComponentRegistrar);
>-    compReg.registerFactoryLocation(nsComposerCmdLineHandler_CID,
>-                                    "nsComposerCmdLineHandler",
>-                                    ContractIDPrefix + "edit",
>-                                    fileSpec, location, type);
>-    compReg.registerFactoryLocation(nsComposerCmdLineHandler_CID,
>-                                    "nsComposerCmdLineHandler",
>-                                    ContractIDPrefix + "editor",
>-                                    fileSpec, location, type);
>-
>-    var catMan = Components.classes["@mozilla.org/categorymanager;1"].getService(nsICategoryManager);
>-    catMan.addCategoryEntry("command-line-argument-handlers",
>-                            "nsComposerCmdLineHandler",
>-                            ContractIDPrefix + "edit",
>-                            true, true);
>-    catMan.addCategoryEntry("command-line-handler",
>-                            "m-edit",
>-                            ContractIDPrefix + "edit",
>-                            true, true);
>-  },
>-
>-  unregisterSelf: function (compMgr, location, type) {
>-    var compReg = compMgr.QueryInterface(nsIComponentRegistrar);
>-    compReg.unregisterFactoryLocation(nsComposerCmdLineHandler_CID,
>-                                      location);
>-
>-    var catMan = Components.classes["@mozilla.org/categorymanager;1"].getService(nsICategoryManager);
>-    catMan.deleteCategoryEntry("command-line-argument-handlers",
>-                               "nsComposerCmdLineHandler", true);
>-    catMan.deleteCategoryEntry("command-line-handler",
>-                               "m-edit", true);
>-  },    
>-
>-  canUnload: function (compMgr) {
>-    return true;
>-  }
>-};
>-
>-function NSGetModule(compMgr, fileSpec) {
>-  return thisModule;
>-}
>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([nsComposerCmdLineHandler]);
>diff --git a/editor/ui/nsComposerCLH.manifest b/editor/ui/nsComposerCLH.manifest
>new file mode 100644
>--- /dev/null
>+++ b/editor/ui/nsComposerCLH.manifest
>@@ -0,0 +1,4 @@
>+component {f7d8db95-ab5d-4393-a796-9112fe758cfa} nsComposerCLH.js
>+contract @mozilla.org/commandlinehandler/general-startup;1?type=edit {f7d8db95-ab5d-4393-a796-9112fe758cfa}
>+contract @mozilla.org/commandlinehandler/general-startup;1?type=editor {f7d8db95-ab5d-4393-a796-9112fe758cfa}
>+category command-line-handler m-edit @mozilla.org/commandlinehandler/general-startup;1?type=edit
Attachment #456713 - Flags: review?(neil)
>diff --git a/editor/ui/nsComposerCmdLineHandler.js b/editor/ui/nsComposerCLH.js
>rename from editor/ui/nsComposerCmdLineHandler.js
>rename to editor/ui/nsComposerCLH.js

Hmm everythingelse is nsFooHandler.js, e.g. nsMailNewsCommandLineHandler.js. Why this change?
(In reply to comment #40)
> >diff --git a/editor/ui/nsComposerCmdLineHandler.js b/editor/ui/nsComposerCLH.js
> >rename from editor/ui/nsComposerCmdLineHandler.js
> >rename to editor/ui/nsComposerCLH.js
> 
> Hmm everythingelse is nsFooHandler.js, e.g. nsMailNewsCommandLineHandler.js.
> Why this change?

imo it just made the commandline too long, and this makes more sense. I'm happy to let it be reverted back to the original filename though.
Blocks: 577859
Shared builds succeed now, but this test failure happens:

Running TestStaticAtoms tests...
TEST-PASS | foo is a static atom
TEST-PASS | foo is the right pointer
TEST-UNEXPECTED-FAIL | Did not get a static atom for foo
Finished running TestStaticAtoms tests.
make[3]: *** [check] Error 1
make[4]: Entering directory `/builds/slave/comm-central-trunk-linux/build/objdir/mozilla/xpcom/tests/external'

Is this still from this problem, or should we file a new bug for this?
> TEST-UNEXPECTED-FAIL | Did not get a static atom for foo
Isn't that Bug 577500 Core::XPCOM TestStaticAtoms fails getting static atoms
Comment on attachment 456344 [details] [diff] [review]
Fix typo GNOME not Gnome patch [Checkin: Comment 10]

I originally did the the Gnome lines as copies of the Mac lines and retyped over them, and I did originally make other GNOME/Gnome mixups but I happened to spot the other ones before posting the patch ;-)
> the nsSiderbar.js component.
Side*r*bar?

> SuiteCommon.manifest
Although the patches actually call it nsSuiteCommon.manifest ... I don't see a need for the ns prefix except when the manifest only contains one component in which case it should have whatever name that component has.
Comment on attachment 456713 [details] [diff] [review]
-editor fixup

>rename from editor/ui/nsComposerCmdLineHandler.js
>rename to editor/ui/nsComposerCLH.js
removed-files.in?

>+contract @mozilla.org/commandlinehandler/general-startup;1?type=edit {f7d8db95-ab5d-4393-a796-9112fe758cfa}
>+contract @mozilla.org/commandlinehandler/general-startup;1?type=editor {f7d8db95-ab5d-4393-a796-9112fe758cfa}
Actually we don't need both of these, this was left over from when the old XPFE command line handler needed one contract for each possible argument. The new system only needs one contract for each checkbox in Appearance preferences.
>> the nsSiderbar.js component.
> Side*r*bar?
Hey, I *did* ask Callek to test his patch first...
(In reply to comment #11)
> >-  lockFactory: function lockFactory(lock) {
> >-    /* no-op */
> >   }
> Don't we want to keep lockFactory as we are in nsBrowserContentHandler.js
> r=me with that addressed
But that one was the one in nsAboutAbout.js ...
(In reply to comment #48)
> (In reply to comment #11)
> > >-  lockFactory: function lockFactory(lock) {
> > >-    /* no-op */
> > >   }
> > Don't we want to keep lockFactory as we are in nsBrowserContentHandler.js
> > r=me with that addressed
> But that one was the one in nsAboutAbout.js ...

Yes, I was saying that seeing we're keeping it in nsBrowserContentHandler.js shouldn't we keep it in nsAboutAbout.js
Comment on attachment 456713 [details] [diff] [review]
-editor fixup

>rename from editor/ui/nsComposerCmdLineHandler.js
>rename to editor/ui/nsComposerCLH.js

I don't think this rename is necessary.
>+  QueryInterface: XPCOMUtils.generateQI([nsICommandLineHandler, nsISupports]),
You don't need nsISupports here as it is done as part of makeQI in XPCOMUtils.jsm.

r=me with those addressed (and Neil's comments obviously).
Attachment #456713 - Flags: review?(iann_bugzilla) → review+
(In reply to comment #45)
> > the nsSiderbar.js component.
> Side*r*bar?
> 
> > SuiteCommon.manifest
> Although the patches actually call it nsSuiteCommon.manifest ... I don't see a
> need for the ns prefix except when the manifest only contains one component in
> which case it should have whatever name that component has.

After seeing our other Suite*.manifest uses I agree. rs+/a+ for the followup on that?
r? from Neil for one more look; (did I choose the correct contract?) feedback+ is really a r+ for IanN from first version of this patch.
Attachment #456713 - Attachment is obsolete: true
Attachment #456993 - Flags: review?(neil)
Attachment #456993 - Flags: feedback+
Attachment #456713 - Flags: review?(neil)
after my -editor fix, I think we're done here.
Depends on: 576745, 576869
(In reply to comment #49)
> (In reply to comment #48)
> > (In reply to comment #11)
> > > >-  lockFactory: function lockFactory(lock) {
> > > >-    /* no-op */
> > > >   }
> > > Don't we want to keep lockFactory as we are in nsBrowserContentHandler.js
> > > r=me with that addressed
> > But that one was the one in nsAboutAbout.js ...
> Yes, I was saying that seeing we're keeping it in nsBrowserContentHandler.js
> shouldn't we keep it in nsAboutAbout.js
As I said in comment #6, none of the other Abouts bother with it.

(In reply to comment #51)
> (In reply to comment #45)
> > > SuiteCommon.manifest
> After seeing our other Suite*.manifest uses I agree. rs+/a+ for the followup on
> that?
rs=me (in theory you can wait until after the tree opens)
(In reply to comment #51)
> > > SuiteCommon.manifest
> > Although the patches actually call it nsSuiteCommon.manifest ... I don't see a
> > need for the ns prefix except when the manifest only contains one component in
> > which case it should have whatever name that component has.
> 
> After seeing our other Suite*.manifest uses I agree. rs+/a+ for the followup on
> that?

rs+/a+ from me for this, so we become more consistent.
Note that along with doing that, you also need to fix the package-manifest, where the entry needs to be re-sorted to the correct place. As we didn't ship any nightlies with this, you can leave removed-files untouched, though.
(In reply to comment #54)
> (In reply to comment #49)
> > (In reply to comment #48)
> > > (In reply to comment #11)
> > > > >-  lockFactory: function lockFactory(lock) {
> > > > >-    /* no-op */
> > > > >   }
> > > > Don't we want to keep lockFactory as we are in nsBrowserContentHandler.js
> > > > r=me with that addressed
> > > But that one was the one in nsAboutAbout.js ...
> > Yes, I was saying that seeing we're keeping it in nsBrowserContentHandler.js
> > shouldn't we keep it in nsAboutAbout.js
> As I said in comment #6, none of the other Abouts bother with it.
Ah, I missed comment #6, sorry.
Comment on attachment 456993 [details] [diff] [review]
-editor fixup v2 [Checkin: comment 58]

>-const nsICmdLineHandler     = Components.interfaces.nsICmdLineHandler;
> const nsICommandLineHandler = Components.interfaces.nsICommandLineHandler;
>-const nsIFactory            = Components.interfaces.nsIFactory;

> const nsISupports           = Components.interfaces.nsISupports;
Nit: also no longer used

>-const nsIModule             = Components.interfaces.nsIModule;
>-const nsIComponentRegistrar = Components.interfaces.nsIComponentRegistrar;
>-const nsICategoryManager    = Components.interfaces.nsICategoryManager;
> const nsISupportsString     = Components.interfaces.nsISupportsString;
> const nsIWindowWatcher      = Components.interfaces.nsIWindowWatcher;
> 
>-const NS_ERROR_FAILURE        = Components.results.NS_ERROR_FAILURE;
>-const NS_ERROR_NO_AGGREGATION = Components.results.NS_ERROR_NO_AGGREGATION;
>-const NS_ERROR_NO_INTERFACE   = Components.results.NS_ERROR_NO_INTERFACE;
>+Components.utils.import("resource://gre/modules/XPCOMUtils.jsm");
Nit: I'd prefer the import before the consts.
Attachment #456993 - Flags: review?(neil) → review+
Comment on attachment 456993 [details] [diff] [review]
-editor fixup v2 [Checkin: comment 58]

I did Neil's fixes and also added the new manifest to package-manifest.in:

http://hg.mozilla.org/comm-central/rev/6561dd9db78e

Hopefully the tree will go green again now ;-)
Attachment #456993 - Attachment description: -editor fixup v2 → -editor fixup v2 [Checkin: comment 58]
(In reply to comment #55)
> (In reply to comment #51)
> > > > SuiteCommon.manifest
> > > Although the patches actually call it nsSuiteCommon.manifest ... I don't see a
> > > need for the ns prefix except when the manifest only contains one component in
> > > which case it should have whatever name that component has.
> > 
> > After seeing our other Suite*.manifest uses I agree. rs+/a+ for the followup on
> > that?
> 
> rs+/a+ from me for this, so we become more consistent.
> Note that along with doing that, you also need to fix the package-manifest,
> where the entry needs to be re-sorted to the correct place. As we didn't ship
> any nightlies with this, you can leave removed-files untouched, though.

http://hg.mozilla.org/comm-central/rev/112ee0f39b5f
Comment on attachment 456663 [details] [diff] [review]
SuiteCommon.manifest [Checkin: comment 31]

>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([AboutSessionRestore[);
Silly typo...
(In reply to comment #60)
> Comment on attachment 456663 [details] [diff] [review]
> SuiteCommon.manifest [Checkin: comment 31]
> 
> >+var NSGetFactory = XPCOMUtils.generateNSGetFactory([AboutSessionRestore[);
> Silly typo...

Fixed with http://hg.mozilla.org/comm-central/rev/b25829958fa3
And that typo fix got mochitest-browser-chrome to run and not time out.
Bug 576869 remains, this bug is otherwise completely fixed.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Depends on: 705141
Blocks: 729526
You need to log in before you can comment on or make changes to this bug.