Commands with numbers in them don't get executed

RESOLVED FIXED in Instantbird 44

Status

RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: freaktechnik, Assigned: freaktechnik)

Tracking

trunk
Instantbird 44

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

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

Comment 1

3 years ago
Created attachment 8668599 [details] [diff] [review]
bug-1179866-fix.patch

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 2

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

Comment 3

3 years ago
Created attachment 8668663 [details] [diff] [review]
bug-1179866-fix2.patch

Addressed the nits.
Attachment #8668599 - Attachment is obsolete: true
Attachment #8668663 - Flags: review?(aleth)

Comment 4

3 years ago
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+

Updated

3 years ago
Keywords: checkin-needed

Updated

3 years ago
Status: UNCONFIRMED → RESOLVED
Last Resolved: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Instantbird 44

Updated

3 years ago
Assignee: nobody → martin
You need to log in before you can comment on or make changes to this bug.