Closed
Bug 396156
Opened 17 years ago
Closed 17 years ago
A warning before doing /list should be shown
Categories
(Other Applications :: ChatZilla, enhancement)
Other Applications
ChatZilla
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: stephane.gregoire, Assigned: rdmsoft)
Details
(Whiteboard: [cz-0.9.80])
Attachments
(1 file, 2 obsolete files)
7.02 KB,
patch
|
Gijs
:
review+
|
Details | Diff | Splinter Review |
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•17 years ago
|
||
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.
Updated•17 years ago
|
Version: unspecified → Trunk
Comment 2•17 years ago
|
||
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.
Assignee: rginda → gijskruitbosch+bugs
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #288312 -
Flags: review?(silver)
Comment 3•17 years ago
|
||
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. :(
Attachment #288312 -
Flags: review?(silver) → review+
Comment 4•17 years ago
|
||
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.
Assignee: gijskruitbosch+bugs → rginda
Status: ASSIGNED → NEW
Assignee | ||
Comment 5•17 years ago
|
||
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.
Assignee: rginda → rdmsoft
Status: NEW → ASSIGNED
Assignee | ||
Comment 6•17 years ago
|
||
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. :|
Attachment #290134 -
Flags: review?(silver)
Comment 7•17 years ago
|
||
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.
Attachment #290134 -
Flags: review?(silver) → review+
Comment 8•17 years ago
|
||
- Without server check (CMD_NEED_SRV) - Without breaking 80 char limit in comment - With rlist support - With extra set of braces around c comparison
Attachment #288312 -
Attachment is obsolete: true
Attachment #290134 -
Attachment is obsolete: true
Attachment #290990 -
Flags: review+
Comment 9•17 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Whiteboard: [cz-0.9.80]
You need to log in
before you can comment on or make changes to this bug.
Description
•