Closed Bug 1130886 Opened 5 years ago Closed 5 years ago

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

Categories

(Toolkit :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla38
Tracking Status
firefox38 --- fixed

People

(Reporter: philip.chee, Assigned: philip.chee)

References

Details

Attachments

(1 file, 2 obsolete files)

Attached patch WIP patch (partially working) (obsolete) — Splinter Review
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?
Attachment #8561093 - Flags: feedback?(dtownsend)
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?
Seems to me SeaMonkey should just get rid of its copy.
Component: Search → General
Product: Firefox → Toolkit
(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: gavin@gavinsharp.com] 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.
> becomes
>+if not CONFIG['HAVE_SIDEBAR']:
That should be if CONFIG['...
(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.
Attached patch Patch v1 use HAVE_SIDEBAR (obsolete) — Splinter Review
> 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
Assignee: nobody → philip.chee
Attachment #8561093 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8561093 - Flags: feedback?(dtownsend)
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
Attachment #8561435 - Attachment is obsolete: true
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
(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.
Attachment #8563893 - Flags: review?(dtownsend) → review+
https://hg.mozilla.org/mozilla-central/rev/1dd9353d0e97
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla38
You need to log in before you can comment on or make changes to this bug.