Closed Bug 119746 Opened 23 years ago Closed 21 years ago

Should be able to set charset for each channel

Categories

(Other Applications :: ChatZilla, defect)

Product:
Other Applications
Component:
ChatZilla
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: kazhik, Assigned: rginda)

References

Details

(Whiteboard: cz-patched)

Attachments

(1 file, 9 obsolete files)

patch v7
15.38 KB, patch
rginda
: review+
Details | Diff | Splinter Review 
/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
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.
Attached 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.
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.
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.
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?

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.


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).

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

join_charset [#|&|+] <channel-name> <charset> [<key>]
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.
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. 


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.)
Actually, I thought "/charset gb2312" would set the default, and "/charset
#channel_name koi8-r" would override the charset for a specific channel.
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. 
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).
Attachment #65539 - Attachment is obsolete: true
Attached patch patch(chatzilla.properties) (obsolete) — — Splinter Review 

    
    

    
  


  

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch(irc.js) (obsolete)
        — 
          — Splinter Review
      

    


  

    
    

    
  
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.



    
    

    
  
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?

    
    

    
  
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?

    
    

    
  

      
      
      
      

        Attached 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

    
    

    
  
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.

    
    

    
  
> 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.

    
    

    
  

      
      
      
      

        Attached 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

    
    

    
  
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.

    
  
Whiteboard: cz-patched

    
    

    
  

      
      
      
      

        Attached 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

    
    

    
  
if ("Finish" in client.ucConverter)

    
    

    
  
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.

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch v5 (obsolete)
        — 
          — Splinter Review
      

    


  
Removed client.ucConverter.Finish().

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch v6 (obsolete)
        — 
          — Splinter Review
      

    


  
patch v5 can't handle multibyte channel name.

        Attachment #117364 -
        Attachment is obsolete: true

    
  

        Attachment #117377 -
        Flags: review?(rginda)

    
    

    
  

      
      
      
      

        Attached patch
          
          
        patch v7
        — 
          — Splinter Review
      

    


  
patch v6 doesn't convert NOTICE messages and have been broken
for the fix of bug 190747.

    
  

        Attachment #116595 -
        Attachment is obsolete: true

        Attachment #117377 -
        Attachment is obsolete: true

    
  

        Attachment #121623 -
        Flags: review?(rginda)

    
    

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

Looking forward to implementation.

Regards,
Pavel

    
    

    
  
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.

    
  
Depends on: 204347

    
  
Depends on: 204348

    
  
Blocks: 204347, 204348
No longer depends on: 204347, 204348

    
  

        Attachment #117377 -
        Flags: review?(rginda)

    
    

    
  
Chatzilla 0.8.31 was checked in. Now it's possible to set charset per channel.
Thank you!
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED

    
  

        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.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: