Should be able to set charset for each channel

RESOLVED FIXED

Status

defect
RESOLVED FIXED
18 years ago
15 years ago

People

(Reporter: kazhik, Assigned: rginda)

Tracking

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: cz-patched)

Attachments

(1 attachment, 9 obsolete attachments)

(Reporter)

Description

18 years ago
/charset updates client.charset and it's applied for 
all channel.

But if current channel A is in Japanese and channel B is
in Chinese, /charset should be applied only for channel A.

I think two commands are neccessary. For global(default) setting
and for channel setting.

Original report in Bugzilla-jp:
http://bugzilla.mozilla.gr.jp/show_bug.cgi?id=1695

Comment 1

18 years ago
I would vote against proliferating commands. How about an option under
/charset command?

/charset    -- for default setting for all channels. (preserved in prefs.js)
/charset charset_name #channel_name (or #current)  -- for per channel setting

The latter does not need to be preserved in prefs.js. This also matches
the behavior of the majority of users, who will use the default encoding
most of the time and may want to switch to another encoding for a specific
channel.
(Reporter)

Comment 2

18 years ago
Posted patch patch (obsolete) — Splinter Review
Command syntax:
/charset <charset> [all | channel]

Ex.
/charset euc-kr channel   # set charset for current channel
>The latter does not need to be preserved in prefs.js. This also matches
>the behavior of the majority of users, who will use the default encoding
>most of the time and may want to switch to another encoding for a
>specific channel.

I think there should be a pref GUI for setting the charset for specific channels
on specific networks.

Comment 4

17 years ago
Prefs is an overkill for this.

I think we should:
1. Add a View > Character Coding menu (just like in the browser).
2. Cache the setting for each network / channel.

No need to confuse the user with prefs.

Comment 5

17 years ago
Where do you expect to store this cache?  We don't mean to add prefs to the
prefs panel, we just mean to store the settings in the prefs file, so they are
persistant.

Comment 6

17 years ago
I have been waiting for this feature for a long time.
That would be nice to have the proposed patch as a temporary solution.
When will it be incorporated into nightly builds?
(Reporter)

Comment 7

17 years ago
My patch(attachment 65539 [details] [diff] [review]) is obsolete and doesn't consider
multibyte channel name(See bug 131498).

I have two ideas, but both of them aren't very good.

(1) Change channel command

join [#|&|+]<channel-name> [<key>] [<charset>]

There's no way to distinguish <key> and <charset>.

(2) Add a menu item [Character Coding]

Selected charset should be applied for current tab.
But we have to select a charset before creating tab.

Comment 8

17 years ago
I suggest to implement a menu with the list of channels (a kind of bookmarks),
where user can add/remove channels as well as set up specific properties (like
irc-net, nickname, charset etc.) for each one.  The menu will also provide
quick-join, cycle etc. features. 

Having similar menu the with list  of irc-nets is also a good idea. This will
allow user to specify for each irc-net a list of preferred irc-server and their
priority (I know, some people prefer to connect to irc-server in local country
before the others).
(Reporter)

Comment 9

17 years ago
Channel list is a good idea, but we should provide a command.
How about a new command?

join_charset [#|&|+] <channel-name> <charset> [<key>]
(Assignee)

Comment 10

17 years ago
I kind of like the idea of an optional channel parameter to the charset command,
and/or a new /channel-charset command that is a shortcut to set the charset only
on the current channel.

This channel specific charset information should probably also be included in
the irc: url syntax so the these channels can be bookmarked.  i.e.
irc://moznet/mozillazine-jp&charset=iso-2022-jp .  If the charset is worked into
irc: urls, we might be able to punt on storing the info in prefs for now.

Comment 11

17 years ago
Perhaps something that reflects the reality of chat channels in 
existence today. I gather many channels use Latin 1 (windows-1252 or 
iso-8859-1) as the default. This will cover many European and 
North & South American continents. For this, you need no specific 
charset info. 
Cases of Asian, Bidi, Cyrillic, Greek, Turkish and other scripts/encodings
can be recorded in pref files as exceptions to this rule. This way, 
we need not have many charset entries in the prefs.js. Just the ones 
users want to be specific for: e.g. Japanese, Chinese, Korean, etc. This way 
you can keep the pref file entries small not needing a line for every
chat channel you visit.

Do we need a special command for this? I am not sure if simply adding
the channel parameter to the current "charset" command may not
be enough for this. Of course, this means focring users to set a
charset for each channel. But if the user is doing predominantly, 
Cyrillic channels, we can deal with that by using "all" parameter.
To summarize,

/charset channel_name koi8-r
/charset all gb2312

might be sufficient. 

Comment 12

17 years ago
To extrapolate a bit more:

/charset channel_name koi8-r (= /charset koi8-r. To set channel specific
encoding,        
                               use the channel name or none. )
/charset all gb2312  (To set the default charset, you must include "all" parameter. 
                      This is diffrent from the current behavior. Hopefully
                      the transition will be smooth.)
(Assignee)

Comment 13

17 years ago
Actually, I thought "/charset gb2312" would set the default, and "/charset
#channel_name koi8-r" would override the charset for a specific channel.

Comment 14

17 years ago
rginda, I don't have string feeling for either.

My proposal is based on an assumption that we want "all"
parameter. If so, what I said above seems reasonable.
But this also means changing the current behavior since
right now "/charset koi8-r" would set that charset for
all channels. If people agree to let go of "all"
parameter, then your proposal fits my expectation also.
In fact that would be much simpler and preferred in my 
opinion. 
(Reporter)

Comment 15

17 years ago
rginda,momoi, please consider multibyte channel name.
charset for channel should be supplied with join command.
so I proposed join_charset command.

And the simplest way of changing charset command is:
(1) If the current object is channel, charset command
is applied for the channel.
(2) Otherwise, charset command is applied for global
charset(client.CHARSET).
(Reporter)

Updated

17 years ago
Attachment #65539 - Attachment is obsolete: true
(Reporter)

Comment 17

16 years ago
Posted patch patch(chatzilla.properties) (obsolete) — Splinter Review
(Reporter)

Comment 18

16 years ago
(Reporter)

Comment 19

16 years ago
Posted patch patch(irc.js) (obsolete) — Splinter Review
(Reporter)

Comment 20

16 years ago
attachment 114519 [details] [diff] [review], attachment 114520 [details] [diff] [review] and attachment 114521 [details] [diff] [review] are the patches for
cz0.8.19(http://www.hacksrus.com/~ginda/chatzilla/). They extends charset command
as described in comment 15 and adds a new "joinchar" command.

(Assignee)

Comment 21

16 years ago
Comment on attachment 114521 [details] [diff] [review]
patch(irc.js)

Thanks for taking the time to make these patches, here are a few comments...

>@@ -797,7 +797,12 @@
> function serv_topic (e)
> {
> 
>-    e.channel = new CIRCChannel (this, e.params[1]);
>+    var chname = e.params[1];
>+    for (var c in this.channels) {
>+       if (this.channels[c].encodedName == chname) {
>+          e.channel = this.channels[c];
>+       }
>+    }

The CIRCChannel constructor already does this check, no need for a change here.
 Also, bracing is wrong.  Chatzilla bracing is to have each on its own line, in
the same column as the "i" in if.

>@@ -1308,6 +1312,11 @@
>     if (userIsMe(e.user))
>         e.channel.active = false;
>     e.channel.removeUser(e.user.nick);
>+    for (var c in e.server.channels) {
>+       if (e.server.channels[c].unicodeName == e.params[1]) {
>+           delete e.server.channels[c];
>+       }
>+    }

What's this?  It looks like you're deleting the channel when a user parts.

>@@ -1336,7 +1345,12 @@
> function serv_join (e)
> {
> 
>-    e.channel = new CIRCChannel (this, e.params[1]);
>+    for (var c in this.channels) {
>+        if (this.channels[c].encodedName == e.params[1]) {
>+            e.channel = this.channels[c];
>+        }
>+    }
>+

Again, the CIRCChannel constructor takes care of this, no need for a change
here.

>-function CIRCChannel (parent, name)
>+function CIRCChannel (parent, name, charset)
> {
>+    for (var c in parent.channels) {
>+       if (parent.channels[c].unicodeName == name) {
>+           return parent.channels[c];
>+       }
>+    }
> 

Chatzilla bracing style, please.

>-    var encodedName = fromUnicode(name + " ");
>+    var encodedName = convUnicodeToChannelCharset(name + " ", charset);

I don't like this new name at all.  Let's just keep the toUnicode and
fromUnicode names. They are short, easy to remember (and type), and descriptive
names.	Adding an optional charset parameter to each should be enough.

>@@ -1754,6 +1773,11 @@
>     return i;
>     
> }
>+CIRCChannel.prototype.setCharset =
>+function chan_setcharset (charset)
>+{
>+    this.charset = charset;
>+}

Do we really need a setter for this?  Why not just have clients set
channel.charset directly?
(Assignee)

Comment 22

16 years ago
Comment on attachment 114520 [details] [diff] [review]
patch(commands.js, handlers.js, static.js)

>--- chatzilla.orig/content/chatzilla/commands.js	2003-01-22 06:33:26.000000000 +0900
>+++ chatzilla/content/chatzilla/commands.js	2003-02-14 12:29:16.000000000 +0900
>@@ -56,6 +56,7 @@
>     add ("infobar", "onInputInfobar");
>     add ("invite", "onInputInvite"); 
>     add ("join", "onInputJoin");
>+    add ("joinchar", "onInputJoinChar");

How about something more descriptive, like join-charset.  The length of the
command doesn't matter too much, as you can always [tab] complete the command.

>@@ -58,7 +58,8 @@
>     if (e.keyCode == 13)
>     {        
>         var line = stringTrim(e.target.value);
>-        client.currentObject.setTopic (fromUnicode(line));
>+        client.currentObject.setTopic (convUnicodeToChannelCharset(
>+            line, client.currentObject.charset));

Same comment as for irc.js.  Let's just add an optional parameter to
fromUnicode().

>@@ -1059,7 +1060,11 @@
> {
>     if (e.inputData)
>     {
>-        if (!setCharset(e.inputData))
>+        if (client.currentObject.TYPE == "IRCChannel")
>+        {
>+            client.currentObject.setCharset(e.inputData);
>+        }
>+        else if (!setCharset(e.inputData))

I don't think we should overload the meaning of the /charset command.  It would
be confusing to new users who do a /charset after joining, and are surprised to
see that the other channels did not change.  If you need to change the charset
of an existing channel, why just do a /join-charset again?

>@@ -1974,10 +1985,72 @@
...
>+client.onInputJoinChar =
>+function cli_ijoinchar (e)
>+{

Please refactor this so that this code is not duplicated.  Either implement
onInputJoinChar in terms of onInputJoin, or create a third, common funciton, to
do the heavy lifting.

>--- chatzilla.orig/content/chatzilla/static.js	2003-02-14 11:35:23.000000000 +0900
>+++ chatzilla/content/chatzilla/static.js	2003-02-15 11:47:50.000000000 +0900
>@@ -131,11 +131,32 @@
> function ucConvertIncomingMessage (e)
> {
>     var length = e.params.length;
>-    e.params[length - 1] = toUnicode(e.params[length - 1]);
>+    if (e.code == "PRIVMSG"
>+        && e.params[1][0] != "#" && e.params[1][0] != "&"
>+        && e.params[1][0] != "+" && e.params[1][0] != "!") {
>+  

&&'s should be at the end of the prior line.  Also, it would be nice to cache
e.params[1][0] up front, instead of doing the same thing four times.

>+        e.params[length - 1] = convGlobalCharsetToUnicode(e.params[length - 1]);
>+        return true;
>+    }
>+	for (var c in e.server.channels) {
>+	   if (e.server.channels[c].encodedName == e.params[length - 1] && e.code != "JOIN") {
>+		   e.meat = convChannelCharsetToUnicode(e.params[length - 1],
>+			   e.server.channels[c].charset);

That should line up with the open-paren, and the whole for loop needs to be
outdented two spaces.  Each indent level should be *exactly* 4 *spaces* (no
hard tabs.)  This code needs some coments, too.  Please explain why JOIN is
being special cased here.

ssieb has submitted a patch that removes e.meat.  That patch will be checked in
very soon, and has been available since chatzilla 0.8.14.  Please merge yout
patch with those changes.

>+	   }
>+       for (var i = 0; i < e.params.length; i++) {
>+	       if (e.server.channels[c].encodedName == e.params[i]) {
>+               e.params[i] = convChannelCharsetToUnicode(e.params[i],
>+                   e.server.channels[c].charset);
>+               e.params[length - 1] = convChannelCharsetToUnicode(e.params[length - 1],
>+                   e.server.channels[c].charset);
>+           }
>+       }

Wait, what's going on here?  First you check to see if the last parameter is a
channel, then you check *all* parameters?  Why the first check?
(Reporter)

Comment 23

16 years ago
Posted patch patch v2 (obsolete) — Splinter Review
I took rginda's advice, and added new function checkCharset() which was
used in joincharset command and charset command.

There are two remaining problems:

(1) I think it's necessary to delete the channel object when a user
parts. Isn't it?

(2) If we don't extend charset command, there is no way to see the
charset of current channel. A new command "channelcharset" is needed.
Do you like this?
Attachment #114519 - Attachment is obsolete: true
Attachment #114520 - Attachment is obsolete: true
Attachment #114521 - Attachment is obsolete: true
(Assignee)

Comment 24

16 years ago
Comment on attachment 114947 [details] [diff] [review]
patch v2

Sorry for the lag in response, I wanted to land 0.8.23 before getting back into
this.  This patch looks much better, thanks for making the changes.  More
comments below...

>@@ -56,6 +56,7 @@
>     add ("infobar", "onInputInfobar");
>     add ("invite", "onInputInvite"); 
>     add ("join", "onInputJoin");
>+    add ("joincharset", "onInputJoinCharset");
>     add ("kick", "onInputKick");
>     add ("leave", "onInputLeave");
>     add ("list", "onInputList");

I'd prefer join-charset here.  I know it'll be the first chatzilla command with
a dash, but I use that convention in venkman and have grown used to it.  We may
end up with more long command names when I get around to merging the venkman
command manager with chatzilla.

>diff -u chatzilla.orig/content/chatzilla/handlers.js chatzilla/content/chatzilla/handlers.js
>--- chatzilla.orig/content/chatzilla/handlers.js	2003-02-16 14:40:11.000000000 +0900
>+++ chatzilla/content/chatzilla/handlers.js	2003-02-20 08:59:52.000000000 +0900
>@@ -58,7 +58,8 @@
>     if (e.keyCode == 13)
>     {        
>         var line = stringTrim(e.target.value);
>-        client.currentObject.setTopic (fromUnicode(line));
>+        client.currentObject.setTopic (fromUnicode(
>+            line, client.currentObject.charset));

Indentation is off here.  Indented parameters should always line up with
the open paren.  If, as it appears to be in this case, the resulting line would
still be over 80 characters, then use an intermediate value.  For example...

>+        var charset = client.currentObject.charset;
>+        client.currentObject.setTopic (fromUnicode(line, charset));


>@@ -1059,7 +1060,18 @@
> {
>     if (e.inputData)
>     {
>-        if (!setCharset(e.inputData))
>+        if (client.currentObject.TYPE == "IRCChannel")
>+        {
>+            if(!checkCharset(e.inputData))
>+            {
>+                client.currentObject.display (getMsg("cli_charsetError",
>+                                                 e.inputData),
>+                                          "ERROR");
>+                return false;
>+            }

Again, small indentation problem, should look like...

>+                client.currentObject.display (getMsg("cli_charsetError",
>+                                                     e.inputData),
>+                                              "ERROR");

>@@ -1832,7 +1849,8 @@
> 
>     var msg = filterOutput(ary[2], "PRIVMSG", "ME!");
>     client.currentObject.display (msg, "PRIVMSG", "ME!", usr);
>-    usr.say (fromUnicode(ary[2]));
>+    usr.say (fromUnicode(ary[2],
>+        client.currentObject.charset));

more indentation.

>@@ -1959,9 +1975,32 @@
>                                                  e.network.name1), "ERROR");
>         return false;
>     }
>+
>+    var name;
>+    for (i in namelist)
>+    {
>+        name = namelist[i];
>+        if ((name[0] != "#") && (name[0] != "&") && (name[0] != "+") &&
>+            (name[0] != "!"))
>+            name = "#" + name;

When an if expression spans more than one line, the block should be braced,
like...

>+        if ((name[0] != "#") && (name[0] != "&") && (name[0] != "+") &&
>+            (name[0] != "!"))
>+        {
>+            name = "#" + name;
>+        }

However, situations like this are better solved with a regular expression...

>+        if (name[0].search(/[#&+!]/) != 0)
>+            name = "#" + name;


>diff -u chatzilla.orig/content/chatzilla/static.js chatzilla/content/chatzilla/static.js
>--- chatzilla.orig/content/chatzilla/static.js	2003-02-18 11:09:18.000000000 +0900
>+++ chatzilla/content/chatzilla/static.js	2003-02-20 09:07:00.000000000 +0900
>@@ -131,17 +131,47 @@
> function ucConvertIncomingMessage (e)
> {
>     var length = e.params.length;
>-    e.params[length - 1] = toUnicode(e.params[length - 1]);
>+    var channame = e.params[1];
>+    // Convert private message
>+    if (e.code == "PRIVMSG" &&
>+        channame[0] != "#" && channame[0] != "&" &&
>+        channame[0] != "+" && channame[0] != "!") {
>+   
>+        e.params[length - 1] = toUnicode(e.params[length - 1]);
>+        return true;
>+    }
>+	// Convert channel name and message
>+	for (var c in e.server.channels)
>+    {
>+        for (var i = 0; i < e.params.length; i++)
>+        {
>+	        if (e.server.channels[c].encodedName == e.params[i])
>+            {
>+                // channel name
>+                e.params[i] = toUnicode(e.params[i],
>+                    e.server.channels[c].charset);
>+                // channel message or topic
>+                if (i != length - 1)
>+                {
>+                    e.params[length - 1] = toUnicode(e.params[length - 1],
>+                        e.server.channels[c].charset);
>+                }
>+            }
>+        }
>+	}
>     return true;
> }
> 

I still don't think this is going to fly.  First, looping through every
parameter for every channel for every irc message is unacceptable.  Second, if
one of the parameters in a message just happens to have the same name as a
channel the user is on, we'll convert something we shouldn't have.  Perhaps
these conversions should be coded directly into the appropriate handlers in
irc.js instead.  Of course, irc.js has no knowledge of the client object, let
alone client.CHARSET, so this would have to be moved to, say,
CIRCServer.prototype.CHARSET.

>-function toUnicode (msg)
>+function toUnicode (msg, charset)
> {
>     if (!("ucConverter" in client))
>         return msg;
>     
>     /* XXX set charset again to force the encoder to reset, see bug 114923 */
>-    client.ucConverter.charset = client.CHARSET;
>+    if (typeof(charset) == "undefined")
>+        client.ucConverter.charset = client.CHARSET;
>+    else
>+        client.ucConverter.charset = charset;

typeof is a unary operator, and doesn't require parens.  use...

>+    if (typeof charset == "undefined")

instead.

>-function fromUnicode (msg)
>+function fromUnicode (msg, charset)
> {
>     if (!("ucConverter" in client))
>         return msg;
>     
>     /* XXX set charset again to force the encoder to reset, see bug 114923 */
>-    client.ucConverter.charset = client.CHARSET;
>+    if (typeof(charset) == "undefined")

same here.

>@@ -2288,7 +2347,14 @@
>             msg = filterOutput (msg, "PRIVMSG");
>             client.currentObject.display (msg, "PRIVMSG", "ME!",
>                                           client.currentObject);
>-            client.currentObject.say (fromUnicode(msg));
>+            if (client.currentObject.TYPE == "IRCChannel") {
>+                client.currentObject.say (
>+                    fromUnicode(msg,
>+                    client.currentObject.charset));

indentation is off here.

>+            } else {
>+                client.currentObject.say (
>+                    fromUnicode(msg));

no need for this line break.

>+            }
>             break;
> 
>         case "IRCClient":
>diff -u chatzilla.orig/content/chatzilla/lib/js/irc.js chatzilla/content/chatzilla/lib/js/irc.js
>--- chatzilla.orig/content/chatzilla/lib/js/irc.js	2003-02-14 11:31:35.000000000 +0900
>+++ chatzilla/content/chatzilla/lib/js/irc.js	2003-02-20 08:59:59.000000000 +0900
>@@ -1308,6 +1308,11 @@
>     if (userIsMe(e.user))
>         e.channel.active = false;
>     e.channel.removeUser(e.user.nick);
>+    for (var c in e.server.channels) {
>+       if (e.server.channels[c].unicodeName == e.params[1]) {
>+           delete e.server.channels[c];
>+       }
>+    }

again, don't delete the channel on part.  Anyone could be parting, it's not
necessarily the chatzilla user.  If you delete the channel the first time
someone parts, bad things will happen.	Chatzilla will take care of deleting
the channel later, in CIRCChannel.prototype.onPart of handlers.js.

>-function CIRCChannel (parent, name)
>+function CIRCChannel (parent, name, charset)
> {
>+    for (var c in parent.channels) {
>+       if (parent.channels[c].unicodeName == name) {
>+           return parent.channels[c];
>+       }
>+    }

We shouldn't need this loop.  Why not just use the unicode name as the key in
parent.channels?  More on this below...

> 
>-    var encodedName = fromUnicode(name + " ");
>+    var encodedName = fromUnicode(name + " ", charset);
>     /* bug 114923 */
>     encodedName = encodedName.substr(0,encodedName.length -1);
>-    var unicodeName = toUnicode(encodedName);
>+    var unicodeName = toUnicode(encodedName, charset);

I don't understand why we have to decode the name we just encoded.  Isn't
unicodeName == name?  If so, then we *are* using the unicode name as the key in
parent.channels, and we don't need those |for (c in parent.channels)| loops
anymore.  I realize the code was like that before, but it doesn't loop right to
me this time around :)


As far as the changing the character set of an existing channel goes, as I said
in comment 22, ``If you need to change the charset of an existing channel, why
[not] just do a /join-charset again?''.  You could always alias
/channel-charset to /join-charset in commands.js, if that makes you more
comfortable.
(Assignee)

Comment 25

16 years ago
> Of course, irc.js has no knowledge of the client object, let
> alone client.CHARSET, so this would have to be moved to, say,
> CIRCServer.prototype.CHARSET.

Strike that.  The specifics of the conversion are all hidden by fromUnicode(). 
No need to move client.CHARSET here.
(Reporter)

Comment 26

16 years ago
Posted patch patch (obsolete) — Splinter Review
(1) Added a new command "channel-charset" instead of extending
"charset".

(2) Removed ucConvertIncomingMessage(). Conversion is executed in
message handlers in irc.js.

(3) Removed the temporary code to avoid bug 114923.

(4) Included the fix for bug 194185.
Attachment #114947 - Attachment is obsolete: true
(Assignee)

Comment 27

16 years ago
Comment on attachment 116054 [details] [diff] [review]
patch


>+	client.currentObject.display (getMsg("cli_currentCharset",
>+                                         client.currentObject.charset),
>+                                  "INFO");
>+

Small indentation problem there.

>@@ -2856,7 +2929,7 @@
>     if (!(this.list.regexp) || e.params[2].match(this.list.regexp)
>                             || e.params[4].match(this.list.regexp))

Indentation issue there too.  || should be on the previous line, and second
line should be indented to just after the first open paren.

>@@ -3359,9 +3432,14 @@
> {
> 
>     if ("user" in e)
>+    {
>+        var msg = toUnicode(e.params.slice(1).join(" "),
>+                            e.channel.charset);
>         this.display (getMsg("my_cmodeMsg",
>-                             [toUnicode(e.params.slice(1).join(" ")),
>+                             //[e.params.slice(1).join(" "),
>+                             [msg,
>                               e.user.properNick]), "MODE", e.user, this);

No need for the comment.  Small whitespace issues too.

>+function toUnicode (msg, charset)
> {
>     if (!("ucConverter" in client))
>         return msg;
>     
>-    /* XXX set charset again to force the encoder to reset, see bug 114923 */
>-    client.ucConverter.charset = client.CHARSET;
>+    if (typeof charset == "undefined")
>+        client.ucConverter.charset = client.CHARSET;
>+    else
>+        client.ucConverter.charset = charset;
>+
>     try
>     {
>         return client.ucConverter.ConvertToUnicode(msg);
>@@ -149,22 +145,51 @@
>     catch (ex)
>     {
>         dd ("caught exception " + ex + " converting " + msg + " to charset " +
>-            client.CHARSET);
>+            charset);

Need to account for the fact that charset might not have been passed.  This
should probably look like this instead...

+    if (typeof charset == "undefined")
+	 charset = client.CHARSET;
+
+    client.ucConverter.charset = charset;
etc.

>-function fromUnicode (msg)
>+function fromUnicode (msg, charset)
> {
>     if (!("ucConverter" in client))
>         return msg;
>     
>-    /* XXX set charset again to force the encoder to reset, see bug 114923 */
>-    client.ucConverter.charset = client.CHARSET;
>-    return client.ucConverter.ConvertFromUnicode(msg);
>+    if (typeof charset == "undefined")
>+        client.ucConverter.charset = client.CHARSET;
>+    else
>+        client.ucConverter.charset = charset;
>+
>+    return client.ucConverter.ConvertFromUnicode(msg)
>+        + client.ucConverter.Finish();

Plus should end the previous line, but, does this work in all Mozilla 1.x
builds?  I thought Finish() was only added recently, or only just fixed
recently.

>@@ -2294,7 +2314,12 @@
>             msg = filterOutput (msg, "PRIVMSG");
>             client.currentObject.display (msg, "PRIVMSG", "ME!",
>                                           client.currentObject);
>-            client.currentObject.say (fromUnicode(msg));
>+            if (client.currentObject.TYPE == "IRCChannel") {
>+                var charset = client.currentObject.charset;
>+                client.currentObject.say (fromUnicode(msg, charset));
>+            } else {
>+                client.currentObject.say (fromUnicode(msg));
>+            }

Brace issues here.

>+CIRCServer.prototype.getUnicodeChannelName =
>+function serv_channame (encodedName)
>+{
>+    for (var c in this.channels)
>+    {
>+       if (this.channels[c].encodedName == encodedName)
>+       {
>+           return this.channels[c].unicodeName;
>+       }
>+    }
>+
>+    return encodedName;    
>+}
>+    
> CIRCServer.prototype.getUsersLength =
> function serv_chanlen()
> {
>@@ -703,6 +717,7 @@
> {
>     var ary;
>     var l = e.data;
>+dump("***:" + e.data + "\n");

No need to leave this in.


So except for some minor formatting issues, my only concern is that it might
not work properly on older 1x builds, due to the usConcerter.Finish() call. 
I'll make an XPI with these changes as soon as that is sorted out.
(Assignee)

Updated

16 years ago
Whiteboard: cz-patched
(Reporter)

Comment 28

16 years ago
Posted patch patch v4 (obsolete) — Splinter Review
bug 114923 was fixed in last December. This patch doesn't work with
Mozilla 1.2 and before. 

if (client.ucConverter.Finish() exists)
    return client.ucConverter.ConvertFromUnicode(msg)
	+ client.ucConverter.Finish();
else
    return client.ucConverter.ConvertFromUnicode(msg);
   
Is it possible to write a code like this? Or shouldn't we use
client.ucConverter.Finish()?
Attachment #116054 - Attachment is obsolete: true

Comment 29

16 years ago
if ("Finish" in client.ucConverter)
(Assignee)

Comment 30

16 years ago
yeah, the code that ssieb posted will test to see if ucConverter has a property
named "Finish".  I don't think we should depend on it though.  If there is a
workaround that works in <= mozilla 1.2, that also works with 1.3, we should
probably just stick with it.  A comment that says we're leaving in the hack for
backward compatibility would be nice too.

Comment 31

16 years ago
Posted patch patch v5 (obsolete) — Splinter Review
Removed client.ucConverter.Finish().

Comment 32

16 years ago
Posted patch patch v6 (obsolete) — Splinter Review
patch v5 can't handle multibyte channel name.
Attachment #117364 - Attachment is obsolete: true
(Reporter)

Updated

16 years ago
Attachment #117377 - Flags: review?(rginda)
(Reporter)

Comment 33

16 years ago
Posted patch patch v7Splinter Review
patch v6 doesn't convert NOTICE messages and have been broken
for the fix of bug 190747.
(Reporter)

Updated

16 years ago
Attachment #116595 - Attachment is obsolete: true
Attachment #117377 - Attachment is obsolete: true
(Reporter)

Updated

16 years ago
Attachment #121623 - Flags: review?(rginda)

Comment 34

16 years ago
I really need this feature to be able
to use Japanese and Russian in ChatZilla.

Looking forward to implementation.

Regards,
Pavel
(Assignee)

Comment 35

16 years ago
Thanks for the gentle reminder, sorry for letting this slip so long.

I just applied the patch to my tree and posted 0.8.27 to
http://www.hacksrus.com/~ginda/chatzilla/.  In addition to the patch as supplied
here, I added charset= support to irc: urls, and added an alias to /join, /j,
because now that we had two commands that started with the letter j, /j wasn't
enough to disambiguate the two.  I've updated the irc: url document
http://www.mozilla.org/projects/rt-messaging/chatzilla/irc-urls.html with the
changes, it will take a little while for the site to reflect the changes.

Updated

16 years ago
Depends on: 204347

Updated

16 years ago
Depends on: 204348

Updated

16 years ago
Blocks: 204347, 204348
No longer depends on: 204347, 204348

Updated

16 years ago
Attachment #117377 - Flags: review?(rginda)
(Reporter)

Comment 36

16 years ago
Chatzilla 0.8.31 was checked in. Now it's possible to set charset per channel.
Thank you!
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED
(Assignee)

Updated

16 years ago
Attachment #121623 - Flags: review?(rginda) → review+
Product: Core → Other Applications
You need to log in before you can comment on or make changes to this bug.