Closed Bug 408527 Opened 12 years ago Closed 12 years ago

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

Categories

(Other Applications :: ChatZilla, defect, minor)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: Mitch, Assigned: bugzilla-mozilla-20000923)

References

Details

(Whiteboard: [cz-0.9.80])

Attachments

(1 file)

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.
Certain commands (/kick and /ban, for example) cannot work, but some (/time, for example) could be extended, although not always wise.
Severity: normal → minor
OS: Windows XP → All
Hardware: PC → All
Summary: Context menu commands for a multiple-user selection are performed only on the first-placed selected user. → Some context menu commands don't support multiple (selection) users
Version: unspecified → Trunk
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? :-)
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
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).
Assignee: rginda → silver
Status: NEW → ASSIGNED
Attachment #296451 - Flags: review?(gijskruitbosch+bugs)
Just spotted some style errors; I've fixed the ones I've found locally.
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.
Attachment #296451 - Flags: review?(gijskruitbosch+bugs) → review+
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).
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.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [cz-0.9.80]
Blocks: 412310
Duplicate of this bug: 362374
You need to log in before you can comment on or make changes to this bug.