Key bindings; fix tests...

... and fix two bugs:

- not suppressing keyup event after keyChar matched in keydown.
- we cannot check the passKeys keyChar in keyup because the key state
  has changed; so we track what the next keyup response should be.

6 files changed, 53 insertions, 54 deletions

diff --git a/content_scripts/mode_key_handler.coffee b/content_scripts/mode_key_handler.coffee
index e38a7051..f7be977b 100644
--- a/content_scripts/mode_key_handler.coffee
+++ b/content_scripts/mode_key_handler.coffee
@@ -6,7 +6,7 @@ class KeyHandlerMode extends Mode
 
   constructor: (options) ->
     @commandHandler = options.commandHandler ? (->)
-    @setKeyMapping options.commandHandler ? {}
+    @setKeyMapping options.keyMapping ? {}
 
     delete options[option] for option in ["commandHandler", "keyMapping"]
     super extend options,
@@ -28,7 +28,10 @@ class KeyHandlerMode extends Mode
         DomUtils.suppressKeyupAfterEscape handlerStack
         false # Suppress event.
     else if keyChar and @mappingForKeyChar keyChar
+      @keydownEvents[event.keyCode] = true
       @handleKeyChar event, keyChar
+    else if keyChar
+      @continueBubbling
     else
       # 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.
diff --git a/content_scripts/mode_passkeys.coffee b/content_scripts/mode_passkeys.coffee
index a68fe3be..7efa559c 100644
--- a/content_scripts/mode_passkeys.coffee
+++ b/content_scripts/mode_passkeys.coffee
@@ -1,12 +1,15 @@
 
 class PassKeysMode extends Mode
   constructor: (@normalMode) ->
+    @nextKeyup = @continueBubbling
     super
       name: "passkeys"
       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
+      keyup: => ([_, @nextKeyup] = [@nextKeyup, @continueBubbling])[0]
+      # We cannot track keyup events if we lose the focus.
+      blur: (event) => @alwaysContinueBubbling => @nextKeyup = @continueBubbling if event.target == window
 
   # 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.
@@ -14,7 +17,7 @@ class PassKeysMode extends Mode
     return @continueBubbling if event.altKey or event.ctrlKey or event.metaKey
     return @continueBubbling unless keyChar and @normalMode.isFirstKeyChar keyChar
     return @continueBubbling unless keyChar.length == 1 and 0 <= @passKeys.indexOf keyChar
-    @stopBubblingAndTrue
+    @nextKeyup = @stopBubblingAndTrue
 
 root = exports ? window
 root.PassKeysMode = PassKeysMode
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index 2fc2dc35..e821b136 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -93,42 +93,41 @@ handlerStack.push
         target = target.parentElement
     true
 
-# Only exported for tests.
-window.initializeModes = ->
-  class NormalMode extends KeyHandlerMode
-    constructor: ->
-      super
-        name: "normal"
-        indicator: false # There is no mode indicator in normal mode.
-        commandHandler: @commandHandler.bind this
-        keyMapping: {}
-
-      chrome.storage.local.get "normalModeKeyStateMapping", (items) =>
-        @setKeyMapping items.normalModeKeyStateMapping
-
-      chrome.storage.onChanged.addListener (changes, area) =>
-        if area == "local" and changes.normalModeKeyStateMapping?.newValue
-          @setKeyMapping changes.normalModeKeyStateMapping.newValue
-
-    commandHandler: (registryEntry, count) ->
-      count *= registryEntry.options.count ? 1
-      count = 1 if registryEntry.noRepeat
-
-      if registryEntry.repeatLimit? and registryEntry.repeatLimit < count
-        return unless confirm """
-          You have asked Vimium to perform #{count} repeats of the command: #{registryEntry.description}.\n
-          Are you sure you want to continue?"""
-
-      if registryEntry.isBackgroundCommand
-        chrome.runtime.sendMessage {handler: "runBackgroundCommand", frameId, registryEntry, count}
-      else if registryEntry.passCountToFunction
-        Utils.invokeCommandString registryEntry.command, [count]
-      else
-        Utils.invokeCommandString registryEntry.command for i in [0...count]
-
+class NormalMode extends KeyHandlerMode
+  constructor: (options = {}) ->
+    super extend options,
+      name: "normal"
+      indicator: false # There is no mode indicator in normal mode.
+      commandHandler: @commandHandler.bind this
+
+    chrome.storage.local.get "normalModeKeyStateMapping", (items) =>
+      @setKeyMapping items.normalModeKeyStateMapping
+
+    chrome.storage.onChanged.addListener (changes, area) =>
+      if area == "local" and changes.normalModeKeyStateMapping?.newValue
+        @setKeyMapping changes.normalModeKeyStateMapping.newValue
+
+  commandHandler: (registryEntry, count) ->
+    count *= registryEntry.options.count ? 1
+    count = 1 if registryEntry.noRepeat
+
+    if registryEntry.repeatLimit? and registryEntry.repeatLimit < count
+      return unless confirm """
+        You have asked Vimium to perform #{count} repeats of the command: #{registryEntry.description}.\n
+        Are you sure you want to continue?"""
+
+    if registryEntry.isBackgroundCommand
+      chrome.runtime.sendMessage {handler: "runBackgroundCommand", frameId, registryEntry, count}
+    else if registryEntry.passCountToFunction
+      Utils.invokeCommandString registryEntry.command, [count]
+    else
+      Utils.invokeCommandString registryEntry.command for i in [0...count]
+
+# Only exported for tests; also, "args..." is only for the tests.
+window.initializeModes = (args...) ->
   # Install the permanent modes.  The permanently-installed insert mode tracks focus/blur events, and
   # activates/deactivates itself accordingly.
-  normalMode = new NormalMode
+  normalMode = new NormalMode args...
   new PassKeysMode normalMode
   new InsertMode permanent: true
   Scroller.init()
diff --git a/tests/dom_tests/dom_tests.coffee b/tests/dom_tests/dom_tests.coffee
index 8a96913f..6309525f 100644
--- a/tests/dom_tests/dom_tests.coffee
+++ b/tests/dom_tests/dom_tests.coffee
@@ -24,15 +24,10 @@ for type in [ "keydown", "keypress", "keyup" ]
 initializeModeState = ->
   Mode.reset()
   handlerStack.reset()
-  initializeModes()
-  # We use "m" as the only mapped key, "p" as a passkey, and "u" as an unmapped key.
-  refreshCompletionKeys
-    completionKeys: "mp"
+  initializeModes keyMapping: {m: {}, p: {}, z: {p: {}}}
   handlerStack.bubbleEvent "registerStateChange",
     enabled: true
     passKeys: "p"
-  handlerStack.bubbleEvent "registerKeyQueue",
-    keyQueue: ""
 
 # Tell Settings that it's been loaded.
 Settings.isLoaded = true
@@ -375,10 +370,14 @@ context "Normal mode",
     sendKeyboardEvent "p"
     assert.equal pageKeyboardEventCount, 3
 
-  should "suppress passKeys with a non-empty keyQueue", ->
-    handlerStack.bubbleEvent "registerKeyQueue", keyQueue: "p"
+  should "suppress passKeys with a non-empty key state (a count)", ->
+    sendKeyboardEvent "5"
+    assert.equal 0, pageKeyboardEventCount
+
+  should "suppress passKeys with a non-empty key state (a key)", ->
+    sendKeyboardEvent "z"
     sendKeyboardEvent "p"
-    assert.equal pageKeyboardEventCount, 0
+    assert.equal 0, pageKeyboardEventCount
 
 context "Insert mode",
   setup ->
@@ -397,7 +396,7 @@ context "Insert mode",
   should "resume normal mode after leaving insert mode", ->
     @insertMode.exit()
     sendKeyboardEvent "m"
-    assert.equal pageKeyboardEventCount, 0
+    assert.equal 0, pageKeyboardEventCount
 
 context "Triggering insert mode",
   setup ->
@@ -502,12 +501,6 @@ context "Mode utilities",
     assert.isTrue test.enabled == "one"
     assert.isTrue test.passKeys == "two"
 
-  should "register the keyQueue", ->
-    test = new Mode trackState: true
-    handlerStack.bubbleEvent "registerKeyQueue", keyQueue: "hello"
-
-    assert.isTrue test.keyQueue == "hello"
-
 context "PostFindMode",
   setup ->
     initializeModeState()
diff --git a/tests/dom_tests/dom_tests.html b/tests/dom_tests/dom_tests.html
index 8d355c6d..8c914d3c 100644
--- a/tests/dom_tests/dom_tests.html
+++ b/tests/dom_tests/dom_tests.html
@@ -46,6 +46,7 @@
     <script type="text/javascript" src="../../content_scripts/mode_insert.js"></script>
     <script type="text/javascript" src="../../content_scripts/mode_find.js"></script>
     <script type="text/javascript" src="../../content_scripts/mode_visual_edit.js"></script>
+    <script type="text/javascript" src="../../content_scripts/mode_key_handler.js"></script>
     <script type="text/javascript" src="../../content_scripts/hud.js"></script>
     <script type="text/javascript" src="../../content_scripts/vimium_frontend.js"></script>
 
diff --git a/tests/unit_tests/commands_test.coffee b/tests/unit_tests/commands_test.coffee
index 45cb4cf4..f501a960 100644
--- a/tests/unit_tests/commands_test.coffee
+++ b/tests/unit_tests/commands_test.coffee
@@ -1,6 +1,6 @@
 require "./test_helper.js"
 extend global, require "./test_chrome_stubs.js"
-global.Settings = {postUpdateHooks: {}}
+global.Settings = {postUpdateHooks: {}, get: (-> ""), set: ->}
 {Commands} = require "../../background_scripts/commands.js"
 
 context "Key mappings",