From dc423eff18b2b2654c175633cd11e28ea9279fd7 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Fri, 9 Jan 2015 16:53:35 +0000
Subject: Modes; rework blocker for contentEditable.

---
 content_scripts/mode.coffee        | 4 +++-
 content_scripts/mode_insert.coffee | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee
index 8e37ee36..b6cb5fae 100644
--- a/content_scripts/mode.coffee
+++ b/content_scripts/mode.coffee
@@ -175,7 +175,9 @@ class Mode
       # suppress badge updates while exiting any existing active singleton.  This prevents the badge from
       # flickering in some cases.
       Mode.badgeSuppressor.runSuppresed =>
-        singletons[key].exit() if singletons[key]
+        if singletons[key]
+          console.log singletons[key].count, "singleton:", @name, "(deactivating)"
+          singletons[key].exit()
       singletons[key] = @
 
       @onExit => delete singletons[key] if singletons[key] == @
diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee
index c0a61d31..5375bcdc 100644
--- a/content_scripts/mode_insert.coffee
+++ b/content_scripts/mode_insert.coffee
@@ -44,6 +44,7 @@ class InsertModeTrigger extends Mode
           # and unfortunately, the focus event happens *before* the change is made.  Therefore, we need to
           # check again whether the active element is contentEditable.
           return @continueBubbling unless document.activeElement?.isContentEditable
+          console.log @count, @name, "fired (by keydown)"
           new InsertMode
             targetElement: document.activeElement
           @stopBubblingAndTrue
@@ -52,6 +53,7 @@ class InsertModeTrigger extends Mode
       focus: (event) =>
         triggerSuppressor.unlessSuppressed =>
           return unless DomUtils.isFocusable event.target
+          console.log @count, @name, "fired (by focus)"
           new InsertMode
             targetElement: event.target
 
-- 
cgit v1.2.3


From 8e65f74eea14850794ded12a0039a80e825ffa8d Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Fri, 9 Jan 2015 17:38:57 +0000
Subject: Modes; handle normal mode return values.

Up until this point in the development of modes, we've just let the
normal mode handlers return whatever they previously would have
returned.  This allowed keyboard events to continue bubbling down the
stack, but didn't matter, because normal mode is the last keyboard
handler on the stack.

This changes that.  Now, normal-mode key handlers return the right value
to have the handler stack stop or continue bubbling, as appropriate.
---
 content_scripts/vimium_frontend.coffee | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index d91bb181..97fbc56f 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -465,17 +465,20 @@ onKeypress = (event, extra) ->
     # Enter insert mode when the user enables the native find interface.
     if (keyChar == "f" && KeyboardUtils.isPrimaryModifierKey(event))
       enterInsertModeWithoutShowingIndicator()
-      return true
+      return handlerStack.stopBubblingAndTrue
 
     if (keyChar)
       if (findMode)
         handleKeyCharForFindMode(keyChar)
         DomUtils.suppressEvent(event)
+        return handlerStack.stopBubblingAndTrue
       else if (!isInsertMode() && !findMode)
         if (isPassKey keyChar)
           return handlerStack.stopBubblingAndTrue
         if currentCompletionKeys.indexOf(keyChar) != -1 or isValidFirstKey(keyChar)
           DomUtils.suppressEvent(event)
+          keyPort.postMessage({ keyChar:keyChar, frameId:frameId })
+          return handlerStack.stopBubblingAndTrue
 
         keyPort.postMessage({ keyChar:keyChar, frameId:frameId })
 
@@ -520,37 +523,45 @@ onKeydown = (event, extra) ->
     exitInsertMode()
     DomUtils.suppressEvent event
     KeydownEvents.push event
+    return handlerStack.stopBubblingAndTrue
 
   else if (findMode)
     if (KeyboardUtils.isEscape(event))
       handleEscapeForFindMode()
       DomUtils.suppressEvent event
       KeydownEvents.push event
+      return handlerStack.stopBubblingAndTrue
 
     else if (event.keyCode == keyCodes.backspace || event.keyCode == keyCodes.deleteKey)
       handleDeleteForFindMode()
       DomUtils.suppressEvent event
       KeydownEvents.push event
+      return handlerStack.stopBubblingAndTrue
 
     else if (event.keyCode == keyCodes.enter)
       handleEnterForFindMode()
       DomUtils.suppressEvent event
       KeydownEvents.push event
+      return handlerStack.stopBubblingAndTrue
 
     else if (!modifiers)
       DomUtils.suppressPropagation(event)
       KeydownEvents.push event
+      return handlerStack.stopBubblingAndTrue
 
   else if (isShowingHelpDialog && KeyboardUtils.isEscape(event))
     hideHelpDialog()
     DomUtils.suppressEvent event
     KeydownEvents.push event
+    return handlerStack.stopBubblingAndTrue
 
   else if (!isInsertMode() && !findMode)
     if (keyChar)
       if (currentCompletionKeys.indexOf(keyChar) != -1 or isValidFirstKey(keyChar))
         DomUtils.suppressEvent event
         KeydownEvents.push event
+        keyPort.postMessage({ keyChar:keyChar, frameId:frameId })
+        return handlerStack.stopBubblingAndTrue
 
       keyPort.postMessage({ keyChar:keyChar, frameId:frameId })
 
@@ -572,12 +583,14 @@ onKeydown = (event, extra) ->
       isValidFirstKey(KeyboardUtils.getKeyChar(event))))
     DomUtils.suppressPropagation(event)
     KeydownEvents.push event
+    return handlerStack.stopBubblingAndTrue
 
   return true
 
 onKeyup = (event) ->
-  DomUtils.suppressPropagation(event) if KeydownEvents.pop event
-  return true
+  return true unless KeydownEvents.pop event
+  DomUtils.suppressPropagation(event)
+  handlerStack.stopBubblingAndTrue
 
 checkIfEnabledForUrl = ->
   url = window.location.toString()
-- 
cgit v1.2.3


From e2380ff9417834900649173750edf17a26a8b703 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Fri, 9 Jan 2015 18:12:58 +0000
Subject: Modes; block unmapped keys with contentEditable.

---
 content_scripts/mode_insert.coffee | 34 ++++++++++++++++++++++++++++++++--
 1 file changed, 32 insertions(+), 2 deletions(-)

diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee
index 5375bcdc..cbeea48f 100644
--- a/content_scripts/mode_insert.coffee
+++ b/content_scripts/mode_insert.coffee
@@ -15,8 +15,10 @@ class InsertMode extends Mode
     options = extend defaults, options
     options.exitOnBlur = options.targetElement || null
     super options
+    triggerSuppressor.suppress()
 
   exit: (event = null) ->
+    triggerSuppressor.unsuppress()
     super()
     if @options.blurOnExit
       element = event?.srcElement
@@ -44,7 +46,6 @@ class InsertModeTrigger extends Mode
           # and unfortunately, the focus event happens *before* the change is made.  Therefore, we need to
           # check again whether the active element is contentEditable.
           return @continueBubbling unless document.activeElement?.isContentEditable
-          console.log @count, @name, "fired (by keydown)"
           new InsertMode
             targetElement: document.activeElement
           @stopBubblingAndTrue
@@ -53,7 +54,6 @@ class InsertModeTrigger extends Mode
       focus: (event) =>
         triggerSuppressor.unlessSuppressed =>
           return unless DomUtils.isFocusable event.target
-          console.log @count, @name, "fired (by focus)"
           new InsertMode
             targetElement: event.target
 
@@ -89,6 +89,36 @@ class InsertModeBlocker extends Mode
             new @options.onClickMode
               targetElement: document.activeElement
 
+# There's some unfortunate feature interaction with chrome's content editable handling.  If the selection is
+# content editable and a descendant of the active element, then chrome focuses it on any unsuppressed keyboard
+# events.  This has the unfortunate effect of dropping us unintentally into insert mode.  See #1415.
+# This mode sits near the bottom of the handler stack and suppresses keyboard events if:
+#   - they haven't been handled by any other mode (so not by normal mode, passkeys mode, insert mode, and so
+#     on), and
+#   - 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 it shouldn't normally block other extensions or the page itself from
+# handling keyboard events.
+new class ContentEditableTrap extends Mode
+  constructor: ->
+    super
+      name: "content-editable-trap"
+      keydown: (event) => @handle => DomUtils.suppressPropagation event
+      keypress: (event) => @handle => @suppressEvent
+      keyup: (event) => @handle => @suppressEvent
+
+  # True if the selection is content editable and a descendant of the active element.  In this situation,
+  # chrome unilaterally focuses the element containing the anchor, dropping us into insert mode.
+  isContentEditableFocused: ->
+    element = document.getSelection()?.anchorNode?.parentElement
+    return element?.isContentEditable? and
+             document.activeElement? and
+             DomUtils.isDOMDescendant document.activeElement, element
+
+  handle: (func) ->
+    if @isContentEditableFocused() then func() else @continueBubbling
+
 root = exports ? window
 root.InsertMode = InsertMode
 root.InsertModeTrigger = InsertModeTrigger
-- 
cgit v1.2.3


From d97e7786cb04dbbe5cae8e4b86e25437f66eb799 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Fri, 9 Jan 2015 22:08:37 +0000
Subject: Modes; fix focus return value for InsertModeTrigger.

---
 content_scripts/mode_insert.coffee | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee
index cbeea48f..9a2d5ce1 100644
--- a/content_scripts/mode_insert.coffee
+++ b/content_scripts/mode_insert.coffee
@@ -53,9 +53,10 @@ class InsertModeTrigger extends Mode
     @push
       focus: (event) =>
         triggerSuppressor.unlessSuppressed =>
-          return unless DomUtils.isFocusable event.target
-          new InsertMode
-            targetElement: event.target
+          @alwaysContinueBubbling =>
+            if DomUtils.isFocusable event.target
+              new InsertMode
+                targetElement: event.target
 
     # We may already have focussed an input, so check.
     if document.activeElement and DomUtils.isEditable document.activeElement
@@ -63,7 +64,7 @@ class InsertModeTrigger extends Mode
         targetElement: document.activeElement
 
 # Used by InsertModeBlocker to suppress InsertModeTrigger; see below.
-triggerSuppressor = new Utils.Suppressor true
+triggerSuppressor = new Utils.Suppressor true # Note: true == @continueBubbling
 
 # Suppresses InsertModeTrigger.  This is used by various modes (usually by inheritance) to prevent
 # unintentionally dropping into insert mode on focusable elements.
-- 
cgit v1.2.3


From ac90db47aa2671cd663cc6a9cdf783dc30a582e9 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 10 Jan 2015 00:00:11 +0000
Subject: Modes; more changes...

- Better comments.
- Strip unnecessary handlers for leaving post-find mode.
- Simplify passKeys.
- focusInput now re-bubbles its triggering keydown event.
---
 content_scripts/mode.coffee            |  3 +-
 content_scripts/mode_find.coffee       | 17 ++++-------
 content_scripts/mode_insert.coffee     | 53 +++++++++++++++++-----------------
 content_scripts/mode_passkeys.coffee   | 13 ++++-----
 content_scripts/vimium_frontend.coffee | 25 ++++++++--------
 lib/handler_stack.coffee               | 37 +++++++++++++-----------
 6 files changed, 74 insertions(+), 74 deletions(-)

diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee
index b6cb5fae..37f3a8c2 100644
--- a/content_scripts/mode.coffee
+++ b/content_scripts/mode.coffee
@@ -45,11 +45,12 @@ class Mode
   # If this is true, then we generate a trace of modes being activated and deactivated on the console.
   @debug = true
 
-  # Constants; readable shortcuts for event-handler return values.
+  # Constants; short, readable names for handlerStack event-handler return values.
   continueBubbling: true
   suppressEvent: false
   stopBubblingAndTrue: handlerStack.stopBubblingAndTrue
   stopBubblingAndFalse: handlerStack.stopBubblingAndFalse
+  restartBubbling: handlerStack.restartBubbling
 
   constructor: (@options={}) ->
     @handlers = []
diff --git a/content_scripts/mode_find.coffee b/content_scripts/mode_find.coffee
index 3b9f951e..d63b3319 100644
--- a/content_scripts/mode_find.coffee
+++ b/content_scripts/mode_find.coffee
@@ -2,11 +2,11 @@
 
 # When we use find mode, the selection/focus can end up in a focusable/editable element.  In this situation,
 # special considerations apply.  We implement three special cases:
-#   1. Be an InsertModeBlocker.  This prevents keyboard events from dropping us unintentionally into insert
-#      mode. This is achieved by inheriting from InsertModeBlocker.
+#   1. Prevent keyboard events from dropping us unintentionally into insert mode. This is achieved by
+#      inheriting from InsertModeBlocker.
 #   2. Prevent all keyboard events on the active element from propagating.  This is achieved by setting the
 #      trapAllKeyboardEvents option.  There's some controversy as to whether this is the right thing to do.
-#      See discussion in #1415. This implements option 2 from there, although option 3 would be a reasonable
+#      See discussion in #1415. This implements Option 2 from there, although Option 3 would be a reasonable
 #      alternative.
 #   3. If the very-next keystroke is Escape, then drop immediately into insert mode.
 #
@@ -16,9 +16,10 @@ class PostFindMode extends InsertModeBlocker
 
     super
       name: "post-find"
-      # Be a singleton.  That way, we don't have to keep track of any currently-active instance.  Such  an
-      # instance is automatically deactivated when a new instance is created.
+      # Be a singleton.  That way, we don't have to keep track of any currently-active instance.  Any active
+      # instance is automatically deactivated when a new instance is activated.
       singleton: PostFindMode
+      exitOnBlur: element
       trapAllKeyboardEvents: element
 
     return @exit() unless element and findModeAnchorNode
@@ -42,11 +43,5 @@ class PostFindMode extends InsertModeBlocker
         @remove()
         true
 
-    # Various ways in which we can leave PostFindMode.
-    @push
-      focus: (event) => @alwaysContinueBubbling => @exit()
-      blur: (event) => @alwaysContinueBubbling => @exit()
-      keydown: (event) => @alwaysContinueBubbling => @exit() if document.activeElement != element
-
 root = exports ? window
 root.PostFindMode = PostFindMode
diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee
index 9a2d5ce1..144b0be6 100644
--- a/content_scripts/mode_insert.coffee
+++ b/content_scripts/mode_insert.coffee
@@ -1,5 +1,5 @@
 
-# This mode is installed when insert mode is active.
+# This mode is installed only when insert mode is active.
 class InsertMode extends Mode
   constructor: (options = {}) ->
     defaults =
@@ -11,10 +11,10 @@ class InsertMode extends Mode
       keyup: (event) => @stopBubblingAndTrue
       exitOnEscape: true
       blurOnExit: true
+      targetElement: null
 
-    options = extend defaults, options
-    options.exitOnBlur = options.targetElement || null
-    super options
+    options.exitOnBlur ||= options.targetElement
+    super extend defaults, options
     triggerSuppressor.suppress()
 
   exit: (event = null) ->
@@ -34,8 +34,8 @@ class InsertMode extends Mode
 #   - On a keydown event in a contentEditable element.
 #   - When a focusable element receives the focus.
 #
-# The trigger can be suppressed via triggerSuppressor; see InsertModeBlocker, below.
-# This mode is permanently installed fairly low down on the handler stack.
+# The trigger can be suppressed via triggerSuppressor; see InsertModeBlocker, below.  This mode is permanently
+# installed (just above normal mode and passkeys mode) on the handler stack.
 class InsertModeTrigger extends Mode
   constructor: ->
     super
@@ -44,7 +44,7 @@ class InsertModeTrigger extends Mode
         triggerSuppressor.unlessSuppressed =>
           # Some sites (e.g. inbox.google.com) change the contentEditable attribute on the fly (see #1245);
           # and unfortunately, the focus event happens *before* the change is made.  Therefore, we need to
-          # check again whether the active element is contentEditable.
+          # check (on every keydown) whether the active element is contentEditable.
           return @continueBubbling unless document.activeElement?.isContentEditable
           new InsertMode
             targetElement: document.activeElement
@@ -58,7 +58,7 @@ class InsertModeTrigger extends Mode
               new InsertMode
                 targetElement: event.target
 
-    # We may already have focussed an input, so check.
+    # We may have already focussed an input element, so check.
     if document.activeElement and DomUtils.isEditable document.activeElement
       new InsertMode
         targetElement: document.activeElement
@@ -66,12 +66,13 @@ class InsertModeTrigger extends Mode
 # Used by InsertModeBlocker to suppress InsertModeTrigger; see below.
 triggerSuppressor = new Utils.Suppressor true # Note: true == @continueBubbling
 
-# Suppresses InsertModeTrigger.  This is used by various modes (usually by inheritance) to prevent
+# Suppresses InsertModeTrigger.  This is used by various modes (usually via inheritance) to prevent
 # unintentionally dropping into insert mode on focusable elements.
 class InsertModeBlocker extends Mode
   constructor: (options = {}) ->
     triggerSuppressor.suppress()
     options.name ||= "insert-blocker"
+    # See "click" handler below for an explanation of options.onClickMode.
     options.onClickMode ||= InsertMode
     super options
     @onExit -> triggerSuppressor.unsuppress()
@@ -79,27 +80,29 @@ class InsertModeBlocker extends Mode
     @push
       "click": (event) =>
         @alwaysContinueBubbling =>
-          # The user knows best; so, if the user clicks on something, we get out of the way.
+          # The user knows best; so, if the user clicks on something, the insert-mode blocker gets out of the
+          # way.
           @exit event
-          # However, there's a corner case.  If the active element is focusable, then we would have been in
-          # insert mode had we not been blocking the trigger.  Now, clicking on the element will not generate
-          # a new focus event, so the insert-mode trigger will not fire.  We have to handle this case
-          # specially.  @options.onClickMode is the mode to use.
+          # However, there's a corner case.  If the active element is focusable, then, had we not been
+          # blocking the trigger, we would already have been in insert mode.  Now, a click on that element
+          # will not generate a new focus event, so the insert-mode trigger will not fire.  We have to handle
+          # this case specially.  @options.onClickMode specifies the mode to use (by default, insert mode).
           if document.activeElement and
-              event.target == document.activeElement and DomUtils.isEditable document.activeElement
+             event.target == document.activeElement and DomUtils.isEditable document.activeElement
             new @options.onClickMode
               targetElement: document.activeElement
 
 # There's some unfortunate feature interaction with chrome's content editable handling.  If the selection is
 # content editable and a descendant of the active element, then chrome focuses it on any unsuppressed keyboard
-# events.  This has the unfortunate effect of dropping us unintentally into insert mode.  See #1415.
-# This mode sits near the bottom of the handler stack and suppresses keyboard events if:
+# event.  This has the unfortunate effect of dropping us unintentally into insert mode.  See #1415.
+# A single instance of this mode sits near the bottom of the handler stack and suppresses keyboard events if:
 #   - they haven't been handled by any other mode (so not by normal mode, passkeys mode, insert mode, and so
-#     on), and
+#     on),
 #   - 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 it shouldn't normally block other extensions or the page itself from
+# circumstances outlined above.  So, we shouldn't usually be blocking keyboard events for other extensions or
+# the page itself.
 # handling keyboard events.
 new class ContentEditableTrap extends Mode
   constructor: ->
@@ -109,17 +112,15 @@ new class ContentEditableTrap extends Mode
       keypress: (event) => @handle => @suppressEvent
       keyup: (event) => @handle => @suppressEvent
 
-  # True if the selection is content editable and a descendant of the active element.  In this situation,
-  # chrome unilaterally focuses the element containing the anchor, dropping us into insert mode.
+  handle: (func) -> if @isContentEditableFocused() then func() else @continueBubbling
+
+  # True if the selection is content editable and a descendant of the active element.
   isContentEditableFocused: ->
     element = document.getSelection()?.anchorNode?.parentElement
-    return element?.isContentEditable? and
-             document.activeElement? and
+    return element?.isContentEditable and
+             document.activeElement and
              DomUtils.isDOMDescendant document.activeElement, element
 
-  handle: (func) ->
-    if @isContentEditableFocused() then func() else @continueBubbling
-
 root = exports ? window
 root.InsertMode = InsertMode
 root.InsertModeTrigger = InsertModeTrigger
diff --git a/content_scripts/mode_passkeys.coffee b/content_scripts/mode_passkeys.coffee
index 972dcad7..112e14ed 100644
--- a/content_scripts/mode_passkeys.coffee
+++ b/content_scripts/mode_passkeys.coffee
@@ -3,24 +3,23 @@ class PassKeysMode extends Mode
   constructor: ->
     super
       name: "passkeys"
-      keydown: (event) => @handlePassKeyEvent event
-      keypress: (event) => @handlePassKeyEvent event
       trackState: true
+      keydown: (event) => @handleKeyChar KeyboardUtils.getKeyChar event
+      keypress: (event) => @handleKeyChar String.fromCharCode event.charCode
+      keyup: (event) => @handleKeyChar String.fromCharCode event.charCode
 
   # Decide whether this event should be passed to the underlying page.  Keystrokes are *never* considered
   # 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.
-  handlePassKeyEvent: (event) ->
-    for keyChar in [KeyboardUtils.getKeyChar(event), String.fromCharCode(event.charCode)]
-      return @stopBubblingAndTrue if keyChar and not @keyQueue and 0 <= @passKeys.indexOf(keyChar)
+  handleKeyChar: (keyChar) ->
+    return @stopBubblingAndTrue if keyChar and not @keyQueue and 0 <= @passKeys.indexOf keyChar
     @continueBubbling
 
   configure: (request) ->
     @keyQueue = request.keyQueue if request.keyQueue?
 
   chooseBadge: (badge) ->
-    @badge = if @passKeys and not @keyQueue then "P" else ""
-    super badge
+    badge.badge ||= "P" if @passKeys and not @keyQueue
 
 root = exports ? window
 root.PassKeysMode = PassKeysMode
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index 97fbc56f..a9bf30a3 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -384,8 +384,8 @@ extend window,
           # shouldn't happen anyway.  However, it does no harm to enforce it.
           singleton: FocusSelector
           targetMode: targetMode
-          # For the InsertModeBlocker super-class (we'll always choose InsertMode on click).  See comment in
-          # InsertModeBlocker for an explanation of why this is needed.
+          # Set the target mode for when/if the active element is clicked.  Usually, the target is insert
+          # mode.  See comment in InsertModeBlocker for an explanation of why this is needed.
           onClickMode: targetMode
           keydown: (event) =>
             if event.keyCode == KeyboardUtils.keyCodes.tab
@@ -397,22 +397,22 @@ extend window,
               @suppressEvent
             else unless event.keyCode == KeyboardUtils.keyCodes.shiftKey
               @exit event
-              @continueBubbling
+              # In @exit(), we just pushed a new mode (usually insert mode).  Restart bubbling, so that the
+              # new mode can now see the event too.
+              @restartBubbling
 
         visibleInputs[selectedInputIndex].element.focus()
-        if visibleInputs.length == 1
-          @exit()
-        else
-          hints[selectedInputIndex].classList.add 'internalVimiumSelectedInputHint'
+        return @exit() if visibleInputs.length == 1
+
+        hints[selectedInputIndex].classList.add 'internalVimiumSelectedInputHint'
 
       exit: ->
         super()
         DomUtils.removeElement hintContainingDiv
         if document.activeElement == visibleInputs[selectedInputIndex].element
-          # The InsertModeBlocker super-class  handles the "click" case.
+          # The InsertModeBlocker super-class  handles "click" events, so we should skip it here.
           unless event?.type == "click"
-            # In the legacy (and probably common) case, we're entering insert mode here.  However, it could be
-            # some other mode.
+            # In most cases, we're entering insert mode here.  However, it could be some other mode.
             new @options.targetMode
               targetElement: document.activeElement
 
@@ -455,7 +455,7 @@ KeydownEvents =
 # Note that some keys will only register keydown events and not keystroke events, e.g. ESC.
 #
 
-onKeypress = (event, extra) ->
+onKeypress = (event) ->
   keyChar = ""
 
   # Ignore modifier keys by themselves.
@@ -484,7 +484,7 @@ onKeypress = (event, extra) ->
 
   return true
 
-onKeydown = (event, extra) ->
+onKeydown = (event) ->
   keyChar = ""
 
   # handle special keys, and normal input keys with modifiers being pressed. don't handle shiftKey alone (to
@@ -789,6 +789,7 @@ class FindMode extends InsertModeBlocker
     super()
     handleEscapeForFindMode() if event?.type == "keydown" and KeyboardUtils.isEscape event
     handleEscapeForFindMode() if event?.type == "click"
+    # If event?.type == "click", then the InsertModeBlocker super-class will be dropping us into insert mode.
     new PostFindMode findModeAnchorNode unless event?.type == "click"
 
 performFindInPlace = ->
diff --git a/lib/handler_stack.coffee b/lib/handler_stack.coffee
index 97e189c5..44c7538b 100644
--- a/lib/handler_stack.coffee
+++ b/lib/handler_stack.coffee
@@ -14,6 +14,11 @@ class HandlerStack
     # processing should take place.
     @stopBubblingAndFalse = new Object()
 
+    # A handler should return this value to indicate that bubbling should be restarted.  Typically, this is
+    # used when, while bubbling an event, a new mode is pushed onto the stack.  See `focusInput` for an
+    # example.
+    @restartBubbling = new Object()
+
   # Adds a handler to the top of the stack. Returns a unique ID for that handler that can be used to remove it
   # later.
   push: (handler) ->
@@ -30,30 +35,26 @@ class HandlerStack
   # event's propagation by returning a falsy value, or stop bubbling by returning @stopBubblingAndFalse or
   # @stopBubblingAndTrue.
   bubbleEvent: (type, event) ->
-    # extra is passed to each handler.  This allows handlers to pass information down the stack.
-    extra = {}
-    # We take a copy of the array, here, in order to avoid interference from concurrent removes (for example,
-    # to avoid calling the same handler twice).
+    # We take a copy of the array in order to avoid interference from concurrent removes (for example, to
+    # avoid calling the same handler twice, because elements have been spliced out of the array by remove).
     for handler in @stack[..].reverse()
-      # A handler may have been removed (handler.id == null).
-      if handler and handler.id
+      # A handler may have been removed (handler.id == null), so check.
+      if handler?.id and handler[type]
         @currentId = handler.id
-        # A handler can register a handler for type "all", which will be invoked on all events.  Such an "all"
-        # handler will be invoked first.
-        for func in [ handler.all, handler[type] ]
-          if func
-            passThrough = func.call @, event, extra
-            if not passThrough
-              DomUtils.suppressEvent(event) if @isChromeEvent event
-              return false
-            return true if passThrough == @stopBubblingAndTrue
-            return false if passThrough == @stopBubblingAndFalse
+        result = handler[type].call @, event
+        if not result
+          DomUtils.suppressEvent(event) if @isChromeEvent event
+          return false
+        return true if result == @stopBubblingAndTrue
+        return false if result == @stopBubblingAndFalse
+        return @bubbleEvent type, event if result == @restartBubbling
     true
 
   remove: (id = @currentId) ->
     for i in [(@stack.length - 1)..0] by -1
       handler = @stack[i]
       if handler.id == id
+        # Mark the handler as removed.
         handler.id = null
         @stack.splice(i, 1)
         break
@@ -63,7 +64,9 @@ class HandlerStack
   isChromeEvent: (event) ->
     event?.preventDefault? or event?.stopImmediatePropagation?
 
-  # Convenience wrappers.
+  # Convenience wrappers.  Handlers must return an approriate value.  These are wrappers which handlers can
+  # use to always return the same value.  This then means that the handler itself can be implemented without
+  # regard to its return value.
   alwaysContinueBubbling: (handler) ->
     handler()
     true
-- 
cgit v1.2.3


From fdcdd0113049042c94b2b56a6b716e2da58b860e Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 10 Jan 2015 08:25:02 +0000
Subject: Modes; instrument for debugging...

- Set Mode.debug to true to see mode activation/deactivation on the
  console.
- Use Mode.log() to see a list of currently-active modes.
- Use handlerStack.debugOn() to enable debugging of the handler stack.
---
 content_scripts/mode.coffee            | 55 +++++++++++++++++++++++++---------
 content_scripts/mode_find.coffee       |  1 +
 content_scripts/mode_insert.coffee     | 20 +++++++------
 content_scripts/scroller.coffee        | 15 ++++++----
 content_scripts/vimium_frontend.coffee | 14 ++++++---
 lib/dom_utils.coffee                   |  1 +
 lib/handler_stack.coffee               | 34 ++++++++++++++++++++-
 7 files changed, 107 insertions(+), 33 deletions(-)

diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee
index 37f3a8c2..a33197b0 100644
--- a/content_scripts/mode.coffee
+++ b/content_scripts/mode.coffee
@@ -38,12 +38,14 @@
 #   myMode.exit() # externally triggered.
 #
 
-# For debug only; to be stripped out.
+# For debug only.
 count = 0
 
 class Mode
-  # If this is true, then we generate a trace of modes being activated and deactivated on the console.
-  @debug = true
+  # If Mode.debug is true, then we generate a trace of modes being activated and deactivated on the console, along
+  # with a list of the currently active modes.
+  debug: true
+  @modes: []
 
   # Constants; short, readable names for handlerStack event-handler return values.
   continueBubbling: true
@@ -60,7 +62,8 @@ class Mode
     @name = @options.name || "anonymous"
 
     @count = ++count
-    console.log @count, "create:", @name if Mode.debug
+    @id = "#{@name}-#{@count}"
+    @logger "activate:", @id if @debug
 
     @push
       keydown: @options.keydown || null
@@ -80,6 +83,7 @@ class Mode
       # Note. This handler ends up above the mode's own key handlers on the handler stack, so it takes
       # priority.
       @push
+        _name: "mode-#{@id}/exitOnEscape"
         "keydown": (event) =>
           return @continueBubbling unless KeyboardUtils.isEscape event
           @exit event
@@ -90,6 +94,7 @@ class Mode
     # loses the focus.
     if @options.exitOnBlur
       @push
+        _name: "mode-#{@id}/exitOnBlur"
         "blur": (event) => @alwaysContinueBubbling => @exit() if event.srcElement == @options.exitOnBlur
 
     # If @options.trackState is truthy, then the mode mainatins the current state in @enabled and @passKeys,
@@ -98,6 +103,7 @@ class Mode
       @enabled = false
       @passKeys = ""
       @push
+        _name: "mode-#{@id}/registerStateChange"
         "registerStateChange": ({ enabled: enabled, passKeys: passKeys }) =>
           @alwaysContinueBubbling =>
             if enabled != @enabled or passKeys != @passKeys
@@ -110,30 +116,38 @@ class Mode
     # from propagating to other extensions or the host page.
     if @options.trapAllKeyboardEvents
       @unshift
-        keydown: (event) => @alwaysContinueBubbling =>
-          DomUtils.suppressPropagation event if event.srcElement == @options.trapAllKeyboardEvents
-        keypress: (event) => @alwaysContinueBubbling =>
-          DomUtils.suppressEvent event if event.srcElement == @options.trapAllKeyboardEvents
-        keyup: (event) => @alwaysContinueBubbling =>
-          DomUtils.suppressPropagation event if event.srcElement == @options.trapAllKeyboardEvents
+        _name: "mode-#{@id}/trapAllKeyboardEvents"
+        keydown: (event) =>
+          if event.srcElement == @options.trapAllKeyboardEvents then @suppressEvent else @continueBubbling
+        keypress: (event) =>
+          if event.srcElement == @options.trapAllKeyboardEvents then @suppressEvent else @continueBubbling
+        keyup: (event) =>
+          if event.srcElement == @options.trapAllKeyboardEvents then @suppressEvent else @continueBubbling
 
     Mode.updateBadge() if @badge
-    # End of Mode.constructor().
+    Mode.modes.push @
+    @log() if @debug
+    handlerStack.debugOn()
+    # End of Mode constructor.
 
   push: (handlers) ->
+    handlers._name ||= "mode-#{@id}"
     @handlers.push handlerStack.push handlers
 
   unshift: (handlers) ->
-    @handlers.unshift handlerStack.push handlers
+    handlers._name ||= "mode-#{@id}"
+    handlers._name += "/unshifted"
+    @handlers.push handlerStack.unshift handlers
 
   onExit: (handler) ->
     @exitHandlers.push handler
 
   exit: ->
     if @modeIsActive
-      console.log @count, "exit:", @name if Mode.debug
+      @logger "deactivate:", @id if @debug
       handler() for handler in @exitHandlers
       handlerStack.remove handlerId for handlerId in @handlers
+      Mode.modes = Mode.modes.filter (mode) => mode != @
       Mode.updateBadge()
       @modeIsActive = false
 
@@ -177,12 +191,24 @@ class Mode
       # flickering in some cases.
       Mode.badgeSuppressor.runSuppresed =>
         if singletons[key]
-          console.log singletons[key].count, "singleton:", @name, "(deactivating)"
+          @logger "singleton:", "deactivating #{singletons[key].id}" if @debug
           singletons[key].exit()
       singletons[key] = @
 
       @onExit => delete singletons[key] if singletons[key] == @
 
+  # Debugging routines.
+  log: ->
+    if Mode.modes.length == 0
+      @logger "It looks like debugging is not enabled in modes.coffee."
+    else
+      @logger "active modes (top to bottom), current: #{@id}"
+      for mode in Mode.modes[..].reverse()
+        @logger " ",  mode.id
+
+  logger: (args...) ->
+    handlerStack.log args...
+
 # BadgeMode is a pseudo mode for triggering badge updates on focus changes and state updates. It sits at the
 # bottom of the handler stack, and so it receives state changes *after* all other modes, and can override the
 # badge choice of the other active modes.
@@ -194,6 +220,7 @@ new class BadgeMode extends Mode
       trackState: true
 
     @push
+      _name: "mode-#{@id}/focus"
       "focus": => @alwaysContinueBubbling -> Mode.updateBadge()
 
   chooseBadge: (badge) ->
diff --git a/content_scripts/mode_find.coffee b/content_scripts/mode_find.coffee
index d63b3319..0ce03af6 100644
--- a/content_scripts/mode_find.coffee
+++ b/content_scripts/mode_find.coffee
@@ -33,6 +33,7 @@ class PostFindMode extends InsertModeBlocker
 
     self = @
     @push
+      _name: "mode-#{@id}/handle-escape"
       keydown: (event) ->
         if element == document.activeElement and KeyboardUtils.isEscape event
           self.exit()
diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee
index 144b0be6..7668d794 100644
--- a/content_scripts/mode_insert.coffee
+++ b/content_scripts/mode_insert.coffee
@@ -51,6 +51,7 @@ class InsertModeTrigger extends Mode
           @stopBubblingAndTrue
 
     @push
+      _name: "mode-#{@id}/activate-on-focus"
       focus: (event) =>
         triggerSuppressor.unlessSuppressed =>
           @alwaysContinueBubbling =>
@@ -78,6 +79,7 @@ class InsertModeBlocker extends Mode
     @onExit -> triggerSuppressor.unsuppress()
 
     @push
+      _name: "mode-#{@id}/bail-on-click"
       "click": (event) =>
         @alwaysContinueBubbling =>
           # The user knows best; so, if the user clicks on something, the insert-mode blocker gets out of the
@@ -92,18 +94,18 @@ class InsertModeBlocker extends Mode
             new @options.onClickMode
               targetElement: document.activeElement
 
-# There's some unfortunate feature interaction with chrome's content editable handling.  If the selection is
-# content editable and a descendant of the active element, then chrome focuses it on any unsuppressed keyboard
-# event.  This has the unfortunate effect of dropping us unintentally into insert mode.  See #1415.
-# A single instance of this mode sits near the bottom of the handler stack and suppresses keyboard events if:
-#   - they haven't been handled by any other mode (so not by normal mode, passkeys mode, insert mode, and so
-#     on),
+# There's some unfortunate feature interaction with chrome's contentEditable handling.  If the selection is
+# contentEditable and a descendant of the active element, then chrome focuses it on any unsuppressed keyboard
+# event.  This has the unfortunate effect of dropping us unintentally into insert mode.  See #1415.  A single
+# instance of this mode sits near the bottom of the handler stack and suppresses keyboard events if:
+#   - they haven't been handled by any other mode (so not by normal mode, passkeys, insert, ...),
 #   - 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.
-# handling keyboard events.
+# There's some controversy as to whether this is the right thing to do.  See discussion in #1415. This
+# implements Option 2 from there, although Option 3 would be a reasonable alternative.
 new class ContentEditableTrap extends Mode
   constructor: ->
     super
@@ -112,10 +114,10 @@ new class ContentEditableTrap extends Mode
       keypress: (event) => @handle => @suppressEvent
       keyup: (event) => @handle => @suppressEvent
 
-  handle: (func) -> if @isContentEditableFocused() then func() else @continueBubbling
+  handle: (func) -> if @wouldTriggerInsert() then func() else @continueBubbling
 
   # True if the selection is content editable and a descendant of the active element.
-  isContentEditableFocused: ->
+  wouldTriggerInsert: ->
     element = document.getSelection()?.anchorNode?.parentElement
     return element?.isContentEditable and
              document.activeElement and
diff --git a/content_scripts/scroller.coffee b/content_scripts/scroller.coffee
index 889dc042..f70d3aed 100644
--- a/content_scripts/scroller.coffee
+++ b/content_scripts/scroller.coffee
@@ -124,12 +124,15 @@ CoreScroller =
     @keyIsDown = false
 
     handlerStack.push
+      _name: 'scroller/track-key-down/up'
       keydown: (event) =>
-        @keyIsDown = true
-        @lastEvent = event
+        handlerStack.alwaysContinueBubbling =>
+          @keyIsDown = true
+          @lastEvent = event
       keyup: =>
-        @keyIsDown = false
-        @time += 1
+        handlerStack.alwaysContinueBubbling =>
+          @keyIsDown = false
+          @time += 1
 
   # Return true if CoreScroller would not initiate a new scroll right now.
   wouldNotInitiateScroll: -> @lastEvent?.repeat and @settings.get "smoothScroll"
@@ -205,7 +208,9 @@ CoreScroller =
 # Scroller contains the two main scroll functions (scrollBy and scrollTo) which are exported to clients.
 Scroller =
   init: (frontendSettings) ->
-    handlerStack.push DOMActivate: -> activatedElement = event.target
+    handlerStack.push
+      _name: 'scroller/active-element'
+      DOMActivate: (event) -> handlerStack.alwaysContinueBubbling -> activatedElement = event.target
     CoreScroller.init frontendSettings
 
   # scroll the active element in :direction by :amount * :factor.
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index a9bf30a3..1406b1e7 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -396,10 +396,16 @@ extend window,
               visibleInputs[selectedInputIndex].element.focus()
               @suppressEvent
             else unless event.keyCode == KeyboardUtils.keyCodes.shiftKey
-              @exit event
-              # In @exit(), we just pushed a new mode (usually insert mode).  Restart bubbling, so that the
-              # new mode can now see the event too.
-              @restartBubbling
+              mode = @exit event
+              if mode
+                # In @exit(), we just pushed a new mode (usually insert mode).  Restart bubbling, so that the
+                # new mode can now see the event too.
+                # Exception: If the new mode exits on Escape, and this key event is Escape, then rebubbling the
+                # event will just cause the mode to exit immediately.  So we suppress Escapes.
+                if mode.options.exitOnEscape and KeyboardUtils.isEscape event
+                  @suppressEvent
+                else
+                  @restartBubbling
 
         visibleInputs[selectedInputIndex].element.focus()
         return @exit() if visibleInputs.length == 1
diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee
index 9d7ca867..322188b3 100644
--- a/lib/dom_utils.coffee
+++ b/lib/dom_utils.coffee
@@ -206,6 +206,7 @@ DomUtils =
   # Suppress the next keyup event for Escape.
   suppressKeyupAfterEscape: (handlerStack) ->
     handlerStack.push
+      _name: "dom_utils/suppressKeyupAfterEscape"
       keyup: (event) ->
         return true unless KeyboardUtils.isEscape event
         @remove()
diff --git a/lib/handler_stack.coffee b/lib/handler_stack.coffee
index 44c7538b..22d04941 100644
--- a/lib/handler_stack.coffee
+++ b/lib/handler_stack.coffee
@@ -3,6 +3,8 @@ root = exports ? window
 class HandlerStack
 
   constructor: ->
+    @debug = false
+    @eventNumber = 0
     @stack = []
     @counter = 0
 
@@ -22,8 +24,10 @@ class HandlerStack
   # Adds a handler to the top of the stack. Returns a unique ID for that handler that can be used to remove it
   # later.
   push: (handler) ->
-    @stack.push handler
     handler.id = ++@counter
+    handler._name ||= "anon-#{@counter}"
+    @stack.push handler
+    handler.id
 
   # Adds a handler to the bottom of the stack. Returns a unique ID for that handler that can be used to remove
   # it later.
@@ -35,6 +39,7 @@ class HandlerStack
   # event's propagation by returning a falsy value, or stop bubbling by returning @stopBubblingAndFalse or
   # @stopBubblingAndTrue.
   bubbleEvent: (type, event) ->
+    @eventNumber += 1
     # We take a copy of the array in order to avoid interference from concurrent removes (for example, to
     # avoid calling the same handler twice, because elements have been spliced out of the array by remove).
     for handler in @stack[..].reverse()
@@ -42,6 +47,7 @@ class HandlerStack
       if handler?.id and handler[type]
         @currentId = handler.id
         result = handler[type].call @, event
+        @logResult type, event, handler, result if @debug
         if not result
           DomUtils.suppressEvent(event) if @isChromeEvent event
           return false
@@ -75,5 +81,31 @@ class HandlerStack
     handler()
     false
 
+  # Debugging.
+  debugOn: -> @debug = true
+  debugOff: -> @debug = false
+
+  logResult: (type, event, handler, result) ->
+    # FIXME(smblott).  Badge updating is too noisy, so we filter it out.  However, we do need to look at how
+    # many badge update events are happening.  It seems to be more than necessary.
+    return if type == "updateBadge"
+    label =
+      switch result
+        when @stopBubblingAndTrue then "stop/true"
+        when @stopBubblingAndFalse then "stop/false"
+        when @restartBubbling then "rebubble"
+        when true then "continue"
+    label ||= if result then "continue/truthy" else "suppress"
+    @log @eventNumber, type, handler._name, label
+
+  logRecords: []
+  log: (args...) ->
+    line = args.join " "
+    @logRecords.push line
+    console.log line
+
+  clipLog: ->
+    Clipboard.copy logRecords.join "\n"
+
 root.HandlerStack = HandlerStack
 root.handlerStack = new HandlerStack
-- 
cgit v1.2.3


From 2199ad1bf9a7b063cc68a8e75f7a4a76ba125588 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 10 Jan 2015 11:31:57 +0000
Subject: Modes; revert to master's handling of #1415.

The behaviour in the situations described in #1415 require more thought
and discussion.
---
 content_scripts/mode_find.coffee   |  9 ++-------
 content_scripts/mode_insert.coffee | 29 -----------------------------
 2 files changed, 2 insertions(+), 36 deletions(-)

diff --git a/content_scripts/mode_find.coffee b/content_scripts/mode_find.coffee
index 0ce03af6..740358d5 100644
--- a/content_scripts/mode_find.coffee
+++ b/content_scripts/mode_find.coffee
@@ -1,14 +1,10 @@
 # NOTE(smblott).  Ultimately, all of the FindMode-related code should be moved to this file.
 
 # When we use find mode, the selection/focus can end up in a focusable/editable element.  In this situation,
-# special considerations apply.  We implement three special cases:
+# special considerations apply.  We implement two special cases:
 #   1. Prevent keyboard events from dropping us unintentionally into insert mode. This is achieved by
 #      inheriting from InsertModeBlocker.
-#   2. Prevent all keyboard events on the active element from propagating.  This is achieved by setting the
-#      trapAllKeyboardEvents option.  There's some controversy as to whether this is the right thing to do.
-#      See discussion in #1415. This implements Option 2 from there, although Option 3 would be a reasonable
-#      alternative.
-#   3. If the very-next keystroke is Escape, then drop immediately into insert mode.
+#   2. If the very-next keystroke is Escape, then drop immediately into insert mode.
 #
 class PostFindMode extends InsertModeBlocker
   constructor: (findModeAnchorNode) ->
@@ -20,7 +16,6 @@ class PostFindMode extends InsertModeBlocker
       # instance is automatically deactivated when a new instance is activated.
       singleton: PostFindMode
       exitOnBlur: element
-      trapAllKeyboardEvents: element
 
     return @exit() unless element and findModeAnchorNode
 
diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee
index 7668d794..1887adbc 100644
--- a/content_scripts/mode_insert.coffee
+++ b/content_scripts/mode_insert.coffee
@@ -94,35 +94,6 @@ class InsertModeBlocker extends Mode
             new @options.onClickMode
               targetElement: document.activeElement
 
-# There's some unfortunate feature interaction with chrome's contentEditable handling.  If the selection is
-# contentEditable and a descendant of the active element, then chrome focuses it on any unsuppressed keyboard
-# event.  This has the unfortunate effect of dropping us unintentally into insert mode.  See #1415.  A single
-# instance of this mode sits near the bottom of the handler stack and suppresses keyboard events if:
-#   - they haven't been handled by any other mode (so not by normal mode, passkeys, insert, ...),
-#   - 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, although Option 3 would be a reasonable alternative.
-new class ContentEditableTrap extends Mode
-  constructor: ->
-    super
-      name: "content-editable-trap"
-      keydown: (event) => @handle => DomUtils.suppressPropagation event
-      keypress: (event) => @handle => @suppressEvent
-      keyup: (event) => @handle => @suppressEvent
-
-  handle: (func) -> if @wouldTriggerInsert() then func() else @continueBubbling
-
-  # True if the selection is content editable and a descendant of the active element.
-  wouldTriggerInsert: ->
-    element = document.getSelection()?.anchorNode?.parentElement
-    return element?.isContentEditable and
-             document.activeElement and
-             DomUtils.isDOMDescendant document.activeElement, element
-
 root = exports ? window
 root.InsertMode = InsertMode
 root.InsertModeTrigger = InsertModeTrigger
-- 
cgit v1.2.3


From 35cb54fec7242fac5c68503a32ef9dd4fea5d9b6 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 10 Jan 2015 12:17:22 +0000
Subject: Modes; minor changes.

---
 content_scripts/mode_insert.coffee | 4 +++-
 content_scripts/scroller.coffee    | 2 +-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee
index 1887adbc..5720c901 100644
--- a/content_scripts/mode_insert.coffee
+++ b/content_scripts/mode_insert.coffee
@@ -1,5 +1,6 @@
 
-# This mode is installed only when insert mode is active.
+# This mode is installed only when insert mode is active.  It is a singleton, so a newly-activated instance
+# displaces any active instance.
 class InsertMode extends Mode
   constructor: (options = {}) ->
     defaults =
@@ -13,6 +14,7 @@ class InsertMode extends Mode
       blurOnExit: true
       targetElement: null
 
+    # If options.targetElement blurs, we exit.
     options.exitOnBlur ||= options.targetElement
     super extend defaults, options
     triggerSuppressor.suppress()
diff --git a/content_scripts/scroller.coffee b/content_scripts/scroller.coffee
index f70d3aed..6e2e1ffc 100644
--- a/content_scripts/scroller.coffee
+++ b/content_scripts/scroller.coffee
@@ -124,7 +124,7 @@ CoreScroller =
     @keyIsDown = false
 
     handlerStack.push
-      _name: 'scroller/track-key-down/up'
+      _name: 'scroller/track-key-status'
       keydown: (event) =>
         handlerStack.alwaysContinueBubbling =>
           @keyIsDown = true
-- 
cgit v1.2.3


From c554d1fd5b6d81506864516b6f86a14f8672bec5 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 10 Jan 2015 15:05:58 +0000
Subject: Modes; reinstate key blockers:

- when the selection is contentEditable
- in PostFindMode

Restricted to printable characters.
---
 content_scripts/mode.coffee        | 20 +++++++++-----------
 content_scripts/mode_find.coffee   |  9 +++++++--
 content_scripts/mode_insert.coffee | 28 ++++++++++++++++++++++++++++
 lib/dom_utils.coffee               |  3 +++
 4 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee
index a33197b0..5c6201e0 100644
--- a/content_scripts/mode.coffee
+++ b/content_scripts/mode.coffee
@@ -111,23 +111,21 @@ class Mode
               @passKeys = passKeys
               @registerStateChange?()
 
-    # If @options.trapAllKeyboardEvents is truthy, then it should be an element.  All keyboard events on that
-    # element are suppressed *after* bubbling the event down the handler stack.  This prevents such events
-    # from propagating to other extensions or the host page.
-    if @options.trapAllKeyboardEvents
+    # 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
       @unshift
-        _name: "mode-#{@id}/trapAllKeyboardEvents"
-        keydown: (event) =>
-          if event.srcElement == @options.trapAllKeyboardEvents then @suppressEvent else @continueBubbling
+        _name: "mode-#{@id}/suppressPrintableEvents"
         keypress: (event) =>
-          if event.srcElement == @options.trapAllKeyboardEvents then @suppressEvent else @continueBubbling
-        keyup: (event) =>
-          if event.srcElement == @options.trapAllKeyboardEvents then @suppressEvent else @continueBubbling
+          if DomUtils.isPrintable(event) and
+            event.srcElement == @options.suppressPrintableEvents then @suppressEvent else @continueBubbling
 
     Mode.updateBadge() if @badge
     Mode.modes.push @
     @log() if @debug
-    handlerStack.debugOn()
+    # handlerStack.debugOn()
     # End of Mode constructor.
 
   push: (handlers) ->
diff --git a/content_scripts/mode_find.coffee b/content_scripts/mode_find.coffee
index 740358d5..91ae4507 100644
--- a/content_scripts/mode_find.coffee
+++ b/content_scripts/mode_find.coffee
@@ -1,10 +1,14 @@
 # NOTE(smblott).  Ultimately, all of the FindMode-related code should be moved to this file.
 
 # When we use find mode, the selection/focus can end up in a focusable/editable element.  In this situation,
-# special considerations apply.  We implement two special cases:
+# special considerations apply.  We implement three special cases:
 #   1. Prevent keyboard events from dropping us unintentionally into insert mode. This is achieved by
 #      inheriting from InsertModeBlocker.
-#   2. If the very-next keystroke is Escape, then drop immediately into insert mode.
+#   2. Prevent all printable keyboard events on the active element from propagating.  This is achieved by setting the
+#      suppressPrintableEvents option.  There's some controversy as to whether this is the right thing to do.
+#      See discussion in #1415. This implements Option 2 from there, although Option 3 would be a reasonable
+#      alternative.
+#   3. If the very-next keystroke is Escape, then drop immediately into insert mode.
 #
 class PostFindMode extends InsertModeBlocker
   constructor: (findModeAnchorNode) ->
@@ -16,6 +20,7 @@ class PostFindMode extends InsertModeBlocker
       # instance is automatically deactivated when a new instance is activated.
       singleton: PostFindMode
       exitOnBlur: element
+      suppressPrintableEvents: element
 
     return @exit() unless element and findModeAnchorNode
 
diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee
index 5720c901..b907f22e 100644
--- a/content_scripts/mode_insert.coffee
+++ b/content_scripts/mode_insert.coffee
@@ -96,6 +96,34 @@ 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
+             DomUtils. isPrintable event and
+             DomUtils.isDOMDescendant document.activeElement, element
+
 root = exports ? window
 root.InsertMode = InsertMode
 root.InsertModeTrigger = InsertModeTrigger
diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee
index 322188b3..fd2427c4 100644
--- a/lib/dom_utils.coffee
+++ b/lib/dom_utils.coffee
@@ -212,5 +212,8 @@ DomUtils =
         @remove()
         false
 
+  isPrintable: (event) ->
+    not (event.metaKey or event.ctrlKey or event.altKey)
+
 root = exports ? window
 root.DomUtils = DomUtils
-- 
cgit v1.2.3


From 704ae28629154a732e20e16d56b23af265d51b85 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 10 Jan 2015 16:01:40 +0000
Subject: Modes; better printable detection, move to keyboard_utils.

---
 content_scripts/mode.coffee        | 2 +-
 content_scripts/mode_insert.coffee | 2 +-
 lib/dom_utils.coffee               | 3 ---
 lib/keyboard_utils.coffee          | 6 ++++++
 4 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee
index 5c6201e0..19354d94 100644
--- a/content_scripts/mode.coffee
+++ b/content_scripts/mode.coffee
@@ -119,7 +119,7 @@ class Mode
       @unshift
         _name: "mode-#{@id}/suppressPrintableEvents"
         keypress: (event) =>
-          if DomUtils.isPrintable(event) and
+          if KeyboardUtils.isPrintable(event) and
             event.srcElement == @options.suppressPrintableEvents then @suppressEvent else @continueBubbling
 
     Mode.updateBadge() if @badge
diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee
index b907f22e..31bae8ec 100644
--- a/content_scripts/mode_insert.coffee
+++ b/content_scripts/mode_insert.coffee
@@ -121,7 +121,7 @@ new class ContentEditableTrap extends Mode
     element = document.getSelection()?.anchorNode?.parentElement
     return element?.isContentEditable and
              document.activeElement and
-             DomUtils. isPrintable event and
+             KeyboardUtils.isPrintable(event) and
              DomUtils.isDOMDescendant document.activeElement, element
 
 root = exports ? window
diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee
index fd2427c4..322188b3 100644
--- a/lib/dom_utils.coffee
+++ b/lib/dom_utils.coffee
@@ -212,8 +212,5 @@ DomUtils =
         @remove()
         false
 
-  isPrintable: (event) ->
-    not (event.metaKey or event.ctrlKey or event.altKey)
-
 root = exports ? window
 root.DomUtils = DomUtils
diff --git a/lib/keyboard_utils.coffee b/lib/keyboard_utils.coffee
index d2a843f9..30d99656 100644
--- a/lib/keyboard_utils.coffee
+++ b/lib/keyboard_utils.coffee
@@ -55,6 +55,12 @@ KeyboardUtils =
     # c-[ is mapped to ESC in Vim by default.
     (event.keyCode == @keyCodes.ESC) || (event.ctrlKey && @getKeyChar(event) == '[')
 
+  # 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
+    @getKeyChar(event)?.length == 1
+
 KeyboardUtils.init()
 
 root = exports ? window
-- 
cgit v1.2.3


From 80ad0bc3087a3bf00d61bdd6c9cf48e971e22480 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 10 Jan 2015 18:52:24 +0000
Subject: Modes; re-architect key suppression and passkeys.

---
 content_scripts/mode.coffee            | 33 ++++++++++++++++++++++++++-------
 content_scripts/mode_insert.coffee     | 28 ----------------------------
 content_scripts/mode_passkeys.coffee   |  4 ++--
 content_scripts/vimium_frontend.coffee |  2 ++
 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
-- 
cgit v1.2.3