1130886 - Warning: Trying to re-register CID '{22117140-9c6e-11d3-aaf1-00805f8a4905}'

Status: RESOLVED FIXED

(Toolkit :: General, defect)

Description

[Philip Chee]

Attachment: WIP patch (partially working)

Bug 1068186 seems to have caused a hiccup when building SeaMonkey.

> Mon Feb 09 2015 01:17:29
> Warning: Trying to re-register CID '{22117140-9c6e-11d3-aaf1-00805f8a4905}' already registered by > jar:file:///C:/T1/hg/objdir-sm/dist/seamonkey/omni.ja!/components/nsSidebar.js.
> Source file: jar:file:///C:/T1/hg/objdir-sm/dist/seamonkey/omni.ja!/components/components.manifest
> Line: 118

SeaMonkey has it's own implementation of window.sidebar which clashes with the one in toolkit. I have a WIP patch but I don't know if this is acceptable or not.

#ifndef MOZ_SUITE
component {22117140-9c6e-11d3-aaf1-00805f8a4905} nsSidebar.js
contract @mozilla.org/sidebar;1 {22117140-9c6e-11d3-aaf1-00805f8a4905}
#endif

This ifdef doesn't seem to work. Am I missing a step or two?

Comment 1

[Dave Townsend [:mossop]]

You would probably need to add MOZ_SUITE to DEFINES for that to work, however why don't we just use a different CID for one of the components? Suite should be able to override with its own sidebar component just fine.

Additionally the nsGlobalWindow bits that are required for this sidebar component to work at all are ifdeffed based on HAVE_SIDEBAR (http://mxr.mozilla.org/mozilla-central/source/dom/base/moz.build#444). It probably makes sense to only include the sidebar component in those cases and that looks like it doesn't include suite?

Comment 2

[:Gavin Sharp [email: [e-mail]]]

Seems to me SeaMonkey should just get rid of its copy.

Comment 3

[Philip Chee]

(In reply to Dave Townsend [:mossop] from comment #1)
> You would probably need to add MOZ_SUITE to DEFINES for that to work,
> however why don't we just use a different CID for one of the components?
> Suite should be able to override with its own sidebar component just fine.
Yes this is the approach recommended by Neil

> Additionally the nsGlobalWindow bits that are required for this sidebar
> component to work at all are ifdeffed based on HAVE_SIDEBAR
> (http://mxr.mozilla.org/mozilla-central/source/dom/base/moz.build#444). It
> probably makes sense to only include the sidebar component in those cases
> and that looks like it doesn't include suite?

Aha. So in toolkit/components/search/moz.build
+if not CONFIG['MOZ_SUITE']:
becomes
+if not CONFIG['HAVE_SIDEBAR']:
and in /toolkit/components/search/toolkitsearch.manifest
+#ifndef MOZ_SUITE
becomes:
+#ifdef HAVE_SIDEBAR

(In reply to :Gavin Sharp [email: [e-mail]] from comment #2)
> Seems to me SeaMonkey should just get rid of its copy.
We still have the addPanel stuff because we still have the tabbed sidebar from Mozilla Suite days. Understandably I don't think you or Dave would  agree to putting those back.

Comment 4

[Philip Chee]

> becomes
>+if not CONFIG['HAVE_SIDEBAR']:
That should be if CONFIG['...

Comment 5

[Dave Townsend [:mossop]]

(In reply to Philip Chee from comment #3)
> (In reply to Dave Townsend [:mossop] from comment #1)
> > You would probably need to add MOZ_SUITE to DEFINES for that to work,
> > however why don't we just use a different CID for one of the components?
> > Suite should be able to override with its own sidebar component just fine.
> Yes this is the approach recommended by Neil
> 
> > Additionally the nsGlobalWindow bits that are required for this sidebar
> > component to work at all are ifdeffed based on HAVE_SIDEBAR
> > (http://mxr.mozilla.org/mozilla-central/source/dom/base/moz.build#444). It
> > probably makes sense to only include the sidebar component in those cases
> > and that looks like it doesn't include suite?
> 
> Aha. So in toolkit/components/search/moz.build
> +if not CONFIG['MOZ_SUITE']:
> becomes
> +if not CONFIG['HAVE_SIDEBAR']:
> and in /toolkit/components/search/toolkitsearch.manifest
> +#ifndef MOZ_SUITE
> becomes:
> +#ifdef HAVE_SIDEBAR

Almost but you've have to copy the definition of HAVE_SIDEBAR into the moz.build first.

Comment 6

[Philip Chee]

Attachment: Patch v1 use HAVE_SIDEBAR

> Almost but you've have to copy the definition of HAVE_SIDEBAR into the  moz.build first.
Doesn't seem to be needed.

> remote: You can view the progress of your build at the following URL:
> remote:   https://treeherder.mozilla.org/#/jobs?repo=try&revision=1745c36d1542
> remote: Alternatively, view them on TBPL (soon to be deprecated):
> remote:   https://tbpl.mozilla.org/?tree=Try&rev=1745c36d1542

Comment 7

[Philip Chee]

Comment on attachment 8561435 [details] [diff] [review]
Patch v1 use HAVE_SIDEBAR

https://treeherder.mozilla.org/logviewer.html#?job_id=4806997&repo=try

09:00:30 INFO - Error: /builds/slave/try-lx-00000000000000000000000/build/src/obj-firefox/browser/installer/package-manifest:278: Missing file(s): bin/components/nsSidebar.js

Comment 8

[Philip Chee]

I've added the following to moz.build but nsSidebar.js is still missing...

if CONFIG['MOZ_BUILD_APP'] in ['browser', 'mobile/android', 'xulrunner']:
    DEFINES['HAVE_SIDEBAR'] = True

Comment 9

[:Gavin Sharp [email: [e-mail]]]

(In reply to Philip Chee from comment #3)
> We still have the addPanel stuff because we still have the tabbed sidebar
> from Mozilla Suite days. Understandably I don't think you or Dave would 
> agree to putting those back.

I'm suggesting you drop support for those because no one uses them, and then you can stop having to maintain nsSidebar code yourself, or deal with bugs like this one.

Comment 10

[Philip Chee]

Attachment: Patch v2 use HAVE_SIDEBAR

https://treeherder.mozilla.org/#/jobs?repo=try&author=[e-mail]
OR
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3398383e856b
https://treeherder.mozilla.org/#/jobs?repo=try&revision=928cf6bf83f3
https://treeherder.mozilla.org/#/jobs?repo=try&revision=47db9774a856

Comment 11

[Philip Chee]

http://hg.mozilla.org/integration/mozilla-inbound/rev/1dd9353d0e97

Comment 12

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/1dd9353d0e97