Grab back focus...

- add new option "GrabBackFocus"
- use chrome.storage.sync.get() to get option value
- avoid race conditions on load
- fix tests

6 files changed, 54 insertions, 38 deletions

diff --git a/background_scripts/settings.coffee b/background_scripts/settings.coffee
index f43bd4bc..4342aa26 100644
--- a/background_scripts/settings.coffee
+++ b/background_scripts/settings.coffee
@@ -73,6 +73,7 @@ root.Settings = Settings =
     linkHintNumbers: "0123456789"
     filterLinkHints: false
     hideHud: false
+    grabBackfocus: false
     userDefinedLinkHintCss:
       """
       div > .vimiumHintMarker {
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index ddc19d3a..3cbf2d53 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -98,28 +98,27 @@ frameId = Math.floor(Math.random()*999999999)
 
 # If an input grabs the focus before the user has interacted with the page, then grab it back.
 class GrabBackFocus extends Mode
-  constructor: (@insertMode) ->
-    return if @shouldBeDeactivated()
-    super name: "grab-focus", keydown: => @alwaysContinueBubbling => @exit()
+  constructor: ->
+    super
+      name: "grab-back-focus"
+      keydown: => @alwaysContinueBubbling => @exit()
 
     @push
-      _name: "grab-focus-handlers"
+      _name: "grab-back-focus-mousedown"
       mousedown: => @alwaysContinueBubbling => @exit()
-      focus: (event) => @grabBackFocus event.target
 
-    # An input may already be focused. If so, grab back the focus.
-    @grabBackFocus document.activeElement if document.activeElement
+    chrome.storage.sync.get "grabBackfocus", (items) =>
+      return @exit() unless items.grabBackfocus and not chrome.runtime.lastError
+      @push
+        _name: "grab-back-focus-focus"
+        focus: (event) => @grabBackFocus event.target
+      # An input may already be focused. If so, grab back the focus.
+      @grabBackFocus document.activeElement if document.activeElement
 
   grabBackFocus: (element) ->
-    if DomUtils.isEditable(element) and not @shouldBeDeactivated()
-      element.blur()
-      @insertMode.exit null, element
-      return @suppressEvent
-    @exit() if @shouldBeDeactivated()
-    @continueBubbling
-
-  shouldBeDeactivated: ->
-    false and settings.isLoaded and not settings.get "grabBackFocus"
+    return @continueBubbling unless DomUtils.isEditable element
+    element.blur()
+    @suppressEvent
 
 # Only exported for tests.
 window.initializeModes = ->
@@ -139,7 +138,7 @@ window.initializeModes = ->
   new NormalMode
   new PassKeysMode
   new InsertMode permanent: true
-  new GrabBackFocus InsertMode.permanentInstance
+  new GrabBackFocus
 
 #
 # Complete initialization work that sould be done prior to DOMReady.
diff --git a/pages/options.coffee b/pages/options.coffee
index 93c9b503..525508fd 100644
--- a/pages/options.coffee
+++ b/pages/options.coffee
@@ -257,6 +257,7 @@ initOptionsPage = ->
     regexFindMode: CheckBoxOption
     scrollStepSize: NumberOption
     smoothScroll: CheckBoxOption
+    grabBackfocus: CheckBoxOption
     searchEngines: TextOption
     searchUrl: NonEmptyTextOption
     userDefinedLinkHintCss: TextOption
diff --git a/pages/options.html b/pages/options.html
index d37646c4..6df2c92b 100644
--- a/pages/options.html
+++ b/pages/options.html
@@ -133,6 +133,20 @@ b: http://b.com/?q=%s description
             <td verticalAlign="top" class="booleanOption">
               <div class="help">
                 <div class="example">
+                  Prevent the page from focusing an input on load
+                </div>
+              </div>
+              <label>
+                <input id="grabBackfocus" type="checkbox"/>
+                Grab back focus
+              </label>
+            </td>
+          </tr>
+          <tr>
+            <td class="caption"></td>
+            <td verticalAlign="top" class="booleanOption">
+              <div class="help">
+                <div class="example">
                   When enabled, the HUD will not be displayed.
                 </div>
               </div>
diff --git a/tests/dom_tests/chrome.coffee b/tests/dom_tests/chrome.coffee
index 2e7c6a5a..d6c03fc1 100644
--- a/tests/dom_tests/chrome.coffee
+++ b/tests/dom_tests/chrome.coffee
@@ -7,26 +7,23 @@ root.chromeMessages = []
 
 document.hasFocus = -> true
 
-root.chrome = {
-  runtime: {
-    connect: -> {
-      onMessage: {
+root.chrome =
+  runtime:
+    connect: ->
+      onMessage:
         addListener: ->
-      }
-      onDisconnect: {
+      onDisconnect:
         addListener: ->
-      }
       postMessage: ->
-    }
-    onMessage: {
+    onMessage:
       addListener: ->
-    }
     sendMessage: (message) -> chromeMessages.unshift message
     getManifest: ->
     getURL: (url) -> "../../#{url}"
-  }
   storage:
     local:
       get: ->
       set: ->
-}
+    sync:
+      get: ->
+      set: ->
diff --git a/tests/dom_tests/dom_tests.coffee b/tests/dom_tests/dom_tests.coffee
index 11fbe11f..a1ca8723 100644
--- a/tests/dom_tests/dom_tests.coffee
+++ b/tests/dom_tests/dom_tests.coffee
@@ -354,24 +354,24 @@ context "Triggering insert mode",
     document.getElementById("test-div").innerHTML = ""
 
   should "trigger insert mode on focus of text input", ->
-    assert.isTrue Mode.top().name == "insert" and not Mode.top().isActive()
+    assert.isFalse InsertMode.permanentInstance.isActive()
     document.getElementById("first").focus()
-    assert.isTrue Mode.top().name == "insert" and Mode.top().isActive()
+    assert.isTrue InsertMode.permanentInstance.isActive()
 
   should "trigger insert mode on focus of password input", ->
-    assert.isTrue Mode.top().name == "insert" and not Mode.top().isActive()
+    assert.isFalse InsertMode.permanentInstance.isActive()
     document.getElementById("third").focus()
-    assert.isTrue Mode.top().name == "insert" and Mode.top().isActive()
+    assert.isTrue InsertMode.permanentInstance.isActive()
 
   should "trigger insert mode on focus of contentEditable elements", ->
-    assert.isTrue Mode.top().name == "insert" and not Mode.top().isActive()
+    assert.isFalse InsertMode.permanentInstance.isActive()
     document.getElementById("fourth").focus()
-    assert.isTrue Mode.top().name == "insert" and Mode.top().isActive()
+    assert.isTrue InsertMode.permanentInstance.isActive()
 
   should "not trigger insert mode on other elements", ->
-    assert.isTrue Mode.top().name == "insert" and not Mode.top().isActive()
+    assert.isFalse InsertMode.permanentInstance.isActive()
     document.getElementById("fifth").focus()
-    assert.isTrue Mode.top().name == "insert" and not Mode.top().isActive()
+    assert.isFalse InsertMode.permanentInstance.isActive()
 
 context "Mode utilities",
   setup ->
@@ -454,6 +454,10 @@ context "PostFindMode",
     testContent = "<input type='text' id='first'/>"
     document.getElementById("test-div").innerHTML = testContent
     document.getElementById("first").focus()
+    # For these tests, we need to push GrabBackFocus out of the way.  When it exits, it updates the badge,
+    # which interferes with event suppression within insert mode.  This cannot happen in normal operation,
+    # because GrabBackFocus exits on the first keydown.
+    Mode.top().exit()
     @postFindMode = new PostFindMode
 
   tearDown ->
@@ -466,7 +470,7 @@ context "PostFindMode",
 
   should "suppress unmapped printable keys", ->
     sendKeyboardEvent "m"
-    assert.equal pageKeyboardEventCount, 0
+    assert.equal 0, pageKeyboardEventCount
 
   should "be deactivated on click events", ->
     handlerStack.bubbleEvent "click", target: document.activeElement