hotkeys throws new TypeError(INVALID_COMBINATION)

RESOLVED FIXED in 1.0

Status

P1
normal
RESOLVED FIXED
8 years ago
5 years ago

People

(Reporter: mcepl, Assigned: irakli)

Tracking

unspecified

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

(Reporter)

Description

8 years ago
Even the minimal module using hotkeys, e.g., the one consisting only of the following lib/main.js throws on console tracebacks:

"use strict";
var url = require("url");
var Hotkey = require("hotkeys");

function goUp() {
    var url = url.URL(require("window-utils").activeWindow.location.href);
    console.log("current URL is " + url);
}

var showHotKey = Hotkey({
  combo: "alt-pageup",
  onPress: function() {
    console.log("Pressed Alt+PgUp");
    goUp();
  }
});

The couple of such tracebacks is (and one runtime error on the top, but that's another issue):

bradford:testmodule $ cfx run -g dev
Using binary at '/usr/bin/firefox'.
Using profile at '/home/matej/.mozilla/firefox/rc1clvfj.devel_JP'.
*** e = [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: chrome://browser/content/utilityOverlay.js :: getShellService :: line 339"  data: no]
error: An exception occurred.
Traceback (most recent call last):
  File "resource://jid0-azgwvqko1xbfqeme0ajv76pwfe0-api-utils-lib/keyboard/observer.js", line 67, in handleEvent
    this._emit(event.type, event, event.target.ownerDocument.defaultView);
  File "resource://jid0-azgwvqko1xbfqeme0ajv76pwfe0-api-utils-lib/events.js", line 147, in _emit
    return this._emitOnObject.apply(this, args);
  File "resource://jid0-azgwvqko1xbfqeme0ajv76pwfe0-api-utils-lib/events.js", line 177, in _emitOnObject
    listener.apply(targetObj, params);
  File "resource://jid0-azgwvqko1xbfqeme0ajv76pwfe0-api-utils-lib/keyboard/hotkeys.js", line 129, in onKeypress
    let combination = normalize({ key: key, modifiers: modifiers });
  File "resource://jid0-azgwvqko1xbfqeme0ajv76pwfe0-api-utils-lib/keyboard/utils.js", line 113, in normalize
    return toString(toJSON(hotkey, separator), separator);
  File "resource://jid0-azgwvqko1xbfqeme0ajv76pwfe0-api-utils-lib/keyboard/utils.js", line 158, in toJSON
    throw new TypeError(INVALID_COMBINATION);
TypeError: Hotkey string must contain one or more modifiers and only one key
error: An exception occurred.
Traceback (most recent call last):
  File "resource://jid0-azgwvqko1xbfqeme0ajv76pwfe0-api-utils-lib/keyboard/observer.js", line 67, in handleEvent
    this._emit(event.type, event, event.target.ownerDocument.defaultView);
  File "resource://jid0-azgwvqko1xbfqeme0ajv76pwfe0-api-utils-lib/events.js", line 147, in _emit
    return this._emitOnObject.apply(this, args);
  File "resource://jid0-azgwvqko1xbfqeme0ajv76pwfe0-api-utils-lib/events.js", line 177, in _emitOnObject
    listener.apply(targetObj, params);
  File "resource://jid0-azgwvqko1xbfqeme0ajv76pwfe0-api-utils-lib/keyboard/hotkeys.js", line 129, in onKeypress
    let combination = normalize({ key: key, modifiers: modifiers });
  File "resource://jid0-azgwvqko1xbfqeme0ajv76pwfe0-api-utils-lib/keyboard/utils.js", line 113, in normalize
    return toString(toJSON(hotkey, separator), separator);
  File "resource://jid0-azgwvqko1xbfqeme0ajv76pwfe0-api-utils-lib/keyboard/utils.js", line 158, in toJSON
    throw new TypeError(INVALID_COMBINATION);
TypeError: Hotkey string must contain one or more modifiers and only one key
OK
Total time: 9.042711 seconds
Program terminated successfully.
bradford:testmodule $

Comment 1

8 years ago
I wrote a fix for this and submitted it as a pull request. 
https://github.com/mozilla/addon-sdk/pull/145
Matej thanks for the bug report!

hwiechers thanks for the fix. I have reviewed code and while overall it's looks good, I had few comments (see pull request) that needs to be addressed before we pull that in.
(Reporter)

Comment 3

8 years ago
(In reply to comment #2)
> Matej thanks for the bug report!
> 
> hwiechers thanks for the fix. I have reviewed code and while overall it's looks
> good, I had few comments (see pull request) that needs to be addressed before
> we pull that in.

Just to confirm, that with this pull request applied, I don't get any nonsensical error messages on the console.

Thanks a lot
Created attachment 525659 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/145

Pointer to Github pull-request
Created attachment 525660 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/145#

Pointer to Github pull-request
Comment on attachment 525660 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/145#

Attaching pull request for a review.
Attachment #525660 - Flags: review?(rFobic)
Attachment #525659 - Attachment is obsolete: true
OS: Linux → All
Priority: -- → P1
Hardware: x86_64 → All
Target Milestone: --- → 1.0
Over in the pull request, Irakli agreed to finish this up, so assigning it to him.
Assignee: nobody → rFobic
Created attachment 528822 [details] [diff] [review]
Patch ready to review

Same patch than initial proposal from pull request, but simplified and with review comment adressed.
Attachment #525660 - Attachment is obsolete: true
Attachment #525660 - Flags: review?(rFobic)
Attachment #528822 - Flags: review?(rFobic)
Comment on attachment 528822 [details] [diff] [review]
Patch ready to review

Review of attachment 528822 [details] [diff] [review]:

Thanks Alex looks good! Please add yourself to a contributor lists as well before landing it.
Attachment #528822 - Flags: review?(rFobic) → review+
Myk can you please authorize this change for landing. It's pretty important as otherwise hotkeys API generates error messages in console.
Yup, good point, a=myk for commission during freeze.
Keywords: checkin-needed
https://github.com/mozilla/addon-sdk/commit/58b365d6d989d3c06c554065e12bef6d382aaa36
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED

Updated

8 years ago
Keywords: checkin-needed
You need to log in before you can comment on or make changes to this bug.