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

RESOLVED FIXED

Status

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

People

(Reporter: Mitch, Assigned: James Ross)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [cz-0.9.80])

Attachments

(1 attachment)

(Reporter)

Description

10 years ago
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.
(Assignee)

Comment 1

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

Comment 2

10 years ago
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? :-)
(Assignee)

Comment 3

10 years ago
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.
(Assignee)

Updated

10 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Assignee)

Comment 4

10 years ago
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.
(Assignee)

Comment 5

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

Comment 6

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

Comment 7

10 years ago
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+
(Assignee)

Comment 8

10 years ago
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).
(Assignee)

Comment 9

10 years ago
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
Last Resolved: 10 years ago
Resolution: --- → FIXED
Whiteboard: [cz-0.9.80]

Updated

10 years ago
Blocks: 412310
(Assignee)

Updated

10 years ago
Duplicate of this bug: 362374
You need to log in before you can comment on or make changes to this bug.