349268 - Nickname reclaim botches spaces

Status: RESOLVED FIXED

(Other Applications Graveyard :: ChatZilla, defect)

Description

[James Ross]

When we have a nickname with spaces in, a couple of things get botched:
  - we try to reclaim the nick, as "Foo Bar" (pref) != "Foo_Bar" (used)
  - we reclaim using "NICK Foo Bar" (doh) which ends up with just Foo.

A few possible solutions, not all mutually exclusive:
  - patch up default nickname for spaces (case I hit just now)
  - correct nickname pref when starting a connect
  - try to patch all [important] pref compares (sucky solution)

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/extensions/irc/xul/content/handlers.js&rev=1.134&mark=2046,2055#2042

Comment 1

[Stephanie Daugherty [:sdaugherty]]

Rephrasing this - spaces aren't possible in nicknames, it's entirely out of spec, and as the original reporter indicates, it causes undefined behavior with the nickname reclaim feature and probably elsewhere.

Comment 2

[Mitchell Field [:Mitch]]

Attachment: Patch v1.0

This patch does the following:
  - fixes existing space-underscore replacements
  - makes spaces in default nickname replaced with underscores
  - modifies nickname prompt dialog to replace spaces in nickname with
    underscores, and changes the dialog text accordingly.

Comment 3

[James Ross]

Comment on attachment 315801 [details] [diff] [review]
Patch v1.0

I'm not convinced the msg.nick.prompt change is necessary, but I'll let that go.

>-        e.nickname = prompt(MSG_NICK_PROMPT, curNick);
>+        e.nickname = prompt(MSG_NICK_PROMPT, curNick).replace(/ /g, "_");
>         if (e.nickname == null)
>             return;

prompt() will return |null| if the user was silly enough to cancel. That's what the check afterwards is. If it returns |null|, you'll get a JavaScript error saying |replace is not a function| or something to that effect. I'd suggest this code:

    e.nickname = prompt(MSG_NICK_PROMPT, curNick);
    if (e.nickname == null)
        return;
    e.nickname = e.nickname.replace(/ /g, "_");

Comment 4

[Mitchell Field [:Mitch]]

Attachment: [checked in] Patch v1.1

Comment 5

[James Ross]

Comment on attachment 316048 [details] [diff] [review]
[checked in] Patch v1.1

I'll take that!

Comment 6

[James Ross]

Comment on attachment 316048 [details] [diff] [review]
[checked in] Patch v1.1

Not sure this completely resolves the bug; do we know if it does?

Comment 7

[:Gijs (he/him)]

(In reply to comment #6)
> (From update of attachment 316048 [details] [diff] [review])
> Not sure this completely resolves the bug; do we know if it does?
> 

As far as I know, it should. There are currently no other consumers of changeNick, to my knowledge - except for the pref dialog, but that's bug 363940.

Comment 8

[James Ross]

Alright, marking FIXED.