396156 - A warning before doing /list should be shown

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, enhancement)

Description

[Stéphane Grégoire (yamo)]

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en; rv:1.8.1.6) Gecko/20061201 Epiphany/2.18 Firefox/2.0.0.6 (Ubuntu-feisty)
Build Identifier: ChatZilla 0.9.78.1-rdmsoft [XULRunner 1.8.0.10/0000000000]

Hi, 
I think that a warning before doing /list should tell the Chatzilla user that this command is not recommended on large network.

Could Chatzilla prompt and will continue after typing yes?


Reproducible: Always

Steps to Reproduce:
1. Connect to a large netwaork like dalnet or ircnet
2a . use  "Join channel"
_or_ 
2b. type /list
3. wait for a very very long time ...
Actual Results:  
The CPU usage will be very big. ( I think It's impossible to solve this trouble)
After 2a see the bug 375825 (https://bugzilla.mozilla.org/show_bug.cgi?id=375825)
and after 2b ChatZilla become very slow.
 

Expected Results:  
Ask the user if he really wants to do the /list command

I know it's more an irc trouble that a ChatZilla trouble...

Comment 1

[:Gijs (he/him)]

I guess we could warn if the 254 reply showed there were lots of channels, maybe? From what I can tell most servers send that - at least the big networks do - so that could help, I suppose.

Comment 2

[:Gijs (he/him)]

Attachment: Patch

In addition to doing what I suggested, I took the liberty of removing some client.FOO constants that are no longer used anywhere in the code that I could see.

Comment 3

[James Ross]

Comment on attachment 288312 [details] [diff] [review]
Patch

I'd prefer the buttons labeled "List channels" and "Don't list channels" so they are explicit on the action.

"probably take very long" => "probably take a very long time".

"responsive, or" => "responsive or".

r=silver although I'm not thrilled about a modal dialog from a command. :(

Comment 4

[:Gijs (he/him)]

Abdicating - I asked tH and he said he wouldn't mind picking this up to do inline buttons instead of a dialog. Not sure how far along that is, CC-ing.

Comment 5

[Rob Marshall [tH]]

Attachment: use inline button instead

I think this is nicer. The user can "list anyway" by clicking the link or just repeating the command, and can "cancel" by doing nothing. Two things:
- Should we do this for /rlist as well? It would be consistent, but I'm not sure how widely rlist is used.
- Should the flag that suppresses the warning be reset? Currently you'll only see the warning once per session per network.

Comment 6

[Rob Marshall [tH]]

Comment on attachment 290134 [details] [diff] [review]
use inline button instead

Yes, it did take me this long to realise I hadn't requested review. :|

Comment 7

[James Ross]

Comment on attachment 290134 [details] [diff] [review]
use inline button instead

- Definately do it for /rlist as well; it's mentioned in the FAQ and can therefore be used by any old fool who needs hand-holding. :)
- I think the warning not being reset is fine for now.
- I like the "do it again" aspect of accepting the confirmation. :)

r=silver with the same code in /rlist.

Comment 8

[:Gijs (he/him)]

Attachment: For checkin

- Without server check (CMD_NEED_SRV)
- Without breaking 80 char limit in comment
- With rlist support
- With extra set of braces around c comparison

Comment 9

[:Gijs (he/him)]

Checking in mozilla/extensions/irc/js/lib/irc.js;
/cvsroot/mozilla/extensions/irc/js/lib/irc.js,v  <--  irc.js
new revision: 1.110; previous revision: 1.109
done
Checking in mozilla/extensions/irc/locales/en-US/chrome/chatzilla.properties;
/cvsroot/mozilla/extensions/irc/locales/en-US/chrome/chatzilla.properties,v  <--  chatzilla.properties
new revision: 1.146; previous revision: 1.145
done
Checking in mozilla/extensions/irc/xul/content/commands.js;
/cvsroot/mozilla/extensions/irc/xul/content/commands.js,v  <--  commands.js
new revision: 1.132; previous revision: 1.131
done
Checking in mozilla/extensions/irc/xul/content/static.js;
/cvsroot/mozilla/extensions/irc/xul/content/static.js,v  <--  static.js
new revision: 1.249; previous revision: 1.248
done