Modes; revise InsertMode as two classes.

5 files changed, 149 insertions, 169 deletions

diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee
index 76b65a12..96fc9b0c 100644
--- a/content_scripts/mode.coffee
+++ b/content_scripts/mode.coffee
@@ -61,6 +61,7 @@ class Mode
   constructor: (options={}) ->
     Mode.modes.unshift @
     extend @, options
+    @modeIsActive = true
     @count = ++count
     console.log @count, "create:", @name
 
@@ -75,12 +76,14 @@ class Mode
     @handlers.push handlerStack.push handlers
 
   exit: ->
-    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()
-    Mode.modes = Mode.modes.filter (mode) => mode != @
-    Mode.updateBadge()
+    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()
+      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
@@ -122,9 +125,9 @@ class SingletonMode extends Mode
     SingletonMode.instances[singleton].exit() if SingletonMode.instances[singleton]
 
 # The mode exits when the user hits Esc.
-class ExitOnEscapeMode extends Mode
-  constructor: (options) ->
-    super options
+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.
     @push
@@ -135,23 +138,17 @@ class ExitOnEscapeMode extends Mode
           event: event
         @suppressEvent
 
-# When the user clicks anywhere outside of the given element, the mode is exited.
+# When @element loses the focus.
 class ConstrainedMode extends ExitOnEscapeMode
-  constructor: (@element, options) ->
-    options.name = if options.name? then "constrained-#{options.name}" else "constrained"
-    super options
-
-    @push
-      "click": (event) =>
-        @exit() unless @isDOMDescendant @element, event.srcElement
-        @continueBubbling
-
-  isDOMDescendant: (parent, child) ->
-    node = child
-    while (node != null)
-      return true if (node == parent)
-      node = node.parentNode
-    false
+  constructor: (@element, singleton, options) ->
+    super singleton, options
+
+    if @element
+      @element.focus()
+      @push
+        "blur": (event) =>
+          handlerStack.alwaysContinueBubbling =>
+            @exit() if event.srcElement == @element
 
 # The state mode tracks the enabled state in @enabled and @passKeys, and its initialized state in
 # @initialized.  It calls @registerStateChange() whenever the state changes.
diff --git a/content_scripts/mode_find.coffee b/content_scripts/mode_find.coffee
index d6d1ff33..795e7a14 100644
--- a/content_scripts/mode_find.coffee
+++ b/content_scripts/mode_find.coffee
@@ -1,37 +1,35 @@
 # 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.  Subsequent keyboard
-# events could drop us into insert mode, which is a bad user experience.  The PostFindMode mode is installed
-# after find events to prevent this.
+# 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.
+#   2. If the very-next keystroke is Escape, then drop immediately into insert mode.
 #
-# PostFindMode also maps Esc (on the next keystroke) to immediately drop into insert mode.
-class PostFindMode extends SingletonMode
-  constructor: (insertMode, findModeAnchorNode) ->
+class PostFindMode extends InsertModeBlocker
+  constructor: (findModeAnchorNode) ->
     element = document.activeElement
-    return unless element
+
+    super PostFindMode, element,
+      name: "post-find"
+
+    return @exit() unless element and findModeAnchorNode
 
     # Special cases only arise if the active element is focusable.  So, exit immediately if it is not.
     canTakeInput = DomUtils.isSelectable(element) and DomUtils.isDOMDescendant findModeAnchorNode, element
-    canTakeInput ||= element?.isContentEditable
-    return unless canTakeInput
-
-    super PostFindMode,
-      name: "post-find"
+    canTakeInput ||= element.isContentEditable
+    return @exit() unless canTakeInput
 
-    # If the very next key is Esc, then drop straight into insert mode.
+    self = @
     @push
       keydown: (event) ->
-        @remove()
         if element == document.activeElement and KeyboardUtils.isEscape event
-          PostFindMode.exitModeAndEnterInsert insertMode, element
+          self.exit()
+          new InsertMode element
           return false
+        @remove()
         true
 
-    if element.isContentEditable
-      # Prevent InsertMode from activating on keydown.
-      @push
-        keydown: (event) -> handlerStack.alwaysContinueBubbling -> InsertMode.suppressKeydownTrigger event
-
     # Install various ways in which we can leave this mode.
     @push
       DOMActive: (event) => handlerStack.alwaysContinueBubbling => @exit()
@@ -40,15 +38,5 @@ class PostFindMode extends SingletonMode
       blur: (event) => handlerStack.alwaysContinueBubbling => @exit()
       keydown: (event) => handlerStack.alwaysContinueBubbling => @exit() if document.activeElement != element
 
-  # There's feature interference between PostFindMode, InsertMode and focusInput.  PostFindMode prevents
-  # InsertMode from triggering on keyboard events.  And FindMode prevents InsertMode from triggering on focus
-  # events.  This means that an input element can already be focused, but InsertMode is not active.  When that
-  # element is then (again) focused by focusInput, no new focus event is generated, so we don't drop into
-  # InsertMode as expected.
-  # This hack fixes this.
-  @exitModeAndEnterInsert: (insertMode, element) ->
-    SingletonMode.kill PostFindMode
-    insertMode.activate insertMode, element
-
 root = exports ? window
 root.PostFindMode = PostFindMode
diff --git a/content_scripts/mode_insert.coffee b/content_scripts/mode_insert.coffee
index 5a0ac9eb..32994aef 100644
--- a/content_scripts/mode_insert.coffee
+++ b/content_scripts/mode_insert.coffee
@@ -1,107 +1,102 @@
 
-class InsertMode extends Mode
-  insertModeActive: false
-  insertModeLock: null
-
-  # Input or text elements are considered focusable and able to receieve their own keyboard events, and will
-  # enter insert mode if focused. Also note that the "contentEditable" attribute can be set on any element
-  # which makes it a rich text editor, like the notes on jjot.com.
-  isEditable: (element) ->
-    return true if element.isContentEditable
-    nodeName = element.nodeName?.toLowerCase()
-    # Use a blacklist instead of a whitelist because new form controls are still being implemented for html5.
-    if nodeName == "input" and element.type not in ["radio", "checkbox"]
-      return true
-    nodeName in ["textarea", "select"]
-
-  # Embedded elements like Flash and quicktime players can obtain focus.
-  isEmbed: (element) ->
-    element.nodeName?.toLowerCase() in ["embed", "object"]
-
-  isFocusable: (element) ->
-    @isEditable(element) or @isEmbed element
-
-  # Check whether insert mode is active.  Also, activate insert mode if the current element is content
-  # editable (and the event is not suppressed).
-  isActiveOrActivate: (event) ->
-    return true if @insertModeActive
-    return false if event.suppressKeydownTrigger
-    # 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 re-check
-    # whether the active element is contentEditable.
-    @activate() if document.activeElement?.isContentEditable
-    @insertModeActive
-
-  activate: (target=null) ->
-    unless @insertModeActive
-      @insertModeActive = true
-      @insertModeLock = target
-      @badge = "I"
-      Mode.updateBadge()
-
-  deactivate: ->
-    if @insertModeActive
-      @insertModeActive = false
-      @insertModeLock = null
-      @badge = ""
-      Mode.updateBadge()
-
-  exit: (event) ->
-    if event?.source == ExitOnEscapeMode
-      element = event?.event?.srcElement
-      if element? and @isFocusable element
+# Input or text elements are considered focusable and able to receieve their own keyboard events, and will
+# enter insert mode if focused. Also note that the "contentEditable" attribute can be set on any element
+# which makes it a rich text editor, like the notes on jjot.com.
+isEditable =(element) ->
+  return true if element.isContentEditable
+  nodeName = element.nodeName?.toLowerCase()
+  # Use a blacklist instead of a whitelist because new form controls are still being implemented for html5.
+  if nodeName == "input" and element.type not in ["radio", "checkbox"]
+    return true
+  nodeName in ["textarea", "select"]
+
+# Embedded elements like Flash and quicktime players can obtain focus.
+isEmbed =(element) ->
+  element.nodeName?.toLowerCase() in ["embed", "object"]
+
+isFocusable =(element) ->
+  isEditable(element) or isEmbed element
+
+class InsertMode extends ConstrainedMode
+
+  constructor: (@insertModeLock=null) ->
+    super @insertModeLock, InsertMode,
+      name: "insert"
+      badge: "I"
+      keydown: (event) => @stopBubblingAndTrue
+      keypress: (event) => @stopBubblingAndTrue
+      keyup: (event) => @stopBubblingAndTrue
+
+    @push
+      focus: (event, extra) =>
+        handlerStack.alwaysContinueBubbling =>
+          # Inform InsertModeTrigger that InsertMode is already active.
+          extra.insertModeActive = true
+
+    Mode.updateBadge()
+
+  exit: (event=null) ->
+    if event?.source == ExitOnEscapeMode and event?.event?.srcElement?
+      element = event.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.
         # NOTE(smblott, 2014/12/22) Including embeds for .blur() here is experimental.  It appears to be the
         # right thing to do for most common use cases.  However, it could also cripple flash-based sites and
         # games.  See discussion in #1211 and #1194.
         element.blur()
-    @deactivate()
+    super()
 
+# Trigger insert mode:
+#   - On keydown event in a contentEditable element.
+#   - When a focusable element receives the focus.
+# Can be suppressed by setting extra.suppressInsertModeTrigger.
+class InsertModeTrigger extends Mode
   constructor: ->
     super
-      name: "insert"
-      keydown: (event) =>
-        return @continueBubbling unless @isActiveOrActivate event
-        return @stopBubblingAndTrue unless KeyboardUtils.isEscape event
-        # We're in insert mode, and now exiting.
-        if event.srcElement? and @isFocusable event.srcElement
-          # Remove the focus so the user can't just get himself back into insert mode by typing in the same
-          # input box.
-          # NOTE(smblott, 2014/12/22) Including embeds for .blur() here is experimental.  It appears to be the
-          # right thing to do for most common use cases.  However, it could also cripple flash-based sites and
-          # games.  See discussion in #1211 and #1194.
-          event.srcElement.blur()
-        @deactivate()
-        @suppressEvent
-      keypress: => if @insertModeActive then @stopBubblingAndTrue else @continueBubbling
-      keyup: => if @insertModeActive then @stopBubblingAndTrue else @continueBubbling
+      name: "insert-trigger"
+      keydown: (event, extra) =>
+        handlerStack.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.
+            new InsertMode() if document.activeElement?.isContentEditable
 
     @push
-      focus: (event) =>
-        handlerStack.alwaysContinueBubbling =>
-          if not @insertModeActive and @isFocusable event.target
-            @activate event.target
-      blur: (event) =>
+      focus: (event, extra) =>
         handlerStack.alwaysContinueBubbling =>
-          if @insertModeActive and event.target == @insertModeLock
-            @deactivate()
+          unless extra.suppressInsertModeTrigger?
+            new InsertMode event.target if isFocusable event.target
 
-    # We may already have focussed something, so check, so check.
-    @activate document.activeElement if document.activeElement and @isFocusable document.activeElement
+    # We may already have focussed something, so check.
+    new InsertMode document.activeElement if document.activeElement and isFocusable document.activeElement
 
-  # Used to prevent keydown events from triggering insert mode (following find).
-  # FIXME(smblott)  This is a hack.
-  @suppressKeydownTrigger: (event) ->
-    event.suppressKeydownTrigger = true
+  @suppress: (extra) ->
+    extra.suppressInsertModeTrigger = true
 
-# Activate this mode to prevent a focused, editable element from triggering insert mode.
-class InsertModeSuppressFocusTrigger extends Mode
-  constructor: ->
-    super {name: "suppress-insert-mode-focus-trigger"}
-    @push
-      focus: => @suppressEvent
+# 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.
+class InsertModeBlocker extends SingletonMode
+  constructor: (singleton=InsertModeBlocker, @element=null, options={}) ->
+    options.name ||= "insert-blocker"
+    super singleton, options
+
+    unless @element?
+      @push
+        focus: (event, extra) =>
+          handlerStack.alwaysContinueBubbling =>
+            InsertModeTrigger.suppress extra
+
+    if @element?.isContentEditable
+      @push
+        keydown: (event, extra) =>
+          handlerStack.alwaysContinueBubbling =>
+            InsertModeTrigger.suppress extra if event.srcElement == @element
 
 root = exports ? window
 root.InsertMode = InsertMode
-root.InsertModeSuppressFocusTrigger = InsertModeSuppressFocusTrigger
+root.InsertModeTrigger = InsertModeTrigger
+root.InsertModeBlocker = InsertModeBlocker
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index 75b4172f..299cdcf2 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -134,7 +134,7 @@ initializePreDomReady = ->
 
   # Install passKeys and insert modes.  These too are permanently on the stack (although not always active).
   passKeysMode = new PassKeysMode()
-  insertMode = new InsertMode()
+  new InsertModeTrigger()
   Mode.updateBadge()
 
   checkIfEnabledForUrl()
@@ -339,12 +339,14 @@ extend window,
     HUD.showForDuration("Yanked URL", 1000)
 
   enterInsertMode: ->
-    insertMode?.activate()
+    new InsertMode()
 
   enterVisualMode: =>
     new VisualMode()
 
   focusInput: (count) ->
+    SingletonMode.kill PostFindMode
+
     # Focus the first input element on the page, and create overlays to highlight all the input elements, with
     # the currently-focused element highlighted specially. Tabbing will shift focus to the next input element.
     # Pressing any other key will remove the overlays and the special tab behavior.
@@ -360,10 +362,14 @@ extend window,
 
     selectedInputIndex = Math.min(count - 1, visibleInputs.length - 1)
 
-    # See the definition of PostFindMode.exitModeAndEnterInsert for an explanation of why this is needed.
-    PostFindMode.exitModeAndEnterInsert insertMode, visibleInputs[selectedInputIndex].element
-
-    visibleInputs[selectedInputIndex].element.focus()
+    # There's feature interference between PostFindMode, InsertMode and focusInput.  PostFindMode prevents
+    # InsertMode from triggering on focus events.  Therefore, an input element can already be focused, but
+    # InsertMode is not active.  When that element is then (again) focused by focusInput, below, no new focus
+    # event is generated, so we don't drop into InsertMode as expected.
+    # Therefore we blur() the element before focussing it.
+    element = visibleInputs[selectedInputIndex].element
+    element.blur() if document.activeElement == element
+    element.focus()
 
     return if visibleInputs.length == 1
 
@@ -730,14 +736,12 @@ handleEnterForFindMode = ->
   focusFoundLink()
   document.body.classList.add("vimiumFindMode")
   settings.set("findModeRawQuery", findModeQuery.rawQuery)
-  # If we have found an input element, the pressing <esc> immediately afterwards sends us into insert mode.
-  new PostFindMode insertMode, findModeAnchorNode
 
 class FindMode extends ExitOnEscapeMode
-  constructor: (badge="F") ->
-    super
+  constructor: ->
+    super FindMode,
       name: "find"
-      badge: badge
+      badge: "/"
 
       keydown: (event) =>
         if event.keyCode == keyCodes.backspace || event.keyCode == keyCodes.deleteKey
@@ -761,9 +765,10 @@ class FindMode extends ExitOnEscapeMode
 
     Mode.updateBadge()
 
-  exit: (event) ->
-    handleEscapeForFindMode() if event?.source == ExitOnEscapeMode
+  exit: (extra) ->
+    handleEscapeForFindMode() if extra?.source == ExitOnEscapeMode
     super()
+    new PostFindMode findModeAnchorNode
 
 performFindInPlace = ->
   cachedScrollX = window.scrollX
@@ -792,7 +797,7 @@ executeFind = (query, options) ->
   HUD.hide(true)
   # ignore the selectionchange event generated by find()
   document.removeEventListener("selectionchange",restoreDefaultSelectionHighlight, true)
-  Mode.runIn InsertModeSuppressFocusTrigger, ->
+  Mode.runIn InsertModeBlocker, ->
     result = window.find(query, options.caseSensitive, options.backwards, true, false, true, false)
   setTimeout(
     -> document.addEventListener("selectionchange", restoreDefaultSelectionHighlight, true)
@@ -854,7 +859,7 @@ findAndFocus = (backwards) ->
 
   # if we have found an input element via 'n', pressing <esc> immediately afterwards sends us into insert
   # mode
-  new PostFindMode insertMode, findModeAnchorNode
+  new PostFindMode findModeAnchorNode
 
   focusFoundLink()
 
diff --git a/lib/handler_stack.coffee b/lib/handler_stack.coffee
index 17e4844b..0a34087f 100644
--- a/lib/handler_stack.coffee
+++ b/lib/handler_stack.coffee
@@ -40,17 +40,12 @@ class HandlerStack
     true
 
   remove: (id = @currentId) ->
-    if 0 < @stack.length and @stack[@stack.length-1].id == id
-      # A common case is to remove the handler at the top of the stack.  And we can do this very efficiently.
-      # Tests suggest that this case arises more than half of the time.
-      @stack.pop().id = null
-    else
-      # Otherwise, we'll build a new stack.  This is better than splicing the existing stack since that can
-      # interfere with concurrent bubbleEvents.
-      @stack = @stack.filter (handler) ->
-        # Mark this handler as removed (for any active bubbleEvent call).
-        handler.id = null if handler.id == id
-        handler?.id?
+    # This is more expense than splicing @stack, but better because splicing can interfere with concurrent
+    # bubbleEvents.
+    @stack = @stack.filter (handler) ->
+      # Mark this handler as removed (to notify any concurrent bubbleEvent call).
+      if handler.id == id then handler.id = null
+      handler?.id?
 
   # The handler stack handles chrome events (which may need to be suppressed) and internal (fake) events.
   # This checks whether that the event at hand is a chrome event.