Closed Bug 1134579 Opened 6 years ago Closed 6 years ago

IRC commands broken in nightly from Feb 19th 2015

Categories

(Chat Core :: IRC, defect)

x86
All
defect
Not set
major

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: nhnt11, Assigned: aleth)

Details

Attachments

(1 file, 1 obsolete file)

STR:
1) Try to use any IRC command
OR
2) Try to tab complete a command in an IRC conversation

Nothing happens, and the error console shows:
> Timestamp: 2/19/15, 4:51:32 PM
> Error: NS_NOINTERFACE: Component does not have requested interface [imICommand.usageContext]
> Source File: resource://gre/components/imCommands.js
> Line: 175
Adding 
command.__proto__ = ClassInfo("imICommand", "Chat command");
to jsProtoHelper::registerCommands fixes this.
Attached patch commandinterface.diff (obsolete) — Splinter Review
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Attachment #8566563 - Flags: review?(clokep)
Attachment #8566563 - Flags: review?(clokep) → review+
Comment on attachment 8566563 [details] [diff] [review]
commandinterface.diff

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

Thanks for debugging this! :-)

::: chat/modules/jsProtoHelper.jsm
@@ +817,5 @@
>      if (!this.commands)
>        return;
>  
>      this.commands.forEach(function(command) {
> +      command.QueryInterface = XPCOMUtils.generateQI([Ci.imICommand]);

This would override an existing implementation, which could be different if the object implemented more than one interface.
Attachment #8566563 - Flags: review+ → review-
OK, but I didn't use hasOwnProperty as if it *is* already implemented, it's likely in the prototype chain.
Attachment #8566563 - Attachment is obsolete: true
Attachment #8566637 - Flags: review?(clokep)
Comment on attachment 8566637 [details] [diff] [review]
commandinterface.diff v2

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

Florian is curious how hard it would be to add a test...any idea?
Attachment #8566637 - Flags: review?(clokep) → review+
(In reply to Patrick Cloke [:clokep] from comment #5)
> Florian is curious how hard it would be to add a test...any idea?

imCommands already has decent tests, the problem is that it didn't catch this as that test doesn't go through XPConnect.

A test that could catch things like this would have to set up a conversation with some prpl and then check if it can find commands, without using loadSubScript. Maybe we can think of a good way to do something like this...
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.6
You need to log in before you can comment on or make changes to this bug.