From 0ea060ca5b45acf42eb892cf900b8b7e04fc3397 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 12 Feb 2015 09:15:30 +0000 Subject: Initial grab-back-focus. If an input grabs the focus before the user interacts with the page (keydown, mousedown), then grab back the focus. TBD: 1 This requires a new option, which has not yet been added. For now, it's just permanently enabled. 2 There's a race condition between the setting being loaded and the element being focused. Currently, this defaults to being activated. This is the wrong thing to do. The solution is probably to use chrome.storage for the setting. 3 The tests fail (because of TBD-2, above). --- content_scripts/vimium_frontend.coffee | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 050a271e..ddc19d3a 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -96,6 +96,31 @@ settings = # 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() + + @push + _name: "grab-focus-handlers" + 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 + + 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" + # Only exported for tests. window.initializeModes = -> class NormalMode extends Mode @@ -114,6 +139,7 @@ window.initializeModes = -> new NormalMode new PassKeysMode new InsertMode permanent: true + new GrabBackFocus InsertMode.permanentInstance # # Complete initialization work that sould be done prior to DOMReady. @@ -179,7 +205,7 @@ window.initializeWhenEnabled = -> unless installedListeners # Key event handlers fire on window before they do on document. Prefer window for key events so the page # can't set handlers to grab the keys before us. - for type in ["keydown", "keypress", "keyup", "click", "focus", "blur"] + for type in [ "keydown", "keypress", "keyup", "click", "focus", "blur", "mousedown" ] do (type) -> installListener window, type, (event) -> handlerStack.bubbleEvent type, event installListener document, "DOMActivate", (event) -> handlerStack.bubbleEvent 'DOMActivate', event installedListeners = true -- cgit v1.2.3 From ffc49d3057daee2354fb77d939fffc0cf77ff2e1 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Thu, 12 Feb 2015 10:52:56 +0000 Subject: Grab back focus... - add new option "GrabBackFocus" - use chrome.storage.sync.get() to get option value - avoid race conditions on load - fix tests --- background_scripts/settings.coffee | 1 + content_scripts/vimium_frontend.coffee | 33 ++++++++++++++++----------------- pages/options.coffee | 1 + pages/options.html | 14 ++++++++++++++ tests/dom_tests/chrome.coffee | 21 +++++++++------------ tests/dom_tests/dom_tests.coffee | 22 +++++++++++++--------- 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 @@ -128,6 +128,20 @@ b: http://b.com/?q=%s description + + + +
+
+ Prevent the page from focusing an input on load +
+
+ + + 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 = "" 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 -- cgit v1.2.3 From 358f1ebfddacf75c02bb3a2e8993bab6d2fe28d8 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Fri, 13 Feb 2015 06:53:51 +0000 Subject: Tweaks for grab-back-focus. - Fix incorrect camel case in option name. - Better text on options page. - Loud comments re. using option directly from chrome.storage.sync. --- background_scripts/settings.coffee | 5 ++++- content_scripts/vimium_frontend.coffee | 11 ++++++++--- pages/options.coffee | 2 +- pages/options.html | 6 +++--- 4 files changed, 16 insertions(+), 8 deletions(-) diff --git a/background_scripts/settings.coffee b/background_scripts/settings.coffee index 4342aa26..a60f2e09 100644 --- a/background_scripts/settings.coffee +++ b/background_scripts/settings.coffee @@ -73,7 +73,6 @@ root.Settings = Settings = linkHintNumbers: "0123456789" filterLinkHints: false hideHud: false - grabBackfocus: false userDefinedLinkHintCss: """ div > .vimiumHintMarker { @@ -117,6 +116,10 @@ root.Settings = Settings = settingsVersion: Utils.getCurrentVersion() + # NOTE. This setting is accessed directly via chrome.storage.sync in the front end. There, we assume that + # the default value is false. + grabBackFocus: false + # We use settingsVersion to coordinate any necessary schema changes. if Utils.compareVersions("1.42", Settings.get("settingsVersion")) != -1 diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 3cbf2d53..51b0695f 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -96,7 +96,8 @@ settings = # frameId = Math.floor(Math.random()*999999999) -# If an input grabs the focus before the user has interacted with the page, then grab it back. +# If an input grabs the focus before the user has interacted with the page, then grab it back (if the +# grabBackFocus option is set). class GrabBackFocus extends Mode constructor: -> super @@ -107,8 +108,12 @@ class GrabBackFocus extends Mode _name: "grab-back-focus-mousedown" mousedown: => @alwaysContinueBubbling => @exit() - chrome.storage.sync.get "grabBackfocus", (items) => - return @exit() unless items.grabBackfocus and not chrome.runtime.lastError + # HACK. We use chrome.storage.sync directly here (rather than settings). This avoids a race condition. + # An input can be focused by the page either before we install our handlers or after, and we handle both + # cases. There's no uncertainty period while we wait to learn whether the option is set or not. + # Note. We also assume that the default value for grabBackFocus is false. + chrome.storage.sync.get "grabBackFocus", (items) => + return @exit() if chrome.runtime.lastError or not items.grabBackFocus @push _name: "grab-back-focus-focus" focus: (event) => @grabBackFocus event.target diff --git a/pages/options.coffee b/pages/options.coffee index 525508fd..d2950348 100644 --- a/pages/options.coffee +++ b/pages/options.coffee @@ -257,7 +257,7 @@ initOptionsPage = -> regexFindMode: CheckBoxOption scrollStepSize: NumberOption smoothScroll: CheckBoxOption - grabBackfocus: CheckBoxOption + grabBackFocus: CheckBoxOption searchEngines: TextOption searchUrl: NonEmptyTextOption userDefinedLinkHintCss: TextOption diff --git a/pages/options.html b/pages/options.html index 6df2c92b..889d5ea0 100644 --- a/pages/options.html +++ b/pages/options.html @@ -133,12 +133,12 @@ b: http://b.com/?q=%s description
- Prevent the page from focusing an input on load + Prevent pages from focusing an input on load (e.g. Google, Bing, etc.).
-- cgit v1.2.3 From 05f229201a05101ab4947bd436ec02d8864392f9 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sat, 14 Feb 2015 12:19:22 +0000 Subject: Grab back focus: use settings.addEventListener "load". Doh! I didn't know this existed. --- background_scripts/settings.coffee | 5 +---- content_scripts/vimium_frontend.coffee | 12 +++++------- 2 files changed, 6 insertions(+), 11 deletions(-) diff --git a/background_scripts/settings.coffee b/background_scripts/settings.coffee index a60f2e09..3528e8a9 100644 --- a/background_scripts/settings.coffee +++ b/background_scripts/settings.coffee @@ -113,13 +113,10 @@ root.Settings = Settings = # put in an example search engine searchEngines: "w: http://www.wikipedia.org/w/index.php?title=Special:Search&search=%s wikipedia" newTabUrl: "chrome://newtab" + grabBackFocus: false settingsVersion: Utils.getCurrentVersion() - # NOTE. This setting is accessed directly via chrome.storage.sync in the front end. There, we assume that - # the default value is false. - grabBackFocus: false - # We use settingsVersion to coordinate any necessary schema changes. if Utils.compareVersions("1.42", Settings.get("settingsVersion")) != -1 diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 51b0695f..741b54af 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -44,7 +44,7 @@ settings = loadedValues: 0 valuesToLoad: [ "scrollStepSize", "linkHintCharacters", "linkHintNumbers", "filterLinkHints", "hideHud", "previousPatterns", "nextPatterns", "regexFindMode", "userDefinedLinkHintCss", - "helpDialog_showAdvancedCommands", "smoothScroll" ] + "helpDialog_showAdvancedCommands", "smoothScroll", "grabBackFocus" ] isLoaded: false eventListeners: {} @@ -108,18 +108,16 @@ class GrabBackFocus extends Mode _name: "grab-back-focus-mousedown" mousedown: => @alwaysContinueBubbling => @exit() - # HACK. We use chrome.storage.sync directly here (rather than settings). This avoids a race condition. - # An input can be focused by the page either before we install our handlers or after, and we handle both - # cases. There's no uncertainty period while we wait to learn whether the option is set or not. - # Note. We also assume that the default value for grabBackFocus is false. - chrome.storage.sync.get "grabBackFocus", (items) => - return @exit() if chrome.runtime.lastError or not items.grabBackFocus + activate = => + return @exit() unless settings.get "grabBackFocus" @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 + if settings.isLoaded then activate() else settings.addEventListener "load", activate + grabBackFocus: (element) -> return @continueBubbling unless DomUtils.isEditable element element.blur() -- cgit v1.2.3