Key bindings; more tweaks and fixes.

Miscellaneous fixes and tweaks, including:

- Reinstate key logging.
- Fix count handling in line with expected behaviour in #2024.
- Remove `noCount` option; we don't need it.
- Simplify logic in various places.

Fixes #2024.

4 files changed, 36 insertions, 63 deletions

diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee
index 454a6ed9..efbc9cf4 100644
--- a/content_scripts/mode.coffee
+++ b/content_scripts/mode.coffee
@@ -120,20 +120,13 @@ class Mode
         singletons[key] = this
 
     # If @options.trackState is truthy, then the mode mainatins the current state in @enabled and @passKeys,
-    # and calls @registerStateChange() (if defined) whenever the state changes. The mode also tracks the
-    # current keyQueue in @keyQueue.
+    # and calls @registerStateChange() (if defined) whenever the state changes.
     if @options.trackState
       @enabled = false
       @passKeys = ""
-      @keyQueue = ""
       @push
         _name: "mode-#{@id}/registerStateChange"
-        registerStateChange: ({ enabled: enabled, passKeys: passKeys }) => @alwaysContinueBubbling =>
-          if enabled != @enabled or passKeys != @passKeys
-            @enabled = enabled
-            @passKeys = passKeys
-            @registerStateChange?()
-        registerKeyQueue: ({ keyQueue: keyQueue }) => @alwaysContinueBubbling => @keyQueue = keyQueue
+        registerStateChange: ({ enabled: @enabled, passKeys: @passKeys }) => @alwaysContinueBubbling =>
 
     # If @options.passInitialKeyupEvents is set, then we pass initial non-printable keyup events to the page
     # or to other extensions (because the corresponding keydown events were passed).  This is used when
diff --git a/content_scripts/mode_key_handler.coffee b/content_scripts/mode_key_handler.coffee
index 9b044923..c9cab244 100644
--- a/content_scripts/mode_key_handler.coffee
+++ b/content_scripts/mode_key_handler.coffee
@@ -1,32 +1,25 @@
 
 class KeyHandlerMode extends Mode
-  useCount: true
   countPrefix: 0
   keydownEvents: {}
   keyState: []
 
   constructor: (options) ->
-    # A function accepting a command name and a count; required.
     @commandHandler = options.commandHandler ? (->)
-    @useCount = false if options.noCount
-    @reset()
-
-    # We don't pass these options on to super().
-    options = Utils.copyObjectOmittingProperties options, "commandHandler", "keyMapping", "noCount"
+    @setKeyMapping options.commandHandler ? {}
 
+    delete options[option] for option in ["commandHandler", "keyMapping"]
     super extend options,
       keydown: @onKeydown.bind this
       keypress: @onKeypress.bind this
       keyup: @onKeyup.bind this
       # We cannot track matching keydown/keyup events if we lose the focus.
-      blur: (event) => @alwaysContinueBubbling =>
-        @keydownEvents = {} if event.target == window
+      blur: (event) => @alwaysContinueBubbling => @keydownEvents = {} if event.target == window
 
   setKeyMapping: (@keyMapping) -> @reset()
 
   onKeydown: (event) ->
     keyChar = KeyboardUtils.getKeyCharString event
-
     if KeyboardUtils.isEscape event
       if @isInResetState()
         @continueBubbling
@@ -39,16 +32,14 @@ class KeyHandlerMode extends Mode
       @handleKeyChar event, keyChar
 
     else
-      # We did not handle the event, but we might handle the subsequent keypress event.  If we *will* be
-      # handling that event, then we need to suppress propagation of this keydown event to prevent triggering
-      # page features like Google instant search.
+      # We did not handle the event, but we might handle a subsequent keypress.  If we will be handling that
+      # event, then we suppress propagation of this keydown to prevent triggering page events.
       keyChar = KeyboardUtils.getKeyChar event
       if keyChar and (@mappingForKeyChar(keyChar) or @isCountKey keyChar)
         DomUtils.suppressPropagation event
-        @keydownEvents[@getEventCode event] = true
+        @keydownEvents[event.keyCode] = true
         @stopBubblingAndTrue
       else
-        @countPrefix = 0 if keyChar
         @continueBubbling
 
   onKeypress: (event) ->
@@ -56,50 +47,46 @@ class KeyHandlerMode extends Mode
     if keyChar and @mappingForKeyChar keyChar
       @handleKeyChar event, keyChar
     else if keyChar and @isCountKey keyChar
-      @reset @countPrefix * 10 + parseInt keyChar
+      digit = parseInt keyChar
+      @reset if @keyState.length == 1 then @countPrefix * 10 + digit else digit
       false # Suppress event.
     else
+      @reset()
       @continueBubbling
 
   onKeyup: (event) ->
-    eventCode = @getEventCode event
-    if eventCode of @keydownEvents
-      delete @keydownEvents[eventCode]
+    if event.keyCode of @keydownEvents
+      delete @keydownEvents[event.keyCode]
       DomUtils.suppressPropagation event
       @stopBubblingAndTrue
     else
       @continueBubbling
 
   handleKeyChar: (event, keyChar) ->
+    bgLog "Handling key #{keyChar}, mode=#{@name}."
     @advanceKeyState keyChar
     commands = @keyState.filter (entry) -> entry.command
-    @invokeCommand commands[0] if 0 < commands.length
+    if 0 < commands.length
+      countPrefix = if 0 < @countPrefix then @countPrefix else 1
+      @reset()
+      bgLog "Calling mode=#{@name}, command=#{commands[0].command}, count=#{countPrefix}."
+      @commandHandler commands[0], countPrefix
     false # Suppress event.
 
-  # This returns the first mapping for which keyChar is mapped. The return value is truthy if a match is found
-  # and falsy otherwise.
+  # This returns the first key-state entry for which keyChar is mapped. The return value is truthy if a match
+  # is found and falsy otherwise.
   mappingForKeyChar: (keyChar) ->
-    for mapping in @keyState
-      return mapping if keyChar of mapping
-    null
+    (mapping for mapping in @keyState when keyChar of mapping)[0]
 
-  # This is called whenever a keyChar is matched.  We keep any existing entries matching keyChar, and append a
-  # new copy of the global key mappings.
+  # This is called whenever a keyChar is matched.  We keep any existing mappings matching keyChar, and append
+  # a new copy of the mode's global key mappings.
   advanceKeyState: (keyChar) ->
-    newKeyState =
-      for mapping in @keyState
-        continue unless keyChar of mapping
-        mapping[keyChar]
-    @keyState = [newKeyState..., @keyMapping]
-
-  # This is called to invoke a command and reset the key state.
-  invokeCommand: (command) ->
-    countPrefix = if 0 < @countPrefix then @countPrefix else 1
-    @reset()
-    @commandHandler command, countPrefix
+    newMappings = (mapping[keyChar] for mapping in @keyState when keyChar of mapping)
+    @keyState = [newMappings..., @keyMapping]
 
   # Reset the state (as if no keys had been handled), but retaining the count - if one is provided.
   reset: (count = 0) ->
+    bgLog "Clearing key queue, set count=#{count}."
     @countPrefix = count
     @keyState = [@keyMapping]
 
@@ -110,18 +97,15 @@ class KeyHandlerMode extends Mode
 
   # This tests whether keyChar should be treated as a count key.
   isCountKey: (keyChar) ->
-    return false unless @useCount and keyChar.length == 1
+    return false unless keyChar.length == 1
     if 0 < @countPrefix
       '0' <= keyChar <= '9'
     else
       '1' <= keyChar <= '9'
 
-  # True if keyChar would be the first character of a command mapping.  This is used by passKeys to decide
-  # whether keyChar is a continuation of a command which the user has already begin entering.
+  # Test whether keyChar would be the very first character of a command mapping.
   isFirstKeyChar: (keyChar) ->
-    @countPrefix == 0 and @keyMapping == @mappingForKeyChar keyChar
-
-  getEventCode: (event) -> event.keyCode
+    keyChar and @countPrefix == 0 and (@mappingForKeyChar(keyChar) == @keyMapping or @isCountKey keyChar)
 
 root = exports ? window
 root.KeyHandlerMode = KeyHandlerMode
diff --git a/content_scripts/mode_passkeys.coffee b/content_scripts/mode_passkeys.coffee
index 426e5369..a68fe3be 100644
--- a/content_scripts/mode_passkeys.coffee
+++ b/content_scripts/mode_passkeys.coffee
@@ -3,20 +3,17 @@ class PassKeysMode extends Mode
   constructor: (@normalMode) ->
     super
       name: "passkeys"
-      trackState: true # Maintain @enabled, @passKeys and @keyQueue.
+      trackState: true # Maintain @passKeys.
       keydown: (event) => @handleKeyChar event, KeyboardUtils.getKeyChar event
       keypress: (event) => @handleKeyChar event, String.fromCharCode event.charCode
       keyup: (event) => @handleKeyChar event, KeyboardUtils.getKeyChar event
 
-  # Keystrokes are *never* considered passKeys if the user has begin entering a command.  So, for example, if
-  # 't' is a passKey, then 'gt' and '99t' will neverthless be handled by Vimium.
+  # Keystrokes are *never* considered passKeys if the user has begun entering a command.  So, for example, if
+  # 't' is a passKey, then 'gt' and '99t' are neverthless be handled by Vimium.
   handleKeyChar: (event, keyChar) ->
     return @continueBubbling if event.altKey or event.ctrlKey or event.metaKey
-    return @continueBubbling unless keyChar and keyChar.length == 1
-    # Test whether the user has already begun entering a command.
-    return @continueBubbling unless @normalMode.isFirstKeyChar keyChar
-    return @continueBubbling unless 0 <= @passKeys.indexOf keyChar
-    # This is a passkey.
+    return @continueBubbling unless keyChar and @normalMode.isFirstKeyChar keyChar
+    return @continueBubbling unless keyChar.length == 1 and 0 <= @passKeys.indexOf keyChar
     @stopBubblingAndTrue
 
 root = exports ? window
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index 5c594d1c..3220b084 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -7,7 +7,6 @@
 
 isEnabledForUrl = true
 isIncognitoMode = chrome.extension.inIncognitoContext
-passKeys = null
 
 # We track whther the current window has the focus or not.
 windowIsFocused = do ->
@@ -162,7 +161,7 @@ initializePreDomReady = ->
     return if request.handler and not request.name
     shouldHandleRequest = isEnabledForUrl
     # We always handle the message if it's one of these listed message types.
-    shouldHandleRequest ||= request.name in [ "checkEnabledAfterURLChange" ]
+    shouldHandleRequest ||= request.name in ["checkEnabledAfterURLChange", "openVomnibar"]
     sendResponse requestHandlers[request.name](request, sender) if shouldHandleRequest
     # Ensure the sendResponse callback is freed.
     false