576745 - Make ChatZilla XPCOM component use new manifests and data tables

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, defect)

Description

[Jim Jeffery not reading bug-mail 1/2/11]

Following the landing on mozilla-central of bug https://bugzilla.mozilla.org/show_bug.cgi?id=568691 changing the way addons register with Firefox, Chatzilla no longer works. 

Tried installing into new profile, no other addons, and trying to start up the application I'm presented with a a Dialog-box asking me to choose an application, and offers 'Mibbit'.

No errors are in the console, and there is no indication / feedback  that the install failed or would not work. 

Tested using the latest trunk build from m-c and Chatzilla version: 0.9.86

Comment 1

[Jim Jeffery not reading bug-mail 1/2/11]

Tools->Chatzilla does open the application, but for some reason it won't start up when using the following link:

irc://irc.mozilla.org/firefox

Comment 2

[Jim Jeffery not reading bug-mail 1/2/11]

I was pointed to this file, and while I'm not a coder, it would appear that some updating to the registration will be required to work with latest Mozilla-central trunk builds. 

http://hg.mozilla.org/chatzilla/file/7560e73fd52f/js/lib/chatzilla-service.js

line: 398 and following ?

Comment 3

[Philip Chee]

See https://developer.mozilla.org/en/XPCOM/XPCOM_changes_in_Gecko_1.9.3

Comment 4

[Ian Neal]

See https://bugzilla.mozilla.org/attachment.cgi?id=455990 as a template perhaps?

Comment 5

[Ian Neal]

Attachment: Temporary fix patch [Checkin: Comment 13]

This patch:
* Adds the same temporary workaround being used by Venkman so it builds, it isn't a permanent solution but it does mean Chatzilla builds and can be used.

Anyone that can review it please feel free to take over the review.

Comment 6

[Robert Kaiser]

Comment on attachment 456634 [details] [diff] [review]
Temporary fix patch [Checkin: Comment 13]

I don't think Rob does many reviews these days, it's best to ask Gijs for that.

Comment 7

[Glen Mailer]

Can you link to the relevant venkman bug or something that explains what this change actually does?

Comment 8

[Robert Kaiser]

bug 568691 has the relevant venkman bug as a dependency as well as this one and should explain what this does and even more what it needs to end up doing - the current patch only suppresses the build error, but ends up making the commandline handler not work in trunk builds. The inspector bug that is a dependency of that bug shows how it ultimately needs to be solved to make that work as well.
The workaround can be one step to that and also makes SeaMonkey builds at least build again, which is quite important for us, as you can imagine.

Comment 9

[Glen Mailer]

Thanks, that's much clearer - the DOMi bug is bug 576910 so we don't all have to hover over the links looking for it.

Comment 10

[Philip Chee]

> Thanks, that's much clearer - the DOMi bug is bug 576910 so we don't all have
> to hover over the links looking for it.
Like how difficult is it to look at the dependency tree?

Comment 11

[Glen Mailer]

(In reply to comment #10)
> > Thanks, that's much clearer - the DOMi bug is bug 576910 so we don't all have
> > to hover over the links looking for it.
> Like how difficult is it to look at the dependency tree?

Ah, never noticed that button before. Excuse my ignorance :)

Comment 12

[James Ross]

Comment on attachment 456634 [details] [diff] [review]
Temporary fix patch [Checkin: Comment 13]

To fix the build, r+ from me, though I can't help but feel the flag should have been the other way around. ;)

Comment 13

[Ian Neal]

Comment on attachment 456634 [details] [diff] [review]
Temporary fix patch [Checkin: Comment 13]

http://hg.mozilla.org/chatzilla/rev/6a02dfb32fe5

Comment 14

[[e-mail]]

Attachment: Minimal fix

Assuming you still want to keep all the nsIModule stuff around for backward compatibility for older applications, this quick hack works...

Comment 15

[Mardeg]

(In reply to comment #14)
> Created attachment 456923 [details] [diff] [review]
> Minimal fix
Does this cover ircs:// links also?

Comment 16

[James Ross]

Comment on attachment 456923 [details] [diff] [review]
Minimal fix

I don't know the precise details of these new manifest options, but it all looks right; r+ as long as you've checked that the following work:
 - irc:// links from web pages
 - ircs:// links from web pages
 - -chat command-line flag

Comment 17

[Justin Wood (:Callek)]

Comment on attachment 456923 [details] [diff] [review]
Minimal fix

Yep, all works well. a+=Callek if you feel we need it since SM is closed-tree

Comment 18

[[e-mail]]

Pushed changeset ca24ef809133 to ssh://hg.mozilla.org/chatzilla/