Last Comment Bug 408527 - Some context menu commands don't support multiple (selection) users
: Some context menu commands don't support multiple (selection) users
Status: RESOLVED FIXED
[cz-0.9.80]
:
Product: Other Applications
Classification: Client Software
Component: ChatZilla (show other bugs)
: Trunk
: All All
: -- minor (vote)
: ---
Assigned To: James Ross
:
Mentors:
: 362374 (view as bug list)
Depends on:
Blocks: 412310
  Show dependency treegraph
 
Reported: 2007-12-15 22:05 PST by Mitchell Field [:Mitch]
Modified: 2008-01-26 14:41 PST (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
Support multiple userlist users for /kick, /kick-ban, /ban and /unban (3.95 KB, patch)
2008-01-10 17:33 PST, James Ross
gijskruitbosch+bugs: review+
Details | Diff | Splinter Review

Description Mitchell Field [:Mitch] 2007-12-15 22:05:27 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.8.1.11) Gecko/20071127 Firefox/2.0.0.11
Build Identifier: ChatZilla 0.9.79-2007121516

Selecting more than one user on the user list, then trying to use commands from the context menu doesn't work correctly for the majority of commands.


Reproducible: Always

Steps to Reproduce:
1. Select multiple names on the user list
2. Right-click on any of the selected users
3. Select any command, except for the "...Status" and "Who is" commands.

Actual Results:  
The selected command is performed only on the user which appears first on the list.


Expected Results:  
The selected command should have been performed on all selected users sequentially.
Comment 1 James Ross 2007-12-15 22:38:44 PST
Certain commands (/kick and /ban, for example) cannot work, but some (/time, for example) could be extended, although not always wise.
Comment 2 :Gijs Kruitbosch (away 26-29 incl.) 2007-12-16 03:24:25 PST
Why couldn't we extend kick and ban? I agree we should consider carefully whether to extend time, version and ping, but I'm not entirely clear why we couldn't manually dispatch multiple /ban-s from cmdBan if we detect we were fired from the context menu. Am I missing something? :-)
Comment 3 James Ross 2007-12-17 06:50:51 PST
They already accept other arguments, and therefore cannot have the argument [<...>] needed to accept a list. If the *List property makes it from the (menu) context to the (command) event even without the argument [<...>], perhaps it can just pretend it supports it without having [<...>] in the params setting.
Comment 4 James Ross 2008-01-10 16:46:47 PST
This turns out to not be too hard, with the following code added to the top of cmdKick for example:

if (e.userList) {
    for (var i = 0; i < e.userList.length; i++) {
        e.sourceObject.dispatch("kick", { user: e.userList[i], nickname:
                                          e.userList[i].canonicalName });
    }
    return;
}

I'll sort out a patch soon-ish.
Comment 5 James Ross 2008-01-10 17:33:07 PST
Created attachment 296451 [details] [diff] [review]
Support multiple userlist users for /kick, /kick-ban, /ban and /unban

This adds support for multiple users being selected in the kick, kick-ban, ban and unban operator commands. It uses code similar to /op and friends to ensure mode changes are grouped (into 4s).
Comment 6 James Ross 2008-01-10 17:35:37 PST
Just spotted some style errors; I've fixed the ones I've found locally.
Comment 7 :Gijs Kruitbosch (away 26-29 incl.) 2008-01-12 13:17:26 PST
Comment on attachment 296451 [details] [diff] [review]
Support multiple userlist users for /kick, /kick-ban, /ban and /unban

>Index: xul/content/commands.js
>===================================================================
> <snip>
> function cmdKick(e)
> {
>+    if (e.userList) {
>+        if (e.command.name == "kick-ban")
>+        {
>+            e.sourceObject.dispatch("ban", { userList: e.userList,
>+                                             canonNickList: e.canonNickList,
>+                                             user: e.user,
>+                                             nickname: e.user.encodedName });
>+        }
>+
>+        // Note that we always do /kick below; the /ban is covered above.
>+        for (var i = 0; i < e.userList.length; i++) {
>+            var e2 = { user: e.userList[i],
>+                       nickname: e.userList[i].encodedName };
>+            e.sourceObject.dispatch("kick", e2);
>+        }
>+        return;
>+    }
>+

Why are you passing a nickname, on both these occasions? The code, if I read it correctly, doesn't use that if it also has a user object. Please check whether I'm making sense and either remove it or comment (either here on in code, depending on how obvious the need for the nickname is).

Then there are several bracing issues, but I presume you've fixed those per your previous comment.

r=me with all of that fixed.
Comment 8 James Ross 2008-01-12 13:22:31 PST
The 'nickname' argument is required for the dispatch code, as the commands in question all have a single, required <nickname> argument. The code will, as you say, use the user object instead, but the commands wont be executed without a non-empty nickname argument (and I thought it better to actually use something real, just in case).
Comment 9 James Ross 2008-01-12 13:44:33 PST
Checked in --> FIXED.

I think the other commands (on the User Commands submenu) should be left alone for now. Specific bugs for any that might be appropriate for multiple users.
Comment 10 James Ross 2008-01-26 14:41:57 PST
*** Bug 362374 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.