576900 - Make suite XPCOM components use new manifests and data tables

Status: RESOLVED FIXED

(SeaMonkey :: Build Config, defect)

Description

[[e-mail]]

+++ 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.

Comment 1

[[e-mail]]

Attachment: C++ fixes [Checkin: Comment 4]

Comment 2

[Justin Wood (:Callek)]

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.

Comment 3

[[e-mail]]

(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 4

[[e-mail]]

Comment on attachment 455979 [details] [diff] [review]
C++ fixes [Checkin: Comment 4]

Pushed changeset 7ab1997b0fc5 to comm-central.

Comment 5

[[e-mail]]

Attachment: feed shell smile JS component fixes [Checkin: Comment 26]

* 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!)

Comment 6

[[e-mail]]

Attachment: browser JS component fixes [Checkin: Comment 26]

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.

Comment 7

[Justin Wood (:Callek)]

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.

Comment 8

[Ian Neal]

Attachment: Fix typo GNOME not Gnome patch [Checkin: Comment 10]

Requesting r/a for CLOSED TREE

Comment 9

[Ian Neal]

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!)

Comment 10

[Ian Neal]

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

Comment 11

[Ian Neal]

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

Comment 12

[Philip Chee]

Attachment: Fix nsSuiteDownloadManagerUI [Checkin: Comment 26]

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

Comment 13

[Justin Wood (:Callek)]

Comment on attachment 456110 [details] [diff] [review]
feed shell smile JS component fixes [Checkin: Comment 26]

r+ pending my nits in c#7

Comment 14

[Justin Wood (: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

Comment 15

[Justin Wood (: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 16

[Justin Wood (:Callek)]

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.

Comment 17

[Robert Kaiser]

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.

Comment 18

[Justin Wood (:Callek)]

(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?

Comment 19

[Philip Chee]

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 20

[Philip Chee]

Comment on attachment 456423 [details] [diff] [review]
Fix nsSuiteDownloadManagerUI [Checkin: Comment 26]

Lets see who can get to review this first.

Comment 21

[Philip Chee]

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.

Comment 22

[Philip Chee]

Comment on attachment 456423 [details] [diff] [review]
Fix nsSuiteDownloadManagerUI [Checkin: Comment 26]

Sorry for the bugspam. I misunderstood Callek.

Comment 23

[Justin Wood (:Callek)]

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

Comment 24

[Justin Wood (:Callek)]

Attachment: SuiteCommon.manifest [Checkin: comment 31]

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)

Comment 25

[Justin Wood (:Callek)]

Comment on attachment 456663 [details] [diff] [review]
SuiteCommon.manifest [Checkin: comment 31]

Whoever gets to it first...

Comment 26

[Philip Chee]

Pushed to comm-central
> attachment 456110 [details] [diff] [review] (feed shell smile)
http://hg.mozilla.org/comm-central/rev/6192acf65e8d
> attachment 456118 [details] [diff] [review] (browser)
http://hg.mozilla.org/comm-central/rev/5e438936f3fa
> attachment 456423 [details] [diff] [review] (nsSuiteDownloadManagerUI)
http://hg.mozilla.org/comm-central/rev/f13a1ff86839

Comment 27

[Philip Chee]

> +  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 28

[Ian Neal]

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 29

[Ian Neal]

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 30

[Ian Neal]

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 31

[Ian Neal]

Comment on attachment 456663 [details] [diff] [review]
SuiteCommon.manifest [Checkin: comment 31]

http://hg.mozilla.org/comm-central/rev/6b4c6913a871

Comment 32

[Robert Kaiser]

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).

Comment 33

[Justin Wood (:Callek)]

Attachment: nsSidebar.js [Checkin: Comment 36]

This updates nsSidebar.js I added XPCOMUtils useage, but did not do much more cleanup than necessary for this.

Comment 34

[Justin Wood (:Callek)]

Comment on attachment 456686 [details] [diff] [review]
nsSidebar.js [Checkin: Comment 36]

># HG changeset patch
># User Justin Wood <[e-mail]>
># 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            <[e-mail]>
>  *   Robert John Churchill   <[e-mail]>
>+ *   Justin Wood             <[e-mail]>
>  *
>  * 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

Comment 35

[Ian Neal]

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

Comment 36

[Ian Neal]

Comment on attachment 456686 [details] [diff] [review]
nsSidebar.js [Checkin: Comment 36]

http://hg.mozilla.org/comm-central/rev/7fed1ff742c4

Comment 37

[Justin Wood (:Callek)]

(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?

Comment 38

[Justin Wood (:Callek)]

Attachment: -editor fixup

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

Comment 39

[Justin Wood (:Callek)]

Comment on attachment 456713 [details] [diff] [review]
-editor fixup

># HG changeset patch
># User Justin Wood <[e-mail]>
># 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

Comment 40

[Philip Chee]

>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?

Comment 41

[Justin Wood (:Callek)]

(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.

Comment 42

[Robert Kaiser]

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?

Comment 43

[Philip Chee]

> TEST-UNEXPECTED-FAIL | Did not get a static atom for foo
Isn't that Bug 577500 Core::XPCOM TestStaticAtoms fails getting static atoms

Comment 44

[[e-mail]]

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 ;-)

Comment 45

[[e-mail]]

> 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 46

[[e-mail]]

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.

Comment 47

[Philip Chee]

>> the nsSiderbar.js component.
> Side*r*bar?
Hey, I *did* ask Callek to test his patch first...

Comment 48

[[e-mail]]

(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 ...

Comment 49

[Ian Neal]

(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 50

[Ian Neal]

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).

Comment 51

[Justin Wood (:Callek)]

(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?

Comment 52

[Justin Wood (:Callek)]

Attachment: -editor fixup v2 [Checkin: comment 58]

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.

Comment 53

[Justin Wood (:Callek)]

after my -editor fix, I think we're done here.

Comment 54

[[e-mail]]

(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)

Comment 55

[Robert Kaiser]

(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.

Comment 56

[Ian Neal]

(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 57

[[e-mail]]

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.

Comment 58

[Mark Banner (:standard8)]

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 ;-)

Comment 59

[Justin Wood (:Callek)]

(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 60

[[e-mail]]

Comment on attachment 456663 [details] [diff] [review]
SuiteCommon.manifest [Checkin: comment 31]

>+var NSGetFactory = XPCOMUtils.generateNSGetFactory([AboutSessionRestore[);
Silly typo...

Comment 61

[Robert Kaiser]

(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

Comment 62

[Robert Kaiser]

And that typo fix got mochitest-browser-chrome to run and not time out.

Comment 63

[Justin Wood (:Callek)]

Bug 576869 remains, this bug is otherwise completely fixed.