Modes; re-architect key suppression and passkeys.

4 files changed, 30 insertions, 37 deletions

diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee
index 19354d94..0fcab675 100644
--- a/content_scripts/mode.coffee
+++ b/content_scripts/mode.coffee
@@ -111,16 +111,18 @@ class Mode
               @passKeys = passKeys
               @registerStateChange?()
 
-    # If @options.suppressPrintableEvents is truthy, then it should be an element.  All printable keyboard
-    # events on that element are suppressed, if necessary (that is, *after* bubbling down the handler stack).
-    # We only suppress keypress events.  This is used by PostFindMode to protect active, editable elements.
-    # Note: We use unshift here, not push, so the handler is installed at the bottom of the stack.
+    # If @options.suppressPrintableEvents is truthy, then it should be an element.  All printable keypress
+    # events on that element are suppressed, if necessary.  They are suppressed *after* bubbling down the
+    # handler stack and finding no handler.  This is used by PostFindMode to protect active, editable
+    # elements.
     if @options.suppressPrintableEvents
-      @unshift
+      @push
         _name: "mode-#{@id}/suppressPrintableEvents"
         keypress: (event) =>
-          if KeyboardUtils.isPrintable(event) and
-            event.srcElement == @options.suppressPrintableEvents then @suppressEvent else @continueBubbling
+          @alwaysContinueBubbling =>
+            if event.srcElement == @options.suppressPrintableEvents
+              if KeyboardUtils.isPrintable(event)
+                event.vimium_suppress_event = true
 
     Mode.updateBadge() if @badge
     Mode.modes.push @
@@ -217,6 +219,9 @@ new class BadgeMode extends Mode
       name: "badge"
       trackState: true
 
+    # FIXME(smblott) BadgeMode is currently triggering and updateBadge event on every focus event.  That's a
+    # lot, considerably more than is necessary.  Really, it only needs to trigger when we change frame, or
+    # when we change tab.
     @push
       _name: "mode-#{@id}/focus"
       "focus": => @alwaysContinueBubbling -> Mode.updateBadge()
@@ -228,5 +233,19 @@ new class BadgeMode extends Mode
   registerStateChange: ->
     Mode.updateBadge()
 
+# KeySuppressor is a pseudo mode (near the bottom of the stack) which suppresses keyboard events tagged with
+# the "vimium_suppress_event" property.  This allows modes higher up in the stack to tag events for
+# suppression, but only after verifying that no other mode (notably, normal mode) wants to handle the event.
+# Note.  We also create the the one-and-only instance, here.
+new class KeySuppressor extends Mode
+  constructor: ->
+    super
+      name: "key-suppressor"
+      keydown: (event) => @handle event
+      keypress: (event) => @handle event
+      keyup: (event) => @handle event
+
+  handle: (event) -> if event.vimium_suppress_event then @suppressEvent else @continueBubbling
+
 root = exports ? window
 root.Mode = Mode
diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee
index 31bae8ec..5720c901 100644
--- a/content_scripts/mode_insert.coffee
+++ b/content_scripts/mode_insert.coffee
@@ -96,34 +96,6 @@ class InsertModeBlocker extends Mode
             new @options.onClickMode
               targetElement: document.activeElement
 
-# There's an unfortunate feature interaction between chrome's contentEditable handling and our insert mode.
-# If the selection is contentEditable and a descendant of the active element, then chrome focuses it on any
-# unsuppressed printable keypress.  This drops us unintentally into insert mode.  See #1415.  A single
-# instance of this mode sits near the bottom of the handler stack and suppresses each keypress event if:
-#   - it hasn't been handled by any other mode (so not by normal mode, passkeys, insert, ...),
-#   - it represents a printable character,
-#   - the selection is content editable, and
-#   - the selection is a descendant of the active element.
-# This should rarely fire, typically only on fudged keypresses in normal mode.  And, even then, only in the
-# circumstances outlined above.  So, we shouldn't usually be blocking keyboard events for other extensions or
-# the page itself.
-# There's some controversy as to whether this is the right thing to do.  See discussion in #1415. This
-# implements Option 2 from there.
-new class ContentEditableTrap extends Mode
-  constructor: ->
-    super
-      name: "content-editable-trap"
-      keypress: (event) =>
-        if @wouldTriggerInsert event then @suppressEvent else @continueBubbling
-
-  # True if the selection is content editable and a descendant of the active element.
-  wouldTriggerInsert: (event) ->
-    element = document.getSelection()?.anchorNode?.parentElement
-    return element?.isContentEditable and
-             document.activeElement and
-             KeyboardUtils.isPrintable(event) and
-             DomUtils.isDOMDescendant document.activeElement, element
-
 root = exports ? window
 root.InsertMode = InsertMode
 root.InsertModeTrigger = InsertModeTrigger
diff --git a/content_scripts/mode_passkeys.coffee b/content_scripts/mode_passkeys.coffee
index 112e14ed..c4df06dc 100644
--- a/content_scripts/mode_passkeys.coffee
+++ b/content_scripts/mode_passkeys.coffee
@@ -12,8 +12,8 @@ class PassKeysMode extends Mode
   # passKeys if the keyQueue is not empty.  So, for example, if 't' is a passKey, then 'gt' and '99t' will
   # neverthless be handled by vimium.
   handleKeyChar: (keyChar) ->
-    return @stopBubblingAndTrue if keyChar and not @keyQueue and 0 <= @passKeys.indexOf keyChar
-    @continueBubbling
+    @alwaysContinueBubbling =>
+      event.vimium_suppress_normal_mode = true if keyChar and not @keyQueue and 0 <= @passKeys.indexOf keyChar
 
   configure: (request) ->
     @keyQueue = request.keyQueue if request.keyQueue?
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index 1406b1e7..0da59f03 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -462,6 +462,7 @@ KeydownEvents =
 #
 
 onKeypress = (event) ->
+  return true if event.vimium_suppress_normal_mode
   keyChar = ""
 
   # Ignore modifier keys by themselves.
@@ -491,6 +492,7 @@ onKeypress = (event) ->
   return true
 
 onKeydown = (event) ->
+  return true if event.vimium_suppress_normal_mode
   keyChar = ""
 
   # handle special keys, and normal input keys with modifiers being pressed. don't handle shiftKey alone (to