User-Agent: Mozilla/5.0 (X11; U; Linux i686; en; rv:220.127.116.11) Gecko/20061201 Epiphany/2.18 Firefox/18.104.22.168 (Ubuntu-feisty) Build Identifier: ChatZilla 0.9.78.1-rdmsoft [XULRunner 22.214.171.124/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...
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.
Created attachment 288312 [details] [diff] [review] 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 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. :(
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.
Created attachment 290134 [details] [diff] [review] 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 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 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.
Created attachment 290990 [details] [diff] [review] 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
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