282680 - Channel modes outside of RFC1459 not displayed in header

Status: RESOLVED FIXED

(Other Applications Graveyard :: ChatZilla, defect)

Description

[timeless]

steps:
1. /sslserver irc.mozilla.org:6697 (or click the link)
2. /join #test
	[INFO]	Channel view for “#test” opened.
	-->|	YOU have joined #test
3. /mode #test +z
	=-=	Mode #test +z by sslmonkey
URL  	 ircs://irc.mozilla.org:6697/test   	Mode  	+n  	Users  	1, 1@, 0%, 0+
4. /mode #test
URL  	 ircs://irc.mozilla.org:6697/test   	Mode  	+n  	Users  	1, 1@, 0%, 0+
5. /server irc.mozilla.org
6. /join #test
	[INFO]	Channel view for “#test” opened.
	===	#test Cannot join channel (+z)
7. openssl s_client -connect irc.mozilla.org:6697
:mecha.mozilla.org NOTICE AUTH :*** Looking up your hostname...
nick probe
PING :44510080
user probe probe probe probe
pong :44510080
:probe MODE probe :+iz
join #test
:probe![e-mail] JOIN :#test
mode #test
:mecha.mozilla.org 324 probe #test +nz
:mecha.mozilla.org 329 probe #test 1108716838

expected results for a channel like #test where ssl says the mode is +zn:
URL  	 ircs://irc.mozilla.org:6697/test   	Mode  	+zn  	Users  	1, 1@, 0%, 0+

actual results:
URL  	 ircs://irc.mozilla.org:6697/test   	Mode  	+n  	Users  	1, 1@, 0%, 0+

Comment 1

[:Gijs (away/busy until Mar 10)]

SSL mode +z is not a universal IRC mode (the dancer-IRC server software, for
example, uses it to define a special kind of 'moderated' mode). ChatZilla
supports the modes described in RFC 1459, and doesn't actively do anything with
the rest of them (as far as I know). I suppose you could ask for the mode string
in the header to be updated at all times, using any mode that's listed in the
supports reply, but this would not be a request specific for this mode. Try
setting mode +V, or +S on moznet channels, and you'll notice they don't "work"
either.

Therefor, I think it may be better to change this bug to be a universal one for
the mode display than one just for this specific mode, as it's a global 'problem'.

Apart from that, we already have a bug for /mode, which even though it doesn't
revolve around supporting modes *already* set on a channel, it does involve
setting those modes in the first place. Whatever we do about either of these
bugs, I think you would want to use the same approach for both in any case.

Comment 2

[James Ross]

As Gijs says, this has nothing to do with SSL or +z.  It has also been on my
Todo list for a while, since adding support for 005 RPL_ISUPPORTS, such that
this is actually possible.

The +z error message can be pretty-fied if someone can be arsed getting the
message number, and we're sure it doesn't conflict with anything. ("/pref
debugMode t", get message, "/pref debugMode -")

Comment 3

[James Ross]

Attachment: Parse MODE according to 005 message

It's just occured to me that I should test this on modes not in the 005
message... well, it works well for moznet with all sorts of odd modes. :)

Comment 4

[James Ross]

Comment on attachment 192508 [details] [diff] [review]
Parse MODE according to 005 message

Whoops, please disregard the makexpi.sh changes and the __cz_suffix change. :)

Comment 5

[Robert Ginda]

+    // Add letters...
+    for (var t in { modeC: 0, modeD: 0 })
+        for (var m in this[t])
+            if (this[t][m])
+                str += m;
+
+    // Now add parameters.
+    for (var t in { modeC: 0 })
+        for (var m in this[t])
+            if (this[t][m])
+                str += " " + this[t][m];

That's pretty funky.  Why are you enumerating object literals and not arrays? 
And why have the second enumeration when you know it only contains "modeC". 
Those four for loops should have braces, too, seeing as how they span multiple
lines.

It seems like it'd be easier to read if you looped over the modeC properties in
one loop and the the modeD properties in another.  For the modeC propertiues,
you could append the parameters to a second string and combine the two at the end.

Comment 6

[:Gijs (away/busy until Mar 10)]

Attachment: Patch with rginda's nits fixed

Rginda, could you please review this so it can hopefully land? Would bring us
yet another bit closer to matching trunk with 0.9.68.5/6. :-)

Comment 7

[Robert Ginda]

Comment on attachment 196191 [details] [diff] [review]
Patch with rginda's nits fixed

I've been hacking on a cross browser dhtml version of chatzilla at work, and
one of the bit I'm hoping not for have to fork is irc.js.  That means we'll
need to avoid mozilla specific js in this file, which means no
__define[GS]etter__.

Comment 8

[:Gijs (away/busy until Mar 10)]

Comment on attachment 196191 [details] [diff] [review]
Patch with rginda's nits fixed

minused based on comment #7

Comment 9

[:Gijs (away/busy until Mar 10)]

Attachment: Patch working around getter/setter limitations

This patch uses a mapping object to see if a mode needs to change some kind of canonical e.channel.mode property as well. It's not as pretty as using getters / setters, but it should work on non-mozilla browsers if necessary.

Comment 10

[:Gijs (away/busy until Mar 10)]

Samuel, any chance you could review this? :-)

Comment 11

[James Ross]

checked in --> FIXED.