Note: There are a few cases of duplicates in user autocompletion which are being worked on.

A warning before doing /list should be shown

RESOLVED FIXED

Status

Other Applications
ChatZilla
--
enhancement
RESOLVED FIXED
10 years ago
10 years ago

People

(Reporter: Stéphane Grégoire (yamo), Assigned: Rob Marshall [tH])

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [cz-0.9.80])

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

10 years ago
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

10 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

10 years ago
Version: unspecified → Trunk

Comment 2

10 years ago
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.
Assignee: rginda → gijskruitbosch+bugs
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #288312 - Flags: review?(silver)

Comment 3

10 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

10 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

10 years ago
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.
Assignee: rginda → rdmsoft
Status: NEW → ASSIGNED
(Assignee)

Comment 6

10 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

10 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

10 years ago
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
Attachment #288312 - Attachment is obsolete: true
Attachment #290134 - Attachment is obsolete: true
Attachment #290990 - Flags: review+

Comment 9

10 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
Last Resolved: 10 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.