Closed
Bug 647907
Opened 13 years ago
Closed 13 years ago
hotkeys throws new TypeError(INVALID_COMBINATION)
Categories
(Add-on SDK Graveyard :: General, defect, P1)
Add-on SDK Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
1.0
People
(Reporter: mcepl, Assigned: irakli)
References
Details
Attachments
(1 file, 2 obsolete files)
2.94 KB,
patch
|
irakli
:
review+
|
Details | Diff | Splinter Review |
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 $
I wrote a fix for this and submitted it as a pull request. https://github.com/mozilla/addon-sdk/pull/145
Assignee | ||
Comment 2•13 years ago
|
||
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•13 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
Assignee | ||
Comment 4•13 years ago
|
||
Pointer to Github pull-request
Assignee | ||
Comment 5•13 years ago
|
||
Pointer to Github pull-request
Assignee | ||
Comment 6•13 years ago
|
||
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)
Assignee | ||
Updated•13 years ago
|
Attachment #525659 -
Attachment is obsolete: true
Updated•13 years ago
|
OS: Linux → All
Priority: -- → P1
Hardware: x86_64 → All
Target Milestone: --- → 1.0
Comment 7•13 years ago
|
||
Over in the pull request, Irakli agreed to finish this up, so assigning it to him.
Assignee: nobody → rFobic
Comment 8•13 years ago
|
||
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)
Assignee | ||
Comment 9•13 years ago
|
||
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+
Assignee | ||
Comment 10•13 years ago
|
||
Myk can you please authorize this change for landing. It's pretty important as otherwise hotkeys API generates error messages in console.
Comment 11•13 years ago
|
||
Yup, good point, a=myk for commission during freeze.
Updated•13 years ago
|
Keywords: checkin-needed
Assignee | ||
Comment 12•13 years ago
|
||
https://github.com/mozilla/addon-sdk/commit/58b365d6d989d3c06c554065e12bef6d382aaa36
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Updated•13 years ago
|
Keywords: checkin-needed
Updated•11 years ago
|
Blocks: sdk/hotkeys
You need to log in
before you can comment on or make changes to this bug.
Description
•