Merge pull request #2470 from smblott-github/rework-key-handling-to-keydown

Rework key handling to keydown

14 files changed, 147 insertions, 344 deletions

diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee
index 741d42cf..eb138caa 100644
--- a/content_scripts/link_hints.coffee
+++ b/content_scripts/link_hints.coffee
@@ -172,11 +172,10 @@ 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
-        (KeyboardUtils.isEscape(event) or event.keyCode in [keyCodes.backspace, keyCodes.deleteKey]))
+        (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,
@@ -230,10 +229,9 @@ class LinkHintsMode
       linkText: desc.linkText
       stableSortCount: ++@stableSortCount
 
-  # Handles <Shift> and <Ctrl>.
+  # Handles all keyboard events.
   onKeyDownInMode: (event) ->
     return if event.repeat
-    @keydownKeyChar = KeyboardUtils.getKeyChar(event).toLowerCase()
 
     previousTabCount = @tabCount
     @tabCount = 0
@@ -241,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.
@@ -268,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 KeyboardUtils.isBackspace event
       if @markerMatcher.popKeyChar()
         @updateVisibleMarkers()
       else
@@ -276,36 +274,27 @@ 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"
 
     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()
-
-    # We've handled the event, so suppress it.
-    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
@@ -445,11 +434,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.
-    @useKeydown = /^[a-z0-9]*$/.test @linkHintCharacters
     @hintKeystrokeQueue = []
 
   fillInMarkers: (hintMarkers) ->
@@ -478,8 +462,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, <Space> always rotates the hints, regardless of modifiers.
@@ -535,10 +519,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 <Space> and characters which are not used for splitting (e.g. "a", "b", etc., but not "-").
@@ -901,7 +882,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/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
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 e206dbc6..1b3b21e7 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
 
@@ -41,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
 
@@ -49,45 +46,22 @@ 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
-    else
-      @continueBubbling
-
-  onKeypress: (event) ->
-    keyChar = KeyboardUtils.getKeyCharString event
-    if @isMappedKey keyChar
-      @handleKeyChar 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()
+      @reset() if keyChar
       @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/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 <Enter>.
       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 2331a8cf..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
@@ -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/lib/dom_utils.coffee b/lib/dom_utils.coffee
index 690d9969..06db1b9b 100644
--- a/lib/dom_utils.coffee
+++ b/lib/dom_utils.coffee
@@ -305,15 +305,26 @@ 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: do ->
+    handlerId = null
+
+    (event, callback = null) ->
+      unless event.repeat
+        handlerStack.remove handlerId if handlerId?
+        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.
diff --git a/lib/keyboard_utils.coffee b/lib/keyboard_utils.coffee
index ead8c037..35e584e3 100644
--- a/lib/keyboard_utils.coffee
+++ b/lib/keyboard_utils.coffee
@@ -3,28 +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" }
-
-  # 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"] # /?
+    "ArrowLeft": "left", "ArrowUp": "up", "ArrowRight": "right", "ArrowDown": "down", " ": "space", "Backspace": "backspace"
 
   init: ->
     if (navigator.userAgent.indexOf("Mac") != -1)
@@ -34,19 +15,9 @@ 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]
+    if event.key of @keyNames
+      @keyNames[event.key]
     # It appears that event.key is not always defined (see #2453).
     else if not event.key?
       ""
@@ -59,44 +30,28 @@ 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 ""
+  getKeyCharString: (event) ->
+    if keyChar = @getKeyChar event
+      modifiers = []
 
-    # 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
+      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
 
-  isPrimaryModifierKey: (event) -> if (@platform == "Mac") then event.metaKey else event.ctrlKey
+      keyChar = [modifiers..., keyChar].join "-"
+      keyChar = "<#{keyChar}>" if 1 < keyChar.length
+      keyChar = mapKeyRegistry[keyChar] ? keyChar
+      keyChar
 
-  isEscape: do ->
+  isEscape: (event) ->
+    # <c-[> is mapped to Escape in Vim by default.
+    event.key == "Escape" || @getKeyCharString(event) == "<c-[>"
 
-    # TODO(smblott) Change this to use event.key.
-    (event) ->
-      event.keyCode == @keyCodes.ESC || do =>
-        keyChar = @getKeyCharString event
-        # <c-[> is mapped to Escape in Vim by default.
-        keyChar == "<c-[>"
+  isBackspace: (event) ->
+    event.key in ["Backspace", "Delete"]
 
-  # 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,37 +61,7 @@ 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
 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..ac7059ec 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 (KeyboardUtils.isBackspace(event) 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 95ef8151..071604b7 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 KeyboardUtils.isBackspace event
       return "delete"
 
     null
@@ -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" ]
diff --git a/tests/dom_tests/dom_tests.coffee b/tests/dom_tests/dom_tests.coffee
index 9088fe30..5439e119 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"
     assert.equal "bg-tab", @linkHints.mode.name
-    sendKeyboardEvent "shift-up"
+    sendKeyboardEvent "Shift", "keyup"
     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"
     assert.equal "fg-tab", @linkHints.mode.name
-    sendKeyboardEvent "ctrl-up"
+    sendKeyboardEvent "Control", "keyup"
     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"
       assert.equal "7", @getActiveHintMarker()
-      sendKeyboardEvent "tab"
+      sendKeyboardEvent "Tab", "keydown"
       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"
     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"
     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"
     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"
     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"
     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"
     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"
+    assert.equal null, commandCount
     assert.isFalse @postFindMode.modeIsActive
 
   should "not enter insert mode on subsequent escapes", ->
     sendKeyboardEvent "a"
-    sendKeyboardEvent "escape"
+    sendKeyboardEvent "Escape", "keydown"
     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"
     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"
     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 = "<input type='text' value='some value' id='input'/>"
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'