IRC commands broken in nightly from Feb 19th 2015

RESOLVED FIXED in 1.6

Status

defect
--
major
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: nhnt11, Assigned: aleth)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

Reporter

Description

4 years ago
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
Assignee

Comment 1

4 years ago
Adding 
command.__proto__ = ClassInfo("imICommand", "Chat command");
to jsProtoHelper::registerCommands fixes this.
Assignee

Comment 2

4 years ago
Posted 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-
Assignee

Comment 4

4 years ago
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+
Assignee

Comment 7

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

Updated

4 years ago
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.6
You need to log in before you can comment on or make changes to this bug.