Move keyboard utils to keydown and migrate normal/visual modes.

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
-        # <c-[> is mapped to Escape in Vim by default.
-        keyChar == "<c-[>"
+  isEscape: (event) ->
+    # <c-[> is mapped to Escape in Vim by default.
+    event.keyCode == @keyCodes.ESC || @getKeyCharString(event) == "<c-[>"
 
-  # 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" ]