131000 - IRC server port specification seems to not work...

Status: RESOLVED FIXED

(Other Applications Graveyard :: ChatZilla, defect)

Description

[Chris P. Ross]

If I try to use a bookmark that's set to the above URL, it will try to
connect to the right server, but to port 6667.  This is clearly not
what the URL intended.  Is this known broken?
This is chatzilla 0.8.5 on mozilla 0.9.7 on BSD/OS.

Also, if I try: "/server irc://chatterbox.notonthe.net:23508/" in an
open chatzilla window, it tries to connect to irc://irc/ (port 6667).
This is even *more* wrong.

Should this be two bugs?  Am I doing something wrong?

Comment 1

[Samuel Sieb]

You have a very old build, please try with a newer one, either 0.9.9 or a nightly.

Comment 2

[Chris P. Ross]

K.  Thought that might be related, but wasn't sure.  I'll have a 0.9.9 to
test against soon...

Comment 3

[Chris P. Ross]

It's even worse with 0.9.9.  If I try to go to that URL, it simply pops
up a dialog box saying:

i is not a registered protocol.

Huh?!?

Comment 4

[Chris P. Ross]

Actually, it's worse than that.  Doing the same process of "Install 0.8.5" from
the chatzilla page on hackrus [as root], it says it installed successfully,
and I should restart.  Which I did, again as root, to allow it to register.
I then exited, and started it up as myself.  Now the 'IRC Chat' is in the
Tasks menu, but when I start it up, it brings up a chatzilla window with no
tabs, and it doesn't ever process any input I give it in the bottom text input
area.
Seems to be totally broken.  But, it worked just fine (sans the mishandling
of URL's) under 0.9.7.
Is this expected to work right under 0.9.9?  Any idea what may be wrong?

Comment 5

[Samuel Sieb]

Chris, for your first problem, delete component.reg.  For the second one, 0.8.5
is too old and doesn't work with recent builds.

Comment 6

[Chris P. Ross]

- I'll try nuking components.reg and see what that changes.  Thanks...
- Well, you suggested I try it with 0.9.9 or a newer build.  Is there someplace
I can get a newer chatzilla than 0.8.5?  That's the newest one I see...

Comment 7

[Chris P. Ross]

I deleted component.reg, and ran mozilla as root (to re-register everything,
which listed registering -chat handler).  When I then run it again as myself,
I get the same behaviours, on all counts.  The dialog from the attempt to
use the bookmark, and a mostly non-functional chatzilla if I call it up
on it's own.
Will wait for more information on getting a chatzilla that works better in
0.9.9.  Thanks.

Comment 8

[Samuel Sieb]

The problem is you messed up chatzilla by installing an old version over it. 
Try re-installing 0.9.9 and it should work (you may need to delete component.reg).

Comment 9

[Samuel Sieb]

rginda just pointed out to me that the latest chatzilla.xpi is available at:
http://ftp.mozilla.org/pub/mozilla/nightly/2002-03-14-11-trunk/windows-xpi/chatzilla.xpi
(yes, windows-xpi is ok, because chatzilla is not platform-dependant)
You can just click that link and it will install.

Comment 10

[Chris P. Ross]

Okay.  Thanks.  With that XPI, I'm back to the previous state.  chatzilla
comes up and works okay, and the bookmark will now invoke chatzilla once again,
but it displays the same problems.  Going to a bookmark with the above URL
as it's target fails upon repeated attempts to open chatterbox.notonthe.net:6667.
There's still a port parsing problem in there somewhere.
This is with:
Chatzilla 0.8.6 [Mozilla/5.0 (X11; U; BSD/OS i386; en-US; rv:0.9.9) Gecko/20020312]

Comment 11

[Peter Van der Beken [:peterv]]

I'm seeing the same thing. Doing /attach irc://irc.w3.org:6665/ gives
"Connecting to irc.w3.org via irc.w3.org:6667, attempt 1 of 5...".

Comment 12

[Peter Van der Beken [:peterv]]

The problem seems to be that onInputServer
(http://lxr.mozilla.org/seamonkey/source/extensions/irc/xul/content/handlers.js#1415)
can't handle the string that it gets from
http://lxr.mozilla.org/seamonkey/source/extensions/irc/xul/content/static.js#1310.
The port seems to end up in ary[4]. I don't really understand that regexp in
onInputServer, so I'm not sure how to fix it.

Comment 13

[Chris P. Ross]

Attachment: Patch to regex in cli_iserver (handlers.js:1410)

Okay.  I did a little research on how JS regex's work, and figured out what the
problem is.  Or at least, know what's going on.
gotoIRCURL
(http://lxr.mozilla.org/seamonkey/source/extensions/irc/xul/content/static.js#1261)
is calling onInputServer with 'host port pass'.  So, in the case that there's
no pass, it gets (for above URL) "chatterbox.notonthe.net 23508 " (note the
trailing space).  It's this trailing space that isn't accounted for in the
regex.
Assuming it wouldn't be better to do some sort of more drastic re-think about
the parameter in question, simply making the change I'm attaching allows for
one space to trail the port number in the case that there's a port number and
no pass.  The space is absorbed otherwise as a separator in the case that there
is a pass in the inputData.
This solution look reasonable to folks?

Comment 14

[Robert Ginda]

The added \s? (maybe match one space) should probably be \s* (match 0 or more
spaces), but other than that it sounds reasonable to me.  That's a hideous
regex, and the server command has *no* reason to accept so many parameter
formats, but it'll all be ripped out when the command manager lands.  In the
mean time, I think we should check this in for 1.0 to get ports in urls working
again.

Comment 15

[Chris P. Ross]

Attachment: updated patch with rgrinda's suggestion

Okay.  I'm good with that.  I thought about that originally, but thought it
might "loosen" the constraints on the regexp matching of the parameter too
much.  But
if it's gonna all be replaced anyway, then it can't matter much.  If you're
good with it, I am.
And I whole-heartedly agree that we should get this committed soon, ie before
1.0. (or whatever the next bundled milestone is)

Comment 16

[Peter Van der Beken [:peterv]]

Comment on attachment 75317 [details] [diff] [review]
updated patch with rgrinda's suggestion

r=peterv

Comment 17

[Asa Dotzler [:asa]]

Comment on attachment 75317 [details] [diff] [review]
updated patch with rgrinda's suggestion

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 18

[Robert Ginda]

checked in, thanks for the patch.