463278 - De-extensionify gloda

Status: VERIFIED FIXED

(MailNews Core :: Backend, defect)

Description

[Nomis101]

User-Agent:       Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10_5_5; de-de) AppleWebKit/528.1 (KHTML, like Gecko) Version/4.0 Safari/528.1
Build Identifier: Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081105 Shredder/3.0b1pre

In the newer builds I get the following warning in the error console:

> Warning: Warning: Duplicate resource declaration for 'gloda' ignored.
> Source File: file:///Volumes/Shredder/Shredder.app/Contents/MacOS/chrome/gloda.manifest
> Line: 1

If I than click on the link in this warning I get two new errors:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISupports.QueryInterface]"  nsresult: "0x80004002 (NS_NOINTERFACE)"  location: "native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0"  data: no]

and

Error: [Exception... "Component returned failure code: 0x80040154 (NS_ERROR_FACTORY_NOT_REGISTERED) [nsIDocShellHistory.useGlobalHistory]"  nsresult: "0x80040154 (NS_ERROR_FACTORY_NOT_REGISTERED)"  location: "JS frame :: chrome://global/content/bindings/browser.xml ::  :: line 643"  data: no]
Source File: chrome://global/content/bindings/browser.xml
Line: 647

Reproducible: Always

Steps to Reproduce:
1. Open Thunderbird and look into the error console
2. Click on the link in the displayed warning
Actual Results:  
First you will see the warning and than this two errors

Comment 1

[Serge Gautherie (:sgautherie)]

(In reply to comment #0)
> > Warning: Warning: Duplicate resource declaration for 'gloda' ignored.

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081105 SeaMonkey/2.0a2pre] (nightly) (W2Ksp4)
[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081105 Shredder/3.0b1pre] (nightly) (W2Ksp4)

I can't reproduce, from zip file, with either current or new profile.

MacOSX only ?

Comment 2

[Phil Ringnalda (:philor)]

Should be Mac and Tb/Linux, the three places there's no packages file to control what gets shipped.

The NS_NOINTERFACE is something else, it's been around for a while now, Fx and Tb, the useGlobalHistory is bug 463176, and this is just because gloda landed with all its extension skeleton still intact.

Comment 3

[Phil Ringnalda (:philor)]

Attachment: A whack at it [Checkin: Comment 5]

Gloda, I served with extensions, I knew extensions, extensions were friends of mine. Gloda, you're no extension (anymore).

Comment 4

[David :Bienvenu]

Comment on attachment 346621 [details] [diff] [review]
A whack at it
[Checkin: Comment 5]

thx, Phil - this seems to build and work fine on the Mac, though I didn't do an "official" build.

Comment 5

[Phil Ringnalda (:philor)]

http://hg.mozilla.org/comm-central/rev/d5be46adae02

Comment 6

[Nomis101]

I don't wanna be a grinch, but after checkin of this patch I still get the exactly same warning and errors.
I get it in the tinderbox build
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081107 Shredder/3.0b1pre
and in my own (fresh) build
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081107 Thunderbird/3.0b1pre

Comment 7

[Nomis101]

Attachment: Screenshot of error console - current build

Screenshot from the error console: Tinderbox build - Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.5; en-US; rv:1.9.1b2pre) Gecko/20081107 Shredder/3.0b1pre - 1226066261
(I don't know where this third error cames from, never seen this before)

Comment 8

[Phil Ringnalda (:philor)]

Attachment: Whack more moles

I'd sort of vaguely like to know why that single declaration is a duplicate declaration, but not badly enough to spend any more time on this, particularly when using resource://gre/modules/ is The Right Thing, as demonstrated by every other Cu.import use in the tree (except the one freaky resource:///modules/ in browser/).

Comment 9

[Robert Kaiser]

(In reply to comment #8)
> when using resource://gre/modules/ is The Right Thing

Actually, I believe that resource://app/modules/ is The Right Thing if it is an app-specific module and not a platform-wide module.
Once we're running on XULRunner, resource://gre/ points inside the XULRunner directory while resource://app/ points into our app's directory.

What I don't understand though is why gloda is in its own .jar and doesn't just share messenger.jar.

Comment 10

[Nomis101]

(In reply to comment #8)
> Created an attachment (id=347038) [details]
> Whack more moles

Yes, this seems to work much butter. :)
I applied this patch on my own build and now I doesn't get this warning anymore. If I than force another warning in the error console (opening the addressbook) and click on that link, than I only get one of the mentioned errors now.

 Fehler: uncaught exception: [Exception... "Component returned failure code: 0x80004002 (NS_NOINTERFACE) [nsISupports.QueryInterface]"  nsresult: "0x80004002 (NS_NOINTERFACE)"  location: "native frame :: <unknown filename> :: <TOP_LEVEL> :: line 0"  data: no]

Comment 11

[David :Bienvenu]

Comment on attachment 347038 [details] [diff] [review]
Whack more moles

thx, Phil.

Comment 12

[Phil Ringnalda (:philor)]

Attachment: Whack, throw corpses in /app/

Indeed, app.

Comment 13

[Phil Ringnalda (:philor)]

Comment on attachment 347067 [details] [diff] [review]
Whack, throw corpses in /app/

Wups, didn't notice I was midairing my sr, I'll just carry that along with me :)

Comment 15

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 347067 [details] [diff] [review]
Whack, throw corpses in /app/

Per bug 463775, it's okay for us to add our own make rule to install stuff under modules.

As such, let's make gloda install its stuff to dist/bin/modules/gloda, and the namespace be app/modules/gloda/BLAH.

With that change made, we will be able to lose the code in the unit test that creates the "gloda" resource alias programatically.  r-'ing just because there's some non-trivial changes to be made here.

Comment 16

[Phil Ringnalda (:philor)]

Attachment: Whack, corpses and blood everywhere, v.4

Only fully-tested clear through to removed-files.in removing in the Tb/Win installer, but I'm reasonably sure (as sure as I can be without running my own update server) that it'll work right elsewhere too.

Don't suppose we could just leave those removed-files in for a few weeks, and then decide that anyone who hasn't updated their nightly yet deserves pain and confusion? I'd really rather not carry that albatross around our neck for all time, just to clear out a few nightlies.

Comment 17

[Phil Ringnalda (:philor)]

As KaiRo points out, we really need to get done with removing those files pretty quickly, since SeaMonkey wants to ship places, which also ships a modules/utils.js, so it would be a nice thing if we didn't ship a beta including them in modules and clutter up the whole removed-files situation.

Comment 18

[David :Bienvenu]

yes, I think we should do this before b1, if possible.

Comment 19

[Serge Gautherie (:sgautherie)]

(In reply to comment #1)
> I can't reproduce, from zip file, with either current or new profile.

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081113 SeaMonkey/2.0a2pre] (home, optim default) (W2Ksp4)

I can reproduce, presumably since some other packaging bug was fixed:
{
Warning: Warning: Duplicate resource declaration for 'gloda' ignored.
Source File: file:///.../objdir/mozilla/dist/bin/chrome/gloda.manifest
Line: 1
}

Comment 20

[Nomis101]

(In reply to comment #19)
> I can reproduce, presumably since some other packaging bug was fixed:
Ah, this would explane why I get the warning notwithstanding I applied this patch. And the patch worked fine 6 days ago.

Comment 21

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 347728 [details] [diff] [review]
Whack, corpses and blood everywhere, v.4

hooray for philor!  There was some minor rotting in the context, so I'm going to attach the de-rotted version of the patch.

Comment 22

[Andrew Sutherland [:asuth] (he/him)]

Attachment: philor's v4 with context failures de-rotted [Checkin: Comment 25]

This is just philor's v4 patch but applies cleanly to the recent trunk.

Comment 23

[David :Bienvenu]

Comment on attachment 348533 [details] [diff] [review]
philor's v4 with context failures de-rotted
[Checkin: Comment 25]

carrying forward reviews, adding checkin-needed keyword. I'll mark the bit-rotted patch obsolete, if it's not already

Comment 24

[Mark Banner (:standard8)]

(In reply to comment #23)
> (From update of attachment 348533 [details] [diff] [review])
> carrying forward reviews, adding checkin-needed keyword. I'll mark the
> bit-rotted patch obsolete, if it's not already

I forgot to say, Phil, can you co-ordinate landing of this with Andrew please? I think he wants to update extensions at the same time.

Comment 25

[Phil Ringnalda (:philor)]

Done, and with any luck, done.

http://hg.mozilla.org/comm-central/rev/641c14ca5210

Comment 26

[Serge Gautherie (:sgautherie)]

[Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9.1b2pre) Gecko/20081118 SeaMonkey/2.0a2pre] (home, optim default) (W2Ksp4)
(http://hg.mozilla.org/mozilla-central/rev/4b5e00c182be
+http://hg.mozilla.org/comm-central/rev/641c14ca5210)

V.Fixed