From 44555e7863a66b906c47f0c94507d9e055922d3e Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Tue, 11 Apr 2017 15:44:42 +0100 Subject: Move keyboard utils to keydown and migrate normal/visual modes. --- content_scripts/mode_key_handler.coffee | 37 +----------- lib/dom_utils.coffee | 19 ++++++ lib/keyboard_utils.coffee | 103 +++++--------------------------- pages/vomnibar.coffee | 3 +- 4 files changed, 38 insertions(+), 124 deletions(-) diff --git a/content_scripts/mode_key_handler.coffee b/content_scripts/mode_key_handler.coffee index e206dbc6..914eeb6c 100644 --- a/content_scripts/mode_key_handler.coffee +++ b/content_scripts/mode_key_handler.coffee @@ -12,7 +12,6 @@ # consists of a (non-empty) list of such mappings. class KeyHandlerMode extends Mode - keydownEvents: {} setKeyMapping: (@keyMapping) -> @reset() setPassKeys: (@passKeys) -> @reset() # Only for tests. @@ -28,8 +27,6 @@ class KeyHandlerMode extends Mode super extend options, keydown: @onKeydown.bind this - keypress: @onKeypress.bind this - keyup: @onKeyup.bind this # We cannot track keyup events if we lose the focus. blur: (event) => @alwaysContinueBubbling => @keydownEvents = {} if event.target == window @@ -49,45 +46,17 @@ class KeyHandlerMode extends Mode keyChar = KeyboardUtils.getKeyCharString event isEscape = KeyboardUtils.isEscape event if isEscape and (@countPrefix != 0 or @keyState.length != 1) - @keydownEvents[event.keyCode] = true - @reset() - @suppressEvent + DomUtils.consumeKeyup event, => @reset() # If the help dialog loses the focus, then Escape should hide it; see point 2 in #2045. else if isEscape and HelpDialog?.isShowing() - @keydownEvents[event.keyCode] = true - HelpDialog.toggle() - @suppressEvent + DomUtils.consumeKeyup event, -> HelpDialog.toggle() else if isEscape @continueBubbling else if @isMappedKey keyChar - @keydownEvents[event.keyCode] = true - @handleKeyChar keyChar - else if not keyChar and (keyChar = KeyboardUtils.getKeyChar event) and - (@isMappedKey(keyChar) or @isCountKey keyChar) - # We will possibly be handling a subsequent keypress event, so suppress propagation of this event to - # prevent triggering page event listeners (e.g. Google instant Search). - @keydownEvents[event.keyCode] = true - @suppressPropagation + DomUtils.consumeKeyup event, => @handleKeyChar keyChar else @continueBubbling - onKeypress: (event) -> - keyChar = KeyboardUtils.getKeyCharString event - if @isMappedKey keyChar - @handleKeyChar keyChar - else if @isCountKey keyChar - digit = parseInt keyChar - @reset if @keyState.length == 1 then @countPrefix * 10 + digit else digit - @suppressEvent - else - @reset() - @continueBubbling - - onKeyup: (event) -> - return @continueBubbling unless event.keyCode of @keydownEvents - delete @keydownEvents[event.keyCode] - @suppressPropagation - # This tests whether there is a mapping of keyChar in the current key state (and accounts for pass keys). isMappedKey: (keyChar) -> (mapping for mapping in @keyState when keyChar of mapping)[0]? and not @isPassKey keyChar diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index 690d9969..df9ca390 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -315,6 +315,25 @@ DomUtils = false handlerStack.suppressEvent + consumeKeyup: (event, callback = null) -> + @suppressEvent event + keyChar = KeyboardUtils.getKeyCharString event + handlerStack.push + _name: "dom_utils/consumeKeyup" + keydown: (event) -> + @remove() + handlerStack.continueBubbling + keyup: (event) -> + return handlerStack.continueBubbling unless keyChar == KeyboardUtils.getKeyCharString event + @remove() + handlerStack.suppressEvent + # We cannot track keyup events if we lose the focus. + blur: (event) -> + @remove() if event.target == window + handlerStack.continueBubbling + callback?() + handlerStack.suppressEvent + # Adapted from: http://roysharon.com/blog/37. # This finds the element containing the selection focus. getElementWithFocus: (selection, backwards) -> diff --git a/lib/keyboard_utils.coffee b/lib/keyboard_utils.coffee index ead8c037..97fd8a75 100644 --- a/lib/keyboard_utils.coffee +++ b/lib/keyboard_utils.coffee @@ -10,22 +10,6 @@ KeyboardUtils = keyNames: { 37: "left", 38: "up", 39: "right", 40: "down", 32: "space", 8: "backspace" } - # This is a mapping of the incorrect keyIdentifiers generated by Webkit on Windows during keydown events to - # the correct identifiers, which are correctly generated on Mac. We require this mapping to properly handle - # these keys on Windows. See https://bugs.webkit.org/show_bug.cgi?id=19906 for more details. - keyIdentifierCorrectionMap: - "U+00C0": ["U+0060", "U+007E"] # `~ - "U+00BD": ["U+002D", "U+005F"] # -_ - "U+00BB": ["U+003D", "U+002B"] # =+ - "U+00DB": ["U+005B", "U+007B"] # [{ - "U+00DD": ["U+005D", "U+007D"] # ]} - "U+00DC": ["U+005C", "U+007C"] # \| - "U+00BA": ["U+003B", "U+003A"] # ;: - "U+00DE": ["U+0027", "U+0022"] # '" - "U+00BC": ["U+002C", "U+003C"] # ,< - "U+00BE": ["U+002E", "U+003E"] # .> - "U+00BF": ["U+002F", "U+003F"] # /? - init: -> if (navigator.userAgent.indexOf("Mac") != -1) @platform = "Mac" @@ -34,17 +18,7 @@ KeyboardUtils = else @platform = "Windows" - # We are migrating from using event.keyIdentifier to using event.key. For some period of time, we must - # support both. This wrapper can be removed once Chrome 52 is considered too old to support. getKeyChar: (event) -> - # We favor using event.keyIdentifier due to Chromium's currently (Chrome 51) incorrect implementataion of - # event.key; see #2147. - if event.keyIdentifier? - @getKeyCharUsingKeyIdentifier event - else - @getKeyCharUsingKey event - - getKeyCharUsingKey: (event) -> if event.keyCode of @keyNames @keyNames[event.keyCode] # It appears that event.key is not always defined (see #2453). @@ -59,44 +33,25 @@ KeyboardUtils = else "" - getKeyCharUsingKeyIdentifier: (event) -> - # Handle named keys. - keyCode = event.keyCode - if keyCode - if keyCode of @keyNames - return @keyNames[keyCode] - # Function keys. - if @keyCodes.f1 <= keyCode <= @keyCodes.f12 - return "f" + (1 + keyCode - keyCodes.f1) - - keyIdentifier = event.keyIdentifier - - # Not a letter. - if not keyIdentifier.startsWith "U+" - return "" - - # On Windows, the keyIdentifiers for non-letter keys are incorrect. See - # https://bugs.webkit.org/show_bug.cgi?id=19906 for more details. - if ((@platform == "Windows" || @platform == "Linux") && @keyIdentifierCorrectionMap[keyIdentifier]) - correctedIdentifiers = @keyIdentifierCorrectionMap[keyIdentifier] - keyIdentifier = if event.shiftKey then correctedIdentifiers[1] else correctedIdentifiers[0] - unicodeKeyInHex = "0x" + keyIdentifier.substring(2) - character = String.fromCharCode(parseInt(unicodeKeyInHex)).toLowerCase() - if event.shiftKey then character.toUpperCase() else character + getKeyCharString: (event) -> + if keyChar = @getKeyChar event + modifiers = [] - isPrimaryModifierKey: (event) -> if (@platform == "Mac") then event.metaKey else event.ctrlKey + keyChar = keyChar.toUpperCase() if event.shiftKey and keyChar.length == 1 + # These must be in alphabetical order (to match the sorted modifier order in Commands.normalizeKey). + modifiers.push "a" if event.altKey + modifiers.push "c" if event.ctrlKey + modifiers.push "m" if event.metaKey - isEscape: do -> + keyChar = [modifiers..., keyChar].join "-" + keyChar = "<#{keyChar}>" if 1 < keyChar.length + keyChar = mapKeyRegistry[keyChar] ? keyChar + keyChar - # TODO(smblott) Change this to use event.key. - (event) -> - event.keyCode == @keyCodes.ESC || do => - keyChar = @getKeyCharString event - # is mapped to Escape in Vim by default. - keyChar == "" + isEscape: (event) -> + # is mapped to Escape in Vim by default. + event.keyCode == @keyCodes.ESC || @getKeyCharString(event) == "" - # TODO. This is probably a poor way of detecting printable characters. However, it shouldn't incorrectly - # identify any of chrome's own keyboard shortcuts as printable. isPrintable: (event) -> return false if event.metaKey or event.ctrlKey or event.altKey keyChar = @@ -106,34 +61,6 @@ KeyboardUtils = @getKeyChar event keyChar.length == 1 - # Return the Vimium key representation for this keyboard event. Return a falsy value (the empty string or - # undefined) when no Vimium representation is appropriate. - getKeyCharString: (event) -> - switch event.type - when "keypress" - # Ignore modifier keys by themselves. - if 31 < event.keyCode - String.fromCharCode event.charCode - - # TODO(smblott). Currently all (almost?) keyhandling is being done on keydown. All legacy code related - # to key handling on keypress should be reviewed and probably removed. This is not being done right now - # (2017-03-22) because it is better to wait until we've verified that the change to keydown is indeed - # correct and reliable. - when "keydown" - if keyChar = @getKeyChar event - modifiers = [] - - keyChar = keyChar.toUpperCase() if event.shiftKey and keyChar.length == 1 - # These must be in alphabetical order (to match the sorted modifier order in Commands.normalizeKey). - modifiers.push "a" if event.altKey - modifiers.push "c" if event.ctrlKey - modifiers.push "m" if event.metaKey - - keyChar = [modifiers..., keyChar].join "-" - keyChar = "<#{keyChar}>" if 1 < keyChar.length - keyChar = mapKeyRegistry[keyChar] ? keyChar - keyChar - KeyboardUtils.init() root = exports ? window diff --git a/pages/vomnibar.coffee b/pages/vomnibar.coffee index 95ef8151..43db90c9 100644 --- a/pages/vomnibar.coffee +++ b/pages/vomnibar.coffee @@ -125,8 +125,7 @@ class VomnibarUI @lastAction = action = @actionFromKeyEvent event return true unless action # pass through - openInNewTab = @forceNewTab || - (event.shiftKey || event.ctrlKey || event.altKey || KeyboardUtils.isPrimaryModifierKey(event)) + openInNewTab = @forceNewTab || event.shiftKey || event.ctrlKey || event.altKey || event.metaKey if (action == "dismiss") @hide() else if action in [ "tab", "down" ] -- cgit v1.2.3 From 044990fd24f9d39b11147a8430c531c548eb1347 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Tue, 11 Apr 2017 15:55:55 +0100 Subject: Migrate link hints to keydown only. --- content_scripts/link_hints.coffee | 34 +++++++++------------------------- 1 file changed, 9 insertions(+), 25 deletions(-) diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 741d42cf..8f8c5318 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -172,7 +172,6 @@ class LinkHintsMode exitOnEscape: true exitOnClick: true keydown: @onKeyDownInMode.bind this - keypress: @onKeyPressInMode.bind this @hintMode.onExit (event) => if event?.type == "click" or (event?.type == "keydown" and @@ -230,10 +229,9 @@ class LinkHintsMode linkText: desc.linkText stableSortCount: ++@stableSortCount - # Handles and . + # Handles all keyboard events. onKeyDownInMode: (event) -> return if event.repeat - @keydownKeyChar = KeyboardUtils.getKeyChar(event).toLowerCase() previousTabCount = @tabCount @tabCount = 0 @@ -290,22 +288,12 @@ class LinkHintsMode else @tabCount = previousTabCount if event.ctrlKey or event.metaKey or event.altKey - return - - # We've handled the event, so suppress it and update the mode indicator. - DomUtils.suppressEvent event - - # Handles normal input. - onKeyPressInMode: (event) -> - return if event.repeat - - keyChar = String.fromCharCode(event.charCode).toLowerCase() - if keyChar - @markerMatcher.pushKeyChar keyChar, @keydownKeyChar - @updateVisibleMarkers() + return if event.repeat + if keyChar = KeyboardUtils.getKeyChar event + @markerMatcher.pushKeyChar keyChar + @updateVisibleMarkers() - # We've handled the event, so suppress it. - DomUtils.suppressEvent event + DomUtils.consumeKeyup event updateVisibleMarkers: (tabCount = 0) -> {hintKeystrokeQueue, linkTextKeystrokeQueue} = @markerMatcher @@ -449,7 +437,6 @@ class AlphabetHints # settings value, and preserves the legacy behavior (which always used keydown) for users which are # familiar with that behavior. Otherwise, we use keyChar from keypress, which admits non-Latin # characters. See #1722. - @useKeydown = /^[a-z0-9]*$/.test @linkHintCharacters @hintKeystrokeQueue = [] fillInMarkers: (hintMarkers) -> @@ -478,8 +465,8 @@ class AlphabetHints matchString = @hintKeystrokeQueue.join "" linksMatched: hintMarkers.filter (linkMarker) -> linkMarker.hintString.startsWith matchString - pushKeyChar: (keyChar, keydownKeyChar) -> - @hintKeystrokeQueue.push (if @useKeydown then keydownKeyChar else keyChar) + pushKeyChar: (keyChar) -> + @hintKeystrokeQueue.push keyChar popKeyChar: -> @hintKeystrokeQueue.pop() # For alphabet hints, always rotates the hints, regardless of modifiers. @@ -535,10 +522,7 @@ class FilterHints linksMatched: linksMatched userMightOverType: @hintKeystrokeQueue.length == 0 and 0 < @linkTextKeystrokeQueue.length - pushKeyChar: (keyChar, keydownKeyChar) -> - # For filtered hints, we *always* use the keyChar value from keypress, because there is no obvious and - # easy-to-understand meaning for choosing one of keyChar or keydownKeyChar (as there is for alphabet - # hints). + pushKeyChar: (keyChar) -> if 0 <= @linkHintNumbers.indexOf keyChar @hintKeystrokeQueue.push keyChar # We only accept and characters which are not used for splitting (e.g. "a", "b", etc., but not "-"). -- cgit v1.2.3 From bd66c90a6e678fe931bed75f9dda562adc9e4b7d Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Tue, 11 Apr 2017 16:00:16 +0100 Subject: Migrate marks to keydown only. --- content_scripts/marks.coffee | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/content_scripts/marks.coffee b/content_scripts/marks.coffee index 37b062ba..4a2a8203 100644 --- a/content_scripts/marks.coffee +++ b/content_scripts/marks.coffee @@ -35,8 +35,8 @@ Marks = indicator: "Create mark..." exitOnEscape: true suppressAllKeyboardEvents: true - keypress: (event) => - keyChar = String.fromCharCode event.charCode + keydown: (event) => + keyChar = KeyboardUtils.getKeyChar event @exit => if @isGlobalMark event, keyChar # We record the current scroll position, but only if this is the top frame within the tab. @@ -58,27 +58,27 @@ Marks = indicator: "Go to mark..." exitOnEscape: true suppressAllKeyboardEvents: true - keypress: (event) => + keydown: (event) => @exit => - markName = String.fromCharCode event.charCode - if @isGlobalMark event, markName + keyChar = KeyboardUtils.getKeyChar event + if @isGlobalMark event, keyChar # This key must match @getLocationKey() in the back end. - key = "vimiumGlobalMark|#{markName}" + key = "vimiumGlobalMark|#{keyChar}" Settings.storage.get key, (items) -> if key of items - chrome.runtime.sendMessage handler: 'gotoMark', markName: markName - HUD.showForDuration "Jumped to global mark '#{markName}'", 1000 + chrome.runtime.sendMessage handler: 'gotoMark', markName: keyChar + HUD.showForDuration "Jumped to global mark '#{keyChar}'", 1000 else - HUD.showForDuration "Global mark not set '#{markName}'", 1000 + HUD.showForDuration "Global mark not set '#{keyChar}'", 1000 else - markString = @localRegisters[markName] ? localStorage[@getLocationKey markName] + markString = @localRegisters[keyChar] ? localStorage[@getLocationKey keyChar] if markString? @setPreviousPosition() position = JSON.parse markString window.scrollTo position.scrollX, position.scrollY - @showMessage "Jumped to local mark", markName + @showMessage "Jumped to local mark", keyChar else - @showMessage "Local mark not set", markName + @showMessage "Local mark not set", keyChar root = exports ? window root.Marks = Marks -- cgit v1.2.3 From 388d866e995249a0be3154c349db2edac664a3fa Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Tue, 11 Apr 2017 16:05:18 +0100 Subject: Remove out-of-date comment. --- content_scripts/link_hints.coffee | 4 ---- 1 file changed, 4 deletions(-) diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 8f8c5318..e5c8b57b 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -433,10 +433,6 @@ class LinkHintsMode class AlphabetHints constructor: -> @linkHintCharacters = Settings.get("linkHintCharacters").toLowerCase() - # We use the keyChar from keydown if the link-hint characters are all "a-z0-9". This is the default - # settings value, and preserves the legacy behavior (which always used keydown) for users which are - # familiar with that behavior. Otherwise, we use keyChar from keypress, which admits non-Latin - # characters. See #1722. @hintKeystrokeQueue = [] fillInMarkers: (hintMarkers) -> -- cgit v1.2.3 From 8e4119f84bbde748eb595e6766dbb47a6cd0133c Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 13 Apr 2017 14:09:14 +0100 Subject: Rework tests for all key handling on keydown. --- content_scripts/link_hints.coffee | 5 +- content_scripts/mode_key_handler.coffee | 5 + content_scripts/vimium_frontend.coffee | 2 +- tests/dom_tests/dom_tests.coffee | 157 ++++++++++---------------------- tests/dom_tests/phantom_runner.coffee | 24 ----- 5 files changed, 57 insertions(+), 136 deletions(-) diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index e5c8b57b..4d9bbd1f 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -292,8 +292,11 @@ class LinkHintsMode if keyChar = KeyboardUtils.getKeyChar event @markerMatcher.pushKeyChar keyChar @updateVisibleMarkers() + DomUtils.consumeKeyup event + return - DomUtils.consumeKeyup event + # We've handled the event, so suppress it and update the mode indicator. + DomUtils.suppressEvent event updateVisibleMarkers: (tabCount = 0) -> {hintKeystrokeQueue, linkTextKeystrokeQueue} = @markerMatcher diff --git a/content_scripts/mode_key_handler.coffee b/content_scripts/mode_key_handler.coffee index 914eeb6c..82fdc0e6 100644 --- a/content_scripts/mode_key_handler.coffee +++ b/content_scripts/mode_key_handler.coffee @@ -54,7 +54,12 @@ class KeyHandlerMode extends Mode @continueBubbling else if @isMappedKey keyChar DomUtils.consumeKeyup event, => @handleKeyChar keyChar + else if @isCountKey keyChar + digit = parseInt keyChar + @reset if @keyState.length == 1 then @countPrefix * 10 + digit else digit + @suppressEvent else + @reset() @continueBubbling # This tests whether there is a mapping of keyChar in the current key state (and accounts for pass keys). diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 2331a8cf..b3a85bfb 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -687,4 +687,4 @@ root.bgLog = bgLog extend root, {handleEscapeForFindMode, handleEnterForFindMode, performFind, performBackwardsFind, enterFindMode, focusThisFrame} # These are exported only for the tests. -extend root, {installModes, installListeners} +extend root, {installModes} diff --git a/tests/dom_tests/dom_tests.coffee b/tests/dom_tests/dom_tests.coffee index 9088fe30..cbf60532 100644 --- a/tests/dom_tests/dom_tests.coffee +++ b/tests/dom_tests/dom_tests.coffee @@ -1,33 +1,11 @@ window.vimiumDomTestsAreRunning = true # Install frontend event handlers. -installListeners() HUD.init() Frame.registerFrameId chromeFrameId: 0 -installListener = (element, event, callback) -> - element.addEventListener event, (-> callback.apply(this, arguments)), true - getSelection = -> - window.getSelection().toString() - -# A count of the number of keyboard events received by the page (for the most recently-sent keystroke). E.g., -# we expect 3 if the keystroke is passed through (keydown, keypress, keyup), and 0 if it is suppressed. -pageKeyboardEventCount = 0 - -sendKeyboardEvent = (key) -> - pageKeyboardEventCount = 0 - response = window.callPhantom - request: "keyboard" - key: key - -sendKeyboardEvents = (keys) -> - sendKeyboardEvent ch for ch in keys.split() - -# These listeners receive events after the main frontend listeners, and do not receive suppressed events. -for type in [ "keydown", "keypress", "keyup" ] - installListener window, type, (event) -> - pageKeyboardEventCount += 1 + window.getSelection().toString() commandName = commandCount = null @@ -163,6 +141,16 @@ context "jsaction matching", linkHints.deactivateMode() assert.equal 0, hintMarkers.length +sendKeyboardEvent = (key, type="keydown", extra={}) -> + handlerStack.bubbleEvent type, extend extra, + type: type + key: key + preventDefault: -> + stopImmediatePropagation: -> + +sendKeyboardEvents = (keys) -> + sendKeyboardEvent key for key in keys.split "" + inputs = [] context "Test link hints for focusing input elements correctly", @@ -227,16 +215,16 @@ context "Test link hints for changing mode", should "change mode on shift", -> assert.equal "curr-tab", @linkHints.mode.name - sendKeyboardEvent "shift-down" + sendKeyboardEvent "Shift", "keydown", keyCode: keyCodes.shiftKey assert.equal "bg-tab", @linkHints.mode.name - sendKeyboardEvent "shift-up" + sendKeyboardEvent "Shift", "keyup", keyCode: keyCodes.shiftKey assert.equal "curr-tab", @linkHints.mode.name should "change mode on ctrl", -> assert.equal "curr-tab", @linkHints.mode.name - sendKeyboardEvent "ctrl-down" + sendKeyboardEvent "Control", "keydown", keyCode: keyCodes.ctrlKey assert.equal "fg-tab", @linkHints.mode.name - sendKeyboardEvent "ctrl-up" + sendKeyboardEvent "Control", "keyup", keyCode: keyCodes.ctrlKey assert.equal "curr-tab", @linkHints.mode.name context "Alphabetical link hints", @@ -247,6 +235,7 @@ context "Alphabetical link hints", stubSettings "linkHintCharacters", "ab" stub window, "windowIsFocused", -> true + document.getElementById("test-div").innerHTML = "" # Three hints will trigger double hint chars. createLinks 3 @linkHints = activateLinkHintsMode() @@ -258,12 +247,13 @@ context "Alphabetical link hints", should "label the hints correctly", -> hintMarkers = getHintMarkers() expectedHints = ["aa", "b", "ab"] + assert.equal 3, hintMarkers.length for hint, i in expectedHints assert.equal hint, hintMarkers[i].hintString should "narrow the hints", -> hintMarkers = getHintMarkers() - sendKeyboardEvent "A" + sendKeyboardEvent "a" assert.equal "none", hintMarkers[1].style.display assert.equal "", hintMarkers[0].style.display @@ -314,24 +304,25 @@ context "Filtered link hints", should "narrow the hints", -> hintMarkers = getHintMarkers() - sendKeyboardEvent "T" - sendKeyboardEvent "R" + sendKeyboardEvent "t" + sendKeyboardEvent "r" assert.equal "none", hintMarkers[0].style.display assert.equal "3", hintMarkers[1].hintString assert.equal "", hintMarkers[1].style.display - sendKeyboardEvent "A" + sendKeyboardEvent "a" assert.equal "1", hintMarkers[3].hintString - # This test is the same as above, but with an extra non-matching character. + # This test is the same as above, but with an extra non-matching character. The effect should be the + # same. should "narrow the hints and ignore typing mistakes", -> hintMarkers = getHintMarkers() - sendKeyboardEvent "T" - sendKeyboardEvent "R" - sendKeyboardEvent "X" + sendKeyboardEvent "t" + sendKeyboardEvent "r" + sendKeyboardEvent "x" assert.equal "none", hintMarkers[0].style.display assert.equal "3", hintMarkers[1].hintString assert.equal "", hintMarkers[1].style.display - sendKeyboardEvent "A" + sendKeyboardEvent "a" assert.equal "1", hintMarkers[3].hintString context "Image hints", @@ -428,9 +419,9 @@ context "Filtered link hints", should "use tab to select the active hint", -> sendKeyboardEvents "abc" assert.equal "8", @getActiveHintMarker() - sendKeyboardEvent "tab" + sendKeyboardEvent "Tab", "keydown", keyCode: keyCodes.tab assert.equal "7", @getActiveHintMarker() - sendKeyboardEvent "tab" + sendKeyboardEvent "Tab", "keydown", keyCode: keyCodes.tab assert.equal "9", @getActiveHintMarker() context "Input focus", @@ -576,93 +567,55 @@ context "Key mapping", should "set and call command handler", -> sendKeyboardEvent "m" assert.isTrue @handlerCalled - assert.equal 0, pageKeyboardEventCount should "not call command handler for pass keys", -> sendKeyboardEvent "p" assert.isFalse @handlerCalled - assert.equal 3, pageKeyboardEventCount should "accept a count prefix with a single digit", -> sendKeyboardEvent "2" sendKeyboardEvent "m" assert.equal 2, @handlerCalledCount - assert.equal 0, pageKeyboardEventCount should "accept a count prefix with multiple digits", -> sendKeyboardEvent "2" sendKeyboardEvent "0" sendKeyboardEvent "m" assert.equal 20, @handlerCalledCount - assert.equal 0, pageKeyboardEventCount should "cancel a count prefix", -> sendKeyboardEvent "2" sendKeyboardEvent "z" sendKeyboardEvent "m" assert.equal 1, @handlerCalledCount - assert.equal 0, pageKeyboardEventCount should "accept a count prefix for multi-key command mappings", -> - sendKeyboardEvent "2" + sendKeyboardEvent "5" sendKeyboardEvent "z" sendKeyboardEvent "p" - assert.equal 2, @handlerCalledCount - assert.equal 0, pageKeyboardEventCount + assert.equal 5, @handlerCalledCount should "cancel a key prefix", -> sendKeyboardEvent "z" sendKeyboardEvent "m" assert.equal 1, @handlerCalledCount - assert.equal 0, pageKeyboardEventCount should "cancel a count prefix after a prefix key", -> sendKeyboardEvent "2" sendKeyboardEvent "z" sendKeyboardEvent "m" assert.equal 1, @handlerCalledCount - assert.equal 0, pageKeyboardEventCount should "cancel a prefix key on escape", -> sendKeyboardEvent "z" - sendKeyboardEvent "escape" + sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC sendKeyboardEvent "p" assert.equal 0, @handlerCalledCount - should "not handle escape on its own", -> - sendKeyboardEvent "escape" - assert.equal 2, pageKeyboardEventCount - context "Normal mode", setup -> initializeModeState() - should "suppress mapped keys", -> - sendKeyboardEvent "m" - assert.equal 0, pageKeyboardEventCount - - should "not suppress unmapped keys", -> - sendKeyboardEvent "u" - assert.equal 3, pageKeyboardEventCount - - should "not suppress escape", -> - sendKeyboardEvent "escape" - assert.equal 2, pageKeyboardEventCount - - should "not suppress passKeys", -> - sendKeyboardEvent "p" - assert.equal 3, pageKeyboardEventCount - - should "suppress passKeys with a non-empty key state (a count)", -> - sendKeyboardEvent "5" - sendKeyboardEvent "p" - assert.equal 0, pageKeyboardEventCount - - should "suppress passKeys with a non-empty key state (a key)", -> - sendKeyboardEvent "z" - sendKeyboardEvent "p" - assert.equal 0, pageKeyboardEventCount - should "invoke commands for mapped keys", -> sendKeyboardEvent "m" assert.equal "m", commandName @@ -706,7 +659,7 @@ context "Normal mode", assert.equal 2, commandCount should "accept count prefixes of length 2", -> - sendKeyboardEvent "12" + sendKeyboardEvents "12" sendKeyboardEvent "m" assert.equal 12, commandCount @@ -763,19 +716,16 @@ context "Insert mode", initializeModeState() @insertMode = new InsertMode global: true - should "not suppress mapped keys in insert mode", -> - sendKeyboardEvent "m" - assert.equal 3, pageKeyboardEventCount - should "exit on escape", -> assert.isTrue @insertMode.modeIsActive - sendKeyboardEvent "escape" + sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC assert.isFalse @insertMode.modeIsActive should "resume normal mode after leaving insert mode", -> + assert.equal null, commandCount @insertMode.exit() sendKeyboardEvent "m" - assert.equal 0, pageKeyboardEventCount + assert.equal 1, commandCount context "Triggering insert mode", setup -> @@ -833,7 +783,7 @@ context "Caret mode", assert.equal "I", getSelection() should "exit caret mode on escape", -> - sendKeyboardEvent "escape" + sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC assert.equal "", getSelection() should "move caret with l and h", -> @@ -868,7 +818,7 @@ context "Caret mode", assert.equal "I", getSelection() sendKeyboardEvents "ww" assert.equal "a", getSelection() - sendKeyboardEvent "escape" + sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC new VisualMode assert.equal "a", getSelection() @@ -983,16 +933,14 @@ context "Mode utilities", test = new Mode exitOnEscape: true assert.isTrue test.modeIsActive - sendKeyboardEvent "escape" - assert.equal 0, pageKeyboardEventCount + sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC assert.isFalse test.modeIsActive should "not exit on escape if not enabled", -> test = new Mode exitOnEscape: false assert.isTrue test.modeIsActive - sendKeyboardEvent "escape" - assert.equal 2, pageKeyboardEventCount + sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC assert.isTrue test.modeIsActive should "exit on blur", -> @@ -1031,21 +979,21 @@ context "PostFindMode", assert.isFalse @postFindMode.modeIsActive should "suppress unmapped printable keys", -> - sendKeyboardEvent "m" - assert.equal 0, pageKeyboardEventCount + sendKeyboardEvent "a" + assert.equal null, commandCount should "be deactivated on click events", -> handlerStack.bubbleEvent "click", target: document.activeElement assert.isFalse @postFindMode.modeIsActive should "enter insert mode on immediate escape", -> - sendKeyboardEvent "escape" - assert.equal 0, pageKeyboardEventCount + sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC + assert.equal null, commandCount assert.isFalse @postFindMode.modeIsActive should "not enter insert mode on subsequent escapes", -> sendKeyboardEvent "a" - sendKeyboardEvent "escape" + sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC assert.isTrue @postFindMode.modeIsActive context "WaitForEnter", @@ -1057,14 +1005,14 @@ context "WaitForEnter", should "exit with success on Enter", -> assert.isTrue @waitForEnter.modeIsActive assert.isFalse @isSuccess? - sendKeyboardEvent "enter" + sendKeyboardEvent "Enter", "keydown", keyCode: keyCodes.enter assert.isFalse @waitForEnter.modeIsActive assert.isTrue @isSuccess? and @isSuccess == true should "exit without success on Escape", -> assert.isTrue @waitForEnter.modeIsActive assert.isFalse @isSuccess? - sendKeyboardEvent "escape" + sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC assert.isFalse @waitForEnter.modeIsActive assert.isTrue @isSuccess? and @isSuccess == false @@ -1075,17 +1023,6 @@ context "WaitForEnter", assert.isTrue @waitForEnter.modeIsActive assert.isFalse @isSuccess? -context "SuppressAllKeyboardEvents", - setup -> - initializeModeState() - - should "supress keyboard events", -> - sendKeyboardEvent "a" - assert.equal 3, pageKeyboardEventCount - new SuppressAllKeyboardEvents - sendKeyboardEvent "a" - assert.equal 0, pageKeyboardEventCount - context "GrabBackFocus", setup -> testContent = "" diff --git a/tests/dom_tests/phantom_runner.coffee b/tests/dom_tests/phantom_runner.coffee index 09d7d584..b91919bb 100644 --- a/tests/dom_tests/phantom_runner.coffee +++ b/tests/dom_tests/phantom_runner.coffee @@ -21,30 +21,6 @@ page.onError = (msg, trace) -> page.onResourceError = (resourceError) -> console.log(resourceError.errorString) -page.onCallback = (request) -> - switch request.request - when "keyboard" - switch request.key - when "escape" - page.sendEvent "keydown", page.event.key.Escape - page.sendEvent "keyup", page.event.key.Escape - when "enter" - page.sendEvent "keydown", page.event.key.Enter - page.sendEvent "keyup", page.event.key.Enter - when "tab" - page.sendEvent "keydown", page.event.key.Tab - page.sendEvent "keyup", page.event.key.Tab - when "shift-down" - page.sendEvent "keydown", page.event.key.Shift - when "shift-up" - page.sendEvent "keyup", page.event.key.Shift - when "ctrl-down" - page.sendEvent "keydown", page.event.key.Control - when "ctrl-up" - page.sendEvent "keyup", page.event.key.Control - else - page.sendEvent "keypress", request.key - testfile = path.join(path.dirname(system.args[0]), 'dom_tests.html') page.open testfile, (status) -> if status != 'success' -- cgit v1.2.3 From d03845e3151babbd63cf13e9e3d74d98351671f9 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 13 Apr 2017 14:57:26 +0100 Subject: Remove use of keyCodes entirely. event.keyCode` is depricated: - https://developer.mozilla.org/en-US/docs/Web/API/KeyboardEvent/keyCode --- content_scripts/link_hints.coffee | 28 ++++++++++++++-------------- content_scripts/mode_visual.coffee | 2 +- content_scripts/vimium_frontend.coffee | 4 ++-- lib/keyboard_utils.coffee | 15 +++++---------- pages/hud.coffee | 10 +++++----- pages/vomnibar.coffee | 8 ++++---- tests/dom_tests/dom_tests.coffee | 32 ++++++++++++++++---------------- 7 files changed, 47 insertions(+), 52 deletions(-) diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 4d9bbd1f..4481ae92 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -175,7 +175,7 @@ class LinkHintsMode @hintMode.onExit (event) => if event?.type == "click" or (event?.type == "keydown" and - (KeyboardUtils.isEscape(event) or event.keyCode in [keyCodes.backspace, keyCodes.deleteKey])) + (KeyboardUtils.isEscape(event) or event.key in ["Backspace", "Delete"])) HintCoordinator.sendMessage "exit", isSuccess: false # Note(philc): Append these markers as top level children instead of as child nodes to the link itself, @@ -239,25 +239,25 @@ class LinkHintsMode # NOTE(smblott) As of 1.54, the Ctrl modifier doesn't work for filtered link hints; therefore we only # offer the control modifier for alphabet hints. It is not clear whether we should fix this. As of # 16-03-28, nobody has complained. - modifiers = [keyCodes.shiftKey] - modifiers.push keyCodes.ctrlKey unless Settings.get "filterLinkHints" + modifiers = ["Shift"] + modifiers.push "Control" unless Settings.get "filterLinkHints" - if event.keyCode in modifiers and + if event.key in modifiers and @mode in [ OPEN_IN_CURRENT_TAB, OPEN_WITH_QUEUE, OPEN_IN_NEW_BG_TAB, OPEN_IN_NEW_FG_TAB ] @tabCount = previousTabCount # Toggle whether to open the link in a new or current tab. previousMode = @mode - keyCode = event.keyCode + key = event.key - switch keyCode - when keyCodes.shiftKey + switch key + when "Shift" @setOpenLinkMode(if @mode is OPEN_IN_CURRENT_TAB then OPEN_IN_NEW_BG_TAB else OPEN_IN_CURRENT_TAB) - when keyCodes.ctrlKey + when "Control" @setOpenLinkMode(if @mode is OPEN_IN_NEW_FG_TAB then OPEN_IN_NEW_BG_TAB else OPEN_IN_NEW_FG_TAB) handlerId = handlerStack.push keyup: (event) => - if event.keyCode == keyCode + if event.key == key handlerStack.remove() @setOpenLinkMode previousMode true # Continue bubbling the event. @@ -266,7 +266,7 @@ class LinkHintsMode # Therefore, we ensure that it's always removed when hint mode exits. See #1911 and #1926. @hintMode.onExit -> handlerStack.remove handlerId - else if event.keyCode in [ keyCodes.backspace, keyCodes.deleteKey ] + else if event.key in [ "Backspace", "Delete" ] if @markerMatcher.popKeyChar() @updateVisibleMarkers() else @@ -274,15 +274,15 @@ class LinkHintsMode # knows not to restart hints mode. @hintMode.exit event - else if event.keyCode == keyCodes.enter + else if event.key == "Enter" # Activate the active hint, if there is one. Only FilterHints uses an active hint. HintCoordinator.sendMessage "activateActiveHintMarker" if @markerMatcher.activeHintMarker - else if event.keyCode == keyCodes.tab + else if event.key == "Tab" @tabCount = previousTabCount + (if event.shiftKey then -1 else 1) @updateVisibleMarkers @tabCount - else if event.keyCode == keyCodes.space and @markerMatcher.shouldRotateHints event + else if event.key == " " and @markerMatcher.shouldRotateHints event @tabCount = previousTabCount HintCoordinator.sendMessage "rotateHints" @@ -884,7 +884,7 @@ class WaitForEnter extends Mode @push keydown: (event) => - if event.keyCode == keyCodes.enter + if event.key == "Enter" @exit() callback true # true -> isSuccess. else if KeyboardUtils.isEscape event diff --git a/content_scripts/mode_visual.coffee b/content_scripts/mode_visual.coffee index e4e4f541..cc1baf34 100644 --- a/content_scripts/mode_visual.coffee +++ b/content_scripts/mode_visual.coffee @@ -258,7 +258,7 @@ class VisualMode extends KeyHandlerMode _name: "#{@id}/enter/click" # Yank on . keypress: (event) => - if event.keyCode == keyCodes.enter + if event.key == "Enter" unless event.metaKey or event.ctrlKey or event.altKey or event.shiftKey @yank() return @suppressEvent diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index b3a85bfb..cdb23352 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -451,14 +451,14 @@ extend window, name: "focus-selector" exitOnClick: true keydown: (event) => - if event.keyCode == KeyboardUtils.keyCodes.tab + if event.key == "Tab" hints[selectedInputIndex].classList.remove 'internalVimiumSelectedInputHint' selectedInputIndex += hints.length + (if event.shiftKey then -1 else 1) selectedInputIndex %= hints.length hints[selectedInputIndex].classList.add 'internalVimiumSelectedInputHint' DomUtils.simulateSelect visibleInputs[selectedInputIndex].element @suppressEvent - else unless event.keyCode == KeyboardUtils.keyCodes.shiftKey + else unless event.key == "Shift" @exit() # Give the new mode the opportunity to handle the event. @restartBubbling diff --git a/lib/keyboard_utils.coffee b/lib/keyboard_utils.coffee index 97fd8a75..1a1524af 100644 --- a/lib/keyboard_utils.coffee +++ b/lib/keyboard_utils.coffee @@ -3,12 +3,9 @@ mapKeyRegistry = {} Utils?.monitorChromeStorage "mapKeyRegistry", (value) => mapKeyRegistry = value KeyboardUtils = - keyCodes: - { ESC: 27, backspace: 8, deleteKey: 46, enter: 13, ctrlEnter: 10, space: 32, shiftKey: 16, ctrlKey: 17, f1: 112, - f12: 123, tab: 9, downArrow: 40, upArrow: 38 } - + # This maps event.key key names to Vimium key names. keyNames: - { 37: "left", 38: "up", 39: "right", 40: "down", 32: "space", 8: "backspace" } + "ArrowLeft": "left", "ArrowUp": "up", "ArrowRight": "right", "ArrowDown": "down", " ": "space", "Backspace": "backspace" init: -> if (navigator.userAgent.indexOf("Mac") != -1) @@ -19,8 +16,8 @@ KeyboardUtils = @platform = "Windows" getKeyChar: (event) -> - if event.keyCode of @keyNames - @keyNames[event.keyCode] + if event.key of @keyNames + @keyNames[event.key] # It appears that event.key is not always defined (see #2453). else if not event.key? "" @@ -50,7 +47,7 @@ KeyboardUtils = isEscape: (event) -> # is mapped to Escape in Vim by default. - event.keyCode == @keyCodes.ESC || @getKeyCharString(event) == "" + event.key == "Escape" || @getKeyCharString(event) == "" isPrintable: (event) -> return false if event.metaKey or event.ctrlKey or event.altKey @@ -65,5 +62,3 @@ KeyboardUtils.init() root = exports ? window root.KeyboardUtils = KeyboardUtils -# TODO(philc): A lot of code uses this keyCodes hash... maybe we shouldn't export it as a global. -root.keyCodes = KeyboardUtils.keyCodes diff --git a/pages/hud.coffee b/pages/hud.coffee index af528203..98801930 100644 --- a/pages/hud.coffee +++ b/pages/hud.coffee @@ -16,21 +16,21 @@ document.addEventListener "keydown", (event) -> inputElement = document.getElementById "hud-find-input" return unless inputElement? # Don't do anything if we're not in find mode. - if (event.keyCode in [keyCodes.backspace, keyCodes.deleteKey] and inputElement.textContent.length == 0) or - event.keyCode == keyCodes.enter or KeyboardUtils.isEscape event + if (event.key in ["Backspace", "Delete"] and inputElement.textContent.length == 0) or + event.key == "Enter" or KeyboardUtils.isEscape event UIComponentServer.postMessage name: "hideFindMode" - exitEventIsEnter: event.keyCode == keyCodes.enter + exitEventIsEnter: event.key == "Enter" exitEventIsEscape: KeyboardUtils.isEscape event - else if event.keyCode == keyCodes.upArrow + else if event.key == "ArrowUp" if rawQuery = FindModeHistory.getQuery findMode.historyIndex + 1 findMode.historyIndex += 1 findMode.partialQuery = findMode.rawQuery if findMode.historyIndex == 0 setTextInInputElement inputElement, rawQuery findMode.executeQuery() - else if event.keyCode == keyCodes.downArrow + else if event.key == "ArrowDown" findMode.historyIndex = Math.max -1, findMode.historyIndex - 1 rawQuery = if 0 <= findMode.historyIndex then FindModeHistory.getQuery findMode.historyIndex else findMode.partialQuery setTextInInputElement inputElement, rawQuery diff --git a/pages/vomnibar.coffee b/pages/vomnibar.coffee index 43db90c9..edd5fbc9 100644 --- a/pages/vomnibar.coffee +++ b/pages/vomnibar.coffee @@ -106,17 +106,17 @@ class VomnibarUI if (KeyboardUtils.isEscape(event)) return "dismiss" else if (key == "up" || - (event.shiftKey && event.keyCode == keyCodes.tab) || + (event.shiftKey && event.key == "Tab") || (event.ctrlKey && (key == "k" || key == "p"))) return "up" - else if (event.keyCode == keyCodes.tab && !event.shiftKey) + else if (event.key == "Tab" && !event.shiftKey) return "tab" else if (key == "down" || (event.ctrlKey && (key == "j" || key == "n"))) return "down" - else if (event.keyCode == keyCodes.enter) + else if (event.key == "Enter") return "enter" - else if event.keyCode == keyCodes.backspace || event.keyCode == keyCodes.deleteKey + else if event.key in ["Backspace", "Delete"] return "delete" null diff --git a/tests/dom_tests/dom_tests.coffee b/tests/dom_tests/dom_tests.coffee index cbf60532..5439e119 100644 --- a/tests/dom_tests/dom_tests.coffee +++ b/tests/dom_tests/dom_tests.coffee @@ -215,16 +215,16 @@ context "Test link hints for changing mode", should "change mode on shift", -> assert.equal "curr-tab", @linkHints.mode.name - sendKeyboardEvent "Shift", "keydown", keyCode: keyCodes.shiftKey + sendKeyboardEvent "Shift", "keydown" assert.equal "bg-tab", @linkHints.mode.name - sendKeyboardEvent "Shift", "keyup", keyCode: keyCodes.shiftKey + sendKeyboardEvent "Shift", "keyup" assert.equal "curr-tab", @linkHints.mode.name should "change mode on ctrl", -> assert.equal "curr-tab", @linkHints.mode.name - sendKeyboardEvent "Control", "keydown", keyCode: keyCodes.ctrlKey + sendKeyboardEvent "Control", "keydown" assert.equal "fg-tab", @linkHints.mode.name - sendKeyboardEvent "Control", "keyup", keyCode: keyCodes.ctrlKey + sendKeyboardEvent "Control", "keyup" assert.equal "curr-tab", @linkHints.mode.name context "Alphabetical link hints", @@ -419,9 +419,9 @@ context "Filtered link hints", should "use tab to select the active hint", -> sendKeyboardEvents "abc" assert.equal "8", @getActiveHintMarker() - sendKeyboardEvent "Tab", "keydown", keyCode: keyCodes.tab + sendKeyboardEvent "Tab", "keydown" assert.equal "7", @getActiveHintMarker() - sendKeyboardEvent "Tab", "keydown", keyCode: keyCodes.tab + sendKeyboardEvent "Tab", "keydown" assert.equal "9", @getActiveHintMarker() context "Input focus", @@ -608,7 +608,7 @@ context "Key mapping", should "cancel a prefix key on escape", -> sendKeyboardEvent "z" - sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC + sendKeyboardEvent "Escape", "keydown" sendKeyboardEvent "p" assert.equal 0, @handlerCalledCount @@ -718,7 +718,7 @@ context "Insert mode", should "exit on escape", -> assert.isTrue @insertMode.modeIsActive - sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC + sendKeyboardEvent "Escape", "keydown" assert.isFalse @insertMode.modeIsActive should "resume normal mode after leaving insert mode", -> @@ -783,7 +783,7 @@ context "Caret mode", assert.equal "I", getSelection() should "exit caret mode on escape", -> - sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC + sendKeyboardEvent "Escape", "keydown" assert.equal "", getSelection() should "move caret with l and h", -> @@ -818,7 +818,7 @@ context "Caret mode", assert.equal "I", getSelection() sendKeyboardEvents "ww" assert.equal "a", getSelection() - sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC + sendKeyboardEvent "Escape", "keydown" new VisualMode assert.equal "a", getSelection() @@ -933,14 +933,14 @@ context "Mode utilities", test = new Mode exitOnEscape: true assert.isTrue test.modeIsActive - sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC + sendKeyboardEvent "Escape", "keydown" assert.isFalse test.modeIsActive should "not exit on escape if not enabled", -> test = new Mode exitOnEscape: false assert.isTrue test.modeIsActive - sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC + sendKeyboardEvent "Escape", "keydown" assert.isTrue test.modeIsActive should "exit on blur", -> @@ -987,13 +987,13 @@ context "PostFindMode", assert.isFalse @postFindMode.modeIsActive should "enter insert mode on immediate escape", -> - sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC + sendKeyboardEvent "Escape", "keydown" assert.equal null, commandCount assert.isFalse @postFindMode.modeIsActive should "not enter insert mode on subsequent escapes", -> sendKeyboardEvent "a" - sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC + sendKeyboardEvent "Escape", "keydown" assert.isTrue @postFindMode.modeIsActive context "WaitForEnter", @@ -1005,14 +1005,14 @@ context "WaitForEnter", should "exit with success on Enter", -> assert.isTrue @waitForEnter.modeIsActive assert.isFalse @isSuccess? - sendKeyboardEvent "Enter", "keydown", keyCode: keyCodes.enter + sendKeyboardEvent "Enter", "keydown" assert.isFalse @waitForEnter.modeIsActive assert.isTrue @isSuccess? and @isSuccess == true should "exit without success on Escape", -> assert.isTrue @waitForEnter.modeIsActive assert.isFalse @isSuccess? - sendKeyboardEvent "Escape", "keydown", keyCode: keyCodes.ESC + sendKeyboardEvent "Escape", "keydown" assert.isFalse @waitForEnter.modeIsActive assert.isTrue @isSuccess? and @isSuccess == false -- cgit v1.2.3 From ee83d1a9603a304c9328d3587e2db829fcec7aaf Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 13 Apr 2017 15:28:43 +0100 Subject: Ignore keybiard repeats. Keyboard repeats were interfering with smooth scrolling. But we should be ignoreing them anyway. --- lib/dom_utils.coffee | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index df9ca390..8764bdbe 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -318,19 +318,20 @@ DomUtils = consumeKeyup: (event, callback = null) -> @suppressEvent event keyChar = KeyboardUtils.getKeyCharString event - handlerStack.push - _name: "dom_utils/consumeKeyup" - keydown: (event) -> - @remove() - handlerStack.continueBubbling - keyup: (event) -> - return handlerStack.continueBubbling unless keyChar == KeyboardUtils.getKeyCharString event - @remove() - handlerStack.suppressEvent - # We cannot track keyup events if we lose the focus. - blur: (event) -> - @remove() if event.target == window - handlerStack.continueBubbling + unless event.repeat + handlerStack.push + _name: "dom_utils/consumeKeyup" + keydown: (event) -> + @remove() + handlerStack.continueBubbling + keyup: (event) -> + return handlerStack.continueBubbling unless keyChar == KeyboardUtils.getKeyCharString event + @remove() + handlerStack.suppressEvent + # We cannot track keyup events if we lose the focus. + blur: (event) -> + @remove() if event.target == window + handlerStack.continueBubbling callback?() handlerStack.suppressEvent -- cgit v1.2.3 From b88ee579c4471d27dbb4aba59215f5e85b7d32a2 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 13 Apr 2017 15:57:32 +0100 Subject: Avoid repeating Backspace and Delete keys. --- content_scripts/link_hints.coffee | 4 ++-- lib/keyboard_utils.coffee | 3 +++ pages/hud.coffee | 2 +- pages/vomnibar.coffee | 2 +- 4 files changed, 7 insertions(+), 4 deletions(-) diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 4481ae92..a95d2123 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -175,7 +175,7 @@ class LinkHintsMode @hintMode.onExit (event) => if event?.type == "click" or (event?.type == "keydown" and - (KeyboardUtils.isEscape(event) or event.key in ["Backspace", "Delete"])) + (KeyboardUtils.isEscape(event) or KeyboardUtils.isBackspace event)) HintCoordinator.sendMessage "exit", isSuccess: false # Note(philc): Append these markers as top level children instead of as child nodes to the link itself, @@ -266,7 +266,7 @@ class LinkHintsMode # Therefore, we ensure that it's always removed when hint mode exits. See #1911 and #1926. @hintMode.onExit -> handlerStack.remove handlerId - else if event.key in [ "Backspace", "Delete" ] + else if KeyboardUtils.isBackspace event if @markerMatcher.popKeyChar() @updateVisibleMarkers() else diff --git a/lib/keyboard_utils.coffee b/lib/keyboard_utils.coffee index 1a1524af..35e584e3 100644 --- a/lib/keyboard_utils.coffee +++ b/lib/keyboard_utils.coffee @@ -49,6 +49,9 @@ KeyboardUtils = # is mapped to Escape in Vim by default. event.key == "Escape" || @getKeyCharString(event) == "" + isBackspace: (event) -> + event.key in ["Backspace", "Delete"] + isPrintable: (event) -> return false if event.metaKey or event.ctrlKey or event.altKey keyChar = diff --git a/pages/hud.coffee b/pages/hud.coffee index 98801930..ac7059ec 100644 --- a/pages/hud.coffee +++ b/pages/hud.coffee @@ -16,7 +16,7 @@ document.addEventListener "keydown", (event) -> inputElement = document.getElementById "hud-find-input" return unless inputElement? # Don't do anything if we're not in find mode. - if (event.key in ["Backspace", "Delete"] and inputElement.textContent.length == 0) or + if (KeyboardUtils.isBackspace(event) and inputElement.textContent.length == 0) or event.key == "Enter" or KeyboardUtils.isEscape event UIComponentServer.postMessage diff --git a/pages/vomnibar.coffee b/pages/vomnibar.coffee index edd5fbc9..071604b7 100644 --- a/pages/vomnibar.coffee +++ b/pages/vomnibar.coffee @@ -116,7 +116,7 @@ class VomnibarUI return "down" else if (event.key == "Enter") return "enter" - else if event.key in ["Backspace", "Delete"] + else if KeyboardUtils.isBackspace event return "delete" null -- cgit v1.2.3 From 95cd386ce3ae09c45df8fbf7bda78cf8146b0b3d Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 13 Apr 2017 16:10:46 +0100 Subject: Do not reset key state for modifiers. --- content_scripts/mode_key_handler.coffee | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/content_scripts/mode_key_handler.coffee b/content_scripts/mode_key_handler.coffee index 82fdc0e6..d3e0d8fb 100644 --- a/content_scripts/mode_key_handler.coffee +++ b/content_scripts/mode_key_handler.coffee @@ -58,7 +58,7 @@ class KeyHandlerMode extends Mode digit = parseInt keyChar @reset if @keyState.length == 1 then @countPrefix * 10 + digit else digit @suppressEvent - else + else if keyChar @reset() @continueBubbling -- cgit v1.2.3 From 40ece51a53ee4042caad7854e415de56c0c69cab Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 13 Apr 2017 16:15:12 +0100 Subject: Continue bubbling unmapped events. --- content_scripts/mode_key_handler.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/content_scripts/mode_key_handler.coffee b/content_scripts/mode_key_handler.coffee index d3e0d8fb..f306ea06 100644 --- a/content_scripts/mode_key_handler.coffee +++ b/content_scripts/mode_key_handler.coffee @@ -58,8 +58,8 @@ class KeyHandlerMode extends Mode digit = parseInt keyChar @reset if @keyState.length == 1 then @countPrefix * 10 + digit else digit @suppressEvent - else if keyChar - @reset() + else + @reset() if keyChar @continueBubbling # This tests whether there is a mapping of keyChar in the current key state (and accounts for pass keys). -- cgit v1.2.3 From 4c32c0383964e178e0196a87df7fc7a4ad7f8f27 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 14 Apr 2017 07:48:56 +0100 Subject: Use event.code to detect/suppress keyup events. This avoids the possibility of leaking keyup events if the keys a released in a different order from that in which they were pressed. Also, replace suppressKeyupAfterEscape with this same mechanism. This fixes a bug (in master/1.59) whereby we leak the keyup event for `i` when entering insert mode. TODO: - `/`, `` leaks a keyup event - `i` leaks a keyup event --- content_scripts/mode.coffee | 2 +- content_scripts/mode_find.coffee | 2 +- content_scripts/mode_insert.coffee | 2 +- content_scripts/mode_key_handler.coffee | 2 +- lib/dom_utils.coffee | 19 +++---------------- 5 files changed, 7 insertions(+), 20 deletions(-) diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee index 2d8cc9cc..85187b2c 100644 --- a/content_scripts/mode.coffee +++ b/content_scripts/mode.coffee @@ -82,7 +82,7 @@ class Mode "keydown": (event) => return @continueBubbling unless KeyboardUtils.isEscape event @exit event, event.target - DomUtils.suppressKeyupAfterEscape handlerStack + DomUtils.consumeKeyup event # If @options.exitOnBlur is truthy, then it should be an element. The mode will exit when that element # loses the focus. diff --git a/content_scripts/mode_find.coffee b/content_scripts/mode_find.coffee index 63825600..77d3762d 100644 --- a/content_scripts/mode_find.coffee +++ b/content_scripts/mode_find.coffee @@ -48,7 +48,7 @@ class PostFindMode extends SuppressPrintable keydown: (event) => if KeyboardUtils.isEscape event @exit() - DomUtils.suppressKeyupAfterEscape handlerStack + DomUtils.consumeKeyup event else handlerStack.remove() @continueBubbling diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee index 73a24112..a4f1836d 100644 --- a/content_scripts/mode_insert.coffee +++ b/content_scripts/mode_insert.coffee @@ -26,7 +26,7 @@ class InsertMode extends Mode # An editable element in a shadow DOM is focused; blur it. @insertModeLock.blur() @exit event, event.target - DomUtils.suppressKeyupAfterEscape handlerStack + DomUtils.consumeKeyup event defaults = name: "insert" diff --git a/content_scripts/mode_key_handler.coffee b/content_scripts/mode_key_handler.coffee index f306ea06..1b3b21e7 100644 --- a/content_scripts/mode_key_handler.coffee +++ b/content_scripts/mode_key_handler.coffee @@ -38,7 +38,7 @@ class KeyHandlerMode extends Mode keydown: (event) => if KeyboardUtils.isEscape(event) and not @isInResetState() @reset() - DomUtils.suppressKeyupAfterEscape handlerStack + DomUtils.consumeKeyup event else @continueBubbling diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index 8764bdbe..3f0bd7f3 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -305,27 +305,13 @@ DomUtils = event.preventDefault() @suppressPropagation(event) - # Suppress the next keyup event for Escape. - suppressKeyupAfterEscape: (handlerStack) -> - handlerStack.push - _name: "dom_utils/suppressKeyupAfterEscape" - keyup: (event) -> - return true unless KeyboardUtils.isEscape event - @remove() - false - handlerStack.suppressEvent - consumeKeyup: (event, callback = null) -> - @suppressEvent event - keyChar = KeyboardUtils.getKeyCharString event + code = event.code unless event.repeat handlerStack.push _name: "dom_utils/consumeKeyup" - keydown: (event) -> - @remove() - handlerStack.continueBubbling keyup: (event) -> - return handlerStack.continueBubbling unless keyChar == KeyboardUtils.getKeyCharString event + return handlerStack.continueBubbling unless event.code == code @remove() handlerStack.suppressEvent # We cannot track keyup events if we lose the focus. @@ -333,6 +319,7 @@ DomUtils = @remove() if event.target == window handlerStack.continueBubbling callback?() + @suppressEvent event handlerStack.suppressEvent # Adapted from: http://roysharon.com/blog/37. -- cgit v1.2.3 From 17722aa93ba2f0fb08ee87dc76698fb37a2e0fd9 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 14 Apr 2017 13:30:58 +0100 Subject: Fix filtered link hints. For filtered link hints, " " was broken; it was treated as "space". --- content_scripts/link_hints.coffee | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index a95d2123..1cc7fee7 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -288,15 +288,14 @@ class LinkHintsMode else @tabCount = previousTabCount if event.ctrlKey or event.metaKey or event.altKey - return if event.repeat - if keyChar = KeyboardUtils.getKeyChar event - @markerMatcher.pushKeyChar keyChar - @updateVisibleMarkers() - DomUtils.consumeKeyup event - return - - # We've handled the event, so suppress it and update the mode indicator. - DomUtils.suppressEvent event + unless event.repeat + if keyChar = KeyboardUtils.getKeyChar event + keyChar = " " if keyChar == "space" + if keyChar.length == 1 + @markerMatcher.pushKeyChar keyChar + @updateVisibleMarkers() + + DomUtils.consumeKeyup event updateVisibleMarkers: (tabCount = 0) -> {hintKeystrokeQueue, linkTextKeystrokeQueue} = @markerMatcher -- cgit v1.2.3 From 1979d60f224453d9508e55d82a7ccb590a3a2912 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 14 Apr 2017 13:37:24 +0100 Subject: Make the consumeKeyup handler a singleton. That is, allow at most one handler to be installed at any one time. I have not observed this to be necessary. However, if there were any systematic way in which we weren't seeing the necessary keyup events, then these handlers would just be added and added. So this seems safer. --- lib/dom_utils.coffee | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index 3f0bd7f3..1864d973 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -305,22 +305,26 @@ DomUtils = event.preventDefault() @suppressPropagation(event) - consumeKeyup: (event, callback = null) -> - code = event.code - unless event.repeat - handlerStack.push - _name: "dom_utils/consumeKeyup" - keyup: (event) -> - return handlerStack.continueBubbling unless event.code == code - @remove() - handlerStack.suppressEvent - # We cannot track keyup events if we lose the focus. - blur: (event) -> - @remove() if event.target == window - handlerStack.continueBubbling - callback?() - @suppressEvent event - handlerStack.suppressEvent + consumeKeyup: do -> + handlerId = null + + (event, callback = null) -> + unless event.repeat + handlerStack.remove handlerId ? "not-an-id" + code = event.code + handlerId = handlerStack.push + _name: "dom_utils/consumeKeyup" + keyup: (event) -> + return handlerStack.continueBubbling unless event.code == code + @remove() + handlerStack.suppressEvent + # We cannot track keyup events if we lose the focus. + blur: (event) -> + @remove() if event.target == window + handlerStack.continueBubbling + callback?() + @suppressEvent event + handlerStack.suppressEvent # Adapted from: http://roysharon.com/blog/37. # This finds the element containing the selection focus. -- cgit v1.2.3 From 748f1dcc204d1c08d6deb683d896824a15fa4fe3 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 14 Apr 2017 13:57:20 +0100 Subject: Fix or link-hint behaviour. Error was introduced by seemingly innocuous but nevertheless significant change in previous commit. Tests picked up the problem. --- content_scripts/link_hints.coffee | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee index 1cc7fee7..eb138caa 100644 --- a/content_scripts/link_hints.coffee +++ b/content_scripts/link_hints.coffee @@ -294,8 +294,7 @@ class LinkHintsMode if keyChar.length == 1 @markerMatcher.pushKeyChar keyChar @updateVisibleMarkers() - - DomUtils.consumeKeyup event + DomUtils.consumeKeyup event updateVisibleMarkers: (tabCount = 0) -> {hintKeystrokeQueue, linkTextKeystrokeQueue} = @markerMatcher -- cgit v1.2.3 From b80f11b6f36b6302b1b5bb4d033681305fe961fc Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 14 Apr 2017 16:08:50 +0100 Subject: Set handlerId directly to a non-id. --- lib/dom_utils.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index 1864d973..074062a8 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -306,11 +306,11 @@ DomUtils = @suppressPropagation(event) consumeKeyup: do -> - handlerId = null + handlerId = "not-an-id" (event, callback = null) -> unless event.repeat - handlerStack.remove handlerId ? "not-an-id" + handlerStack.remove handlerId code = event.code handlerId = handlerStack.push _name: "dom_utils/consumeKeyup" -- cgit v1.2.3 From 41907f524275415cfd0f5336655a285105aa86e7 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 15 Apr 2017 07:47:07 +0100 Subject: Better check for handlerId. --- lib/dom_utils.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee index 074062a8..06db1b9b 100644 --- a/lib/dom_utils.coffee +++ b/lib/dom_utils.coffee @@ -306,11 +306,11 @@ DomUtils = @suppressPropagation(event) consumeKeyup: do -> - handlerId = "not-an-id" + handlerId = null (event, callback = null) -> unless event.repeat - handlerStack.remove handlerId + handlerStack.remove handlerId if handlerId? code = event.code handlerId = handlerStack.push _name: "dom_utils/consumeKeyup" -- cgit v1.2.3