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.

1 files changed, 28 insertions, 44 deletions

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