408527 - Some context menu commands don't support multiple (selection) users

Status: RESOLVED FIXED

(Other Applications Graveyard :: ChatZilla, defect)

Description

[Mitchell Field [:Mitch]]

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]

Certain commands (/kick and /ban, for example) cannot work, but some (/time, for example) could be extended, although not always wise.

Comment 2

[:Gijs (he/him)]

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]

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]

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]

Attachment: 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]

Just spotted some style errors; I've fixed the ones I've found locally.

Comment 7

[:Gijs (he/him)]

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]

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]

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.