Modes; more minor tweeks.

5 files changed, 80 insertions, 106 deletions

diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee
index e5795190..adc5439d 100644
--- a/content_scripts/mode.coffee
+++ b/content_scripts/mode.coffee
@@ -1,15 +1,16 @@
 # Modes.
 #
-# A mode implements a number of event handlers which are pushed onto the handler stack when the mode starts,
-# and poped when the mode exits.  The Mode base takes as single argument options which can defined:
+# A mode implements a number of keyboard event handlers which are pushed onto the handler stack when the mode
+# starts, and poped when the mode exits.  The Mode base class takes as single argument options which can
+# define:
 #
 # name:
 #   A name for this mode.
 #
 # badge:
 #   A badge (to appear on the browser popup) for this mode.
-#   Optional.  Define a badge is the badge is constant.  Otherwise, do not set a badge and override the
-#   chooseBadge method instead.  Or, if the mode *never* shows a bade, then do neither.
+#   Optional.  Define a badge is the badge is constant.  Otherwise, do not define a badge and override the
+#   chooseBadge method instead.  Or, if the mode *never* shows a badge, then do neither.
 #
 # keydown:
 # keypress:
@@ -24,9 +25,10 @@
 #
 # New mode types are created by inheriting from Mode or one of its sub-classes.  Some generic cub-classes are
 # provided below:
-#   SingletonMode: ensures that at most one instance of the mode should be active at any time.
-#   ConstrainedMode: exits the mode if the user clicks outside of the given element.
-#   ExitOnEscapeMode: exits the mode if the user types Esc.
+#
+#   SingletonMode: ensures that at most one instance of the mode is active at any one time.
+#   ConstrainedMode: exits the mode if the an indicated element loses the focus.
+#   ExitOnEscapeMode: exits the mode on escape.
 #   StateMode: tracks the current Vimium state in @enabled and @passKeys.
 #
 # To install and existing mode, use:
@@ -37,13 +39,12 @@
 #   @exit()       # internally triggered (more common).
 #
 
-# Debug only; to be stripped out.
+# For debug only; to be stripped out.
 count = 0
 
 class Mode
-  # Static members.
+  # Static.
   @modes: []
-  @current: -> Mode.modes[0]
 
   # Constants; readable shortcuts for event-handler return values.
   continueBubbling: true
@@ -54,7 +55,7 @@ class Mode
   # Default values.
   name: ""
   badge: ""
-  keydown: null
+  keydown: null # null will be ignored by handlerStack (so it's a safe default).
   keypress: null
   keyup: null
 
@@ -70,7 +71,7 @@ class Mode
       keydown: @keydown
       keypress: @keypress
       keyup: @keyup
-      updateBadge: (badge) => handlerStack.alwaysContinueBubbling => @chooseBadge badge
+      updateBadge: (badge) => @alwaysContinueBubbling => @chooseBadge badge
 
     Mode.updateBadge() if @badge
 
@@ -80,21 +81,22 @@ class Mode
   exit: ->
     if @modeIsActive
       console.log @count, "exit:", @name
-      # We reverse @handlers, here.  That way, handlers are popped in the opposite order to that in which they
-      # were pushed.
-      handlerStack.remove handlerId for handlerId in @handlers.reverse()
+      handlerStack.remove handlerId for handlerId in @handlers
       Mode.modes = Mode.modes.filter (mode) => mode != @
       Mode.updateBadge()
       @modeIsActive = false
 
   # The badge is chosen by bubbling an "updateBadge" event down the handler stack allowing each mode the
-  # opportunity to choose a badge.  chooseBadge, here, is the default: choose the current mode's badge unless
-  # one has already been chosen.  This is overridden in sub-classes.
+  # opportunity to choose a badge.  chooseBadge, here, is the default. It is overridden in sub-classes.
   chooseBadge: (badge) ->
     badge.badge ||= @badge
 
+  # Shorthand for a long name.
+  alwaysContinueBubbling: (func) -> handlerStack.alwaysContinueBubbling func
+
   # Static method.  Used externally and internally to initiate bubbling of an updateBadge event and to send
-  # the resulting badge to the background page.  We only update the badge if this document has the focus.
+  # the resulting badge to the background page.  We only update the badge if this document (hence this frame)
+  # has the focus.
   @updateBadge: ->
     if document.hasFocus()
       handlerStack.bubbleEvent "updateBadge", badge = {badge: ""}
@@ -102,36 +104,37 @@ class Mode
         handler: "setBadge"
         badge: badge.badge
 
-  # Temporarily install a mode.
+  # Temporarily install a mode to call a function.
   @runIn: (mode, func) ->
     mode = new mode()
     func()
     mode.exit()
 
 # A SingletonMode is a Mode of which there may be at most one instance (of @singleton) active at any one time.
-# New instances cancel previous instances on startup.
+# New instances cancel previously-active instances on startup.
 class SingletonMode extends Mode
   @instances: {}
 
   exit: ->
-    delete SingletonMode.instances[@singleton]
+    delete SingletonMode.instances[@singleton] if @singleton?
     super()
 
   constructor: (@singleton, options={}) ->
-    SingletonMode.kill @singleton
-    SingletonMode.instances[@singleton] = @
+    if @singleton?
+      SingletonMode.kill @singleton
+      SingletonMode.instances[@singleton] = @
     super options
 
-  # Static method. If there's a singleton instance running, then kill it.
+  # Static method. If there's a singleton instance active, then kill it.
   @kill: (singleton) ->
     SingletonMode.instances[singleton].exit() if SingletonMode.instances[singleton]
 
-# The mode exits when the user hits Esc.
+# This mode exits when the user hits Esc.
 class ExitOnEscapeMode extends SingletonMode
   constructor: (singleton, options) ->
     super singleton, options
 
-    # This handler ends up above the mode's own key handlers on the handler stack, so it takes priority.
+    # NOTE. This handler ends up above the mode's own key handlers on the handler stack, so it takes priority.
     @push
       "keydown": (event) =>
         return @continueBubbling unless KeyboardUtils.isEscape event
@@ -140,7 +143,7 @@ class ExitOnEscapeMode extends SingletonMode
           event: event
         @suppressEvent
 
-# Exit mode when @constrainingElement (if defined) loses the focus.
+# This mode exits when @constrainingElement (if defined) loses the focus.
 class ConstrainedMode extends ExitOnEscapeMode
   constructor: (@constrainingElement, singleton, options) ->
     super singleton, options
@@ -148,24 +151,22 @@ class ConstrainedMode extends ExitOnEscapeMode
     if @constrainingElement
       @constrainingElement.focus()
       @push
-        "blur": (event) =>
-          handlerStack.alwaysContinueBubbling =>
-            @exit() if event.srcElement == @constrainingElement
+        "blur": (event) => @alwaysContinueBubbling =>
+          @exit() if event.srcElement == @constrainingElement
 
-# The state mode tracks the enabled state in @enabled and @passKeys, and its initialized state in
-# @initialized.  It calls @registerStateChange() whenever the state changes.
+# The state mode tracks the enabled state in @enabled and @passKeys.  It calls @registerStateChange() whenever
+# the state changes.  The state is distributed by bubbling a "registerStateChange" event down the handler
+# stack.
 class StateMode extends Mode
   constructor: (options) ->
-    @stateInitialized = false
     @enabled = false
     @passKeys = ""
     super options
 
     @push
       "registerStateChange": ({enabled: enabled, passKeys: passKeys}) =>
-        handlerStack.alwaysContinueBubbling =>
-          if enabled != @enabled or passKeys != @passKeys or not @stateInitialized
-            @stateInitialized = true
+        @alwaysContinueBubbling =>
+          if enabled != @enabled or passKeys != @passKeys
             @enabled = enabled
             @passKeys = passKeys
             @registerStateChange()
@@ -173,28 +174,24 @@ class StateMode extends Mode
   # Overridden by sub-classes.
   registerStateChange: ->
 
-# BadgeMode is a psuedo mode for managing 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.
-class BadgeMode extends StateMode
+# BadgeMode is a psuedo 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 choices of all other modes.
+new class BadgeMode extends StateMode
   constructor: (options) ->
-    options.name ||= "badge"
-    super options
+    super
+      name: "badge"
 
     @push
-      "focus": =>
-        handlerStack.alwaysContinueBubbling =>
-          Mode.updateBadge()
+      "focus": => @alwaysContinueBubbling => Mode.updateBadge()
 
   chooseBadge: (badge) ->
-    # If we're not enabled, then post an empty badge (so, no badge at all).
+    # If we're not enabled, then post an empty badge.
     badge.badge = "" unless @enabled
 
   registerStateChange: ->
     Mode.updateBadge()
 
-# Install a single BadgeMode instance.
-new BadgeMode {}
-
 root = exports ? window
 root.Mode = Mode
 root.SingletonMode = SingletonMode
diff --git a/content_scripts/mode_find.coffee b/content_scripts/mode_find.coffee
index 8b458770..0937c510 100644
--- a/content_scripts/mode_find.coffee
+++ b/content_scripts/mode_find.coffee
@@ -2,8 +2,8 @@
 
 # When we use find mode, the selection/focus can end up in a focusable/editable element.  In this situation,
 # PostFindMode handles two special cases:
-#   1. Suppress InsertModeTrigger.  This presents keyboard events from dropping us unintentionaly into insert
-#      mode.  Here, this is achieved by inheriting PostFindMode from InsertModeBlocker.
+#   1. Suppress InsertModeTrigger.  This prevents keyboard events from dropping us unintentionaly into insert
+#      mode.  Here, this is achieved by inheriting from InsertModeBlocker.
 #   2. If the very-next keystroke is Escape, then drop immediately into insert mode.
 #
 class PostFindMode extends InsertModeBlocker
@@ -31,11 +31,11 @@ class PostFindMode extends InsertModeBlocker
 
     # Install various ways in which we can leave this mode.
     @push
-      DOMActive: (event) => handlerStack.alwaysContinueBubbling => @exit()
-      click: (event) => handlerStack.alwaysContinueBubbling => @exit()
-      focus: (event) => handlerStack.alwaysContinueBubbling => @exit()
-      blur: (event) => handlerStack.alwaysContinueBubbling => @exit()
-      keydown: (event) => handlerStack.alwaysContinueBubbling => @exit() if document.activeElement != element
+      DOMActive: (event) => @alwaysContinueBubbling => @exit()
+      click: (event) => @alwaysContinueBubbling => @exit()
+      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 d289ed86..51ef3d8b 100644
--- a/content_scripts/mode_insert.coffee
+++ b/content_scripts/mode_insert.coffee
@@ -17,8 +17,8 @@ isEmbed =(element) ->
 isFocusable =(element) ->
   isEditable(element) or isEmbed element
 
+# This mode is installed when insert mode is active.
 class InsertMode extends ConstrainedMode
-
   constructor: (@insertModeLock=null) ->
     super @insertModeLock, InsertMode,
       name: "insert"
@@ -27,9 +27,9 @@ class InsertMode extends ConstrainedMode
       keypress: (event) => @stopBubblingAndTrue
       keyup: (event) => @stopBubblingAndTrue
 
-  exit: (event=null) ->
-    if event?.source == ExitOnEscapeMode and event?.event?.srcElement?
-      element = event.event.srcElement
+  exit: (extra=null) ->
+    if extra?.source == ExitOnEscapeMode and extra?.event?.srcElement?
+      element = extra.event.srcElement
       if isFocusable element
         # Remove the focus so the user can't just get himself back into insert mode by typing in the same
         # input box.
@@ -40,24 +40,26 @@ class InsertMode extends ConstrainedMode
     super()
 
 # Trigger insert mode:
-#   - On keydown event in a contentEditable element.
+#   - On a keydown event in a contentEditable element.
 #   - When a focusable element receives the focus.
 # Can be suppressed by setting extra.suppressInsertModeTrigger.
+#
+# This mode is permanently installed fairly low down on the handler stack.
 class InsertModeTrigger extends Mode
   constructor: ->
     super
       name: "insert-trigger"
       keydown: (event, extra) =>
-        handlerStack.alwaysContinueBubbling =>
+        @alwaysContinueBubbling =>
           unless extra.suppressInsertModeTrigger?
-            # Some sites (e.g. inbox.google.com) change the contentEditable attribute on the fly (see #1245); and
-            # unfortunately, isEditable() is called *before* the change is made.  Therefore, we need to check
-            # whether the active element is contentEditable.
+            # 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.
             new InsertMode() if document.activeElement?.isContentEditable
 
     @push
       focus: (event, extra) =>
-        handlerStack.alwaysContinueBubbling =>
+        @alwaysContinueBubbling =>
           unless extra.suppressInsertModeTrigger?
             new InsertMode event.target if isFocusable event.target
 
@@ -67,10 +69,9 @@ class InsertModeTrigger extends Mode
   @suppress: (extra) ->
     extra.suppressInsertModeTrigger = true
 
-# Disables InsertModeTrigger.  Used by find mode to prevent unintentionally dropping into insert mode on
-# focusable elements.
-# If @element is provided, then don't block focus events, and block keydown events only on the indicated
-# element.
+# Disables InsertModeTrigger.  Used by find mode and findFocus to prevent unintentionally dropping into insert
+# mode on focusable elements.
+# If @element is provided, then don't suppress focus events, and suppress keydown events only on @element.
 class InsertModeBlocker extends SingletonMode
   constructor: (singleton=InsertModeBlocker, @element=null, options={}) ->
     options.name ||= "insert-blocker"
@@ -79,13 +80,13 @@ class InsertModeBlocker extends SingletonMode
     unless @element?
       @push
         focus: (event, extra) =>
-          handlerStack.alwaysContinueBubbling =>
+          @alwaysContinueBubbling =>
             InsertModeTrigger.suppress extra
 
     if @element?.isContentEditable
       @push
         keydown: (event, extra) =>
-          handlerStack.alwaysContinueBubbling =>
+          @alwaysContinueBubbling =>
             InsertModeTrigger.suppress extra if event.srcElement == @element
 
 root = exports ? window
diff --git a/content_scripts/mode_visual.coffee b/content_scripts/mode_visual.coffee
index 6ef8ed4e..bc4e0901 100644
--- a/content_scripts/mode_visual.coffee
+++ b/content_scripts/mode_visual.coffee
@@ -1,20 +1,19 @@
 
 # Note. ConstrainedMode extends extends ExitOnEscapeMode.  So exit-on-escape is handled there.
 class VisualMode extends ConstrainedMode
-
   constructor: (element=document.body) ->
     super element, VisualMode,
       name: "visual"
       badge: "V"
 
       keydown: (event) =>
-        return Mode.suppressEvent
+        return @suppressEvent
 
       keypress: (event) =>
-        return Mode.suppressEvent
+        return @suppressEvent
 
       keyup: (event) =>
-        return Mode.suppressEvent
+        return @suppressEvent
 
 root = exports ? window
 root.VisualMode = VisualMode
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index fd55348d..82beb90c 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -359,12 +359,12 @@ extend window,
 
     selectedInputIndex = Math.min(count - 1, visibleInputs.length - 1)
 
-    element = visibleInputs[selectedInputIndex].element.focus()
+    # If PostFindMode is active, then the target element may already have the focus, in which case, .focus()
+    # will not generate a "focus" event to trigger insert mode.  So we handle insert mode manually, here.
+    Mode.runIn InsertModeBlocker, ->
+      visibleInputs[selectedInputIndex].element.focus()
 
     if visibleInputs.length == 1
-      # If PostFindMode is was previously active, then element may already have had the focus.  In this case,
-      # focus(), above, will not have generated a "focus" event to trigger insert mode.  So we force insert
-      # mode now.
       new InsertMode visibleInputs[selectedInputIndex].element
       return
 
@@ -385,7 +385,7 @@ extend window,
     hintContainingDiv = DomUtils.addElementList(hints,
       { id: "vimiumInputMarkerContainer", className: "vimiumReset" })
 
-    class FocusSelector extends InsertModeBlocker
+    new class FocusSelector extends InsertModeBlocker
       constructor: ->
         super InsertModeBlocker, null,
           name: "focus-selector"
@@ -393,39 +393,16 @@ extend window,
           keydown: (event) =>
             if event.keyCode == KeyboardUtils.keyCodes.tab
               hints[selectedInputIndex].classList.remove 'internalVimiumSelectedInputHint'
-              if event.shiftKey
-                if --selectedInputIndex == -1
-                  selectedInputIndex = hints.length - 1
-              else
-                if ++selectedInputIndex == hints.length
-                  selectedInputIndex = 0
+              selectedInputIndex += hints.length + (if event.shiftKey then -1 else 1)
+              selectedInputIndex %= hints.length
               hints[selectedInputIndex].classList.add 'internalVimiumSelectedInputHint'
               visibleInputs[selectedInputIndex].element.focus()
               false
             else unless event.keyCode == KeyboardUtils.keyCodes.shiftKey
-              DomUtils.removeElement hintContainingDiv
               @exit()
+              DomUtils.removeElement hintContainingDiv
               new InsertMode visibleInputs[selectedInputIndex].element
 
-    new FocusSelector()
-
-    # handlerStack.push keydown: (event) ->
-    #   if event.keyCode == KeyboardUtils.keyCodes.tab
-    #     hints[selectedInputIndex].classList.remove 'internalVimiumSelectedInputHint'
-    #     if event.shiftKey
-    #       if --selectedInputIndex == -1
-    #         selectedInputIndex = hints.length - 1
-    #     else
-    #       if ++selectedInputIndex == hints.length
-    #         selectedInputIndex = 0
-    #     hints[selectedInputIndex].classList.add 'internalVimiumSelectedInputHint'
-    #     visibleInputs[selectedInputIndex].element.focus()
-    #   else unless event.keyCode == KeyboardUtils.keyCodes.shiftKey
-    #     DomUtils.removeElement hintContainingDiv
-    #     @remove()
-    #     return true
-
-
 # Decide whether this keyChar 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.