Closed Bug 1179866 Opened 9 years ago Closed 9 years ago

Commands with numbers in them don't get executed

Categories

(Chat Core :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
Instantbird 44

People

(Reporter: freaktechnik, Assigned: freaktechnik)

Details

Attachments

(1 file, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:38.0) Gecko/20100101 Firefox/38.0
Build ID: 20150525141253

Steps to reproduce:

Register a command with a number in it in your prpl Protocol (in my case /r9kbeta and /r9kbetaoff).


Actual results:

When executing the command, Instantbird says, that it is not a supported command


Expected results:

The command gets executed

I think this check is responsible for the error: https://dxr.mozilla.org/comm-central/source/chat/components/src/imCommands.js#233
Attached patch bug-1179866-fix.patch (obsolete) — Splinter Review
This fixes commands with a number in their name not getting executed. It also adds tests for the executeCommand function in general.
Attachment #8668599 - Flags: review?(aleth)
Comment on attachment 8668599 [details] [diff] [review]
bug-1179866-fix.patch

Review of attachment 8668599 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, thanks! Just some nits.

::: chat/components/src/test/test_commands.js
@@ +63,3 @@
>    // Array of (possibly partial) command names as entered by the user.
>    let testCmds = ["x", "b", "ba", "bal", "back", "hel", "help", "off", "offline"];
> +  // Array of commands to test execution of.

Let's move this block above the executeCommand test as it's the test data for that one.

@@ +64,5 @@
>    let testCmds = ["x", "b", "ba", "bal", "back", "hel", "help", "off", "offline"];
> +  // Array of commands to test execution of.
> +  let executedTestCmds = [
> +    {
> +      command: "/r9kbeta",

These aren't commands, they are messages (the first parameter for executeCommand)

Maybe also add a message with a command with a parameter?

@@ +167,5 @@
>      }
>    }
>  
> +  // Test command execution.
> +  let conv = {

Nit: I'd call this fakeConversation and move it as a constant to the top of the file, under fakeAccount etc.
Attachment #8668599 - Flags: review?(aleth) → review-
Addressed the nits.
Attachment #8668599 - Attachment is obsolete: true
Attachment #8668663 - Flags: review?(aleth)
Comment on attachment 8668663 [details] [diff] [review]
bug-1179866-fix2.patch

Review of attachment 8668663 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8668663 - Flags: review?(aleth) → review+
Keywords: checkin-needed
Status: UNCONFIRMED → RESOLVED
Closed: 9 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 44
Assignee: nobody → martin
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: