Merge pull request #1580 from smblott-github/mrmr1993-exclusion-rules-pushState-and-hash

Update exclusion rules when the URL is changed by history.pushState/popState or its hash changes (v2)

7 files changed, 42 insertions, 26 deletions

diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee
index 642913a5..f11b3b4a 100644
--- a/background_scripts/main.coffee
+++ b/background_scripts/main.coffee
@@ -89,14 +89,20 @@ getCurrentTabUrl = (request, sender) -> sender.tab.url
 # Checks the user's preferences in local storage to determine if Vimium is enabled for the given URL, and
 # whether any keys should be passed through to the underlying page.
 #
-root.isEnabledForUrl = isEnabledForUrl = (request, sender) ->
+root.isEnabledForUrl = isEnabledForUrl = (request) ->
   rule = Exclusions.getRule(request.url)
   {
     isEnabledForUrl: not rule or rule.passKeys
     passKeys: rule?.passKeys or ""
-    incognito: sender.tab.incognito
   }
 
+onURLChange = (details) ->
+  chrome.tabs.sendMessage details.tabId, name: "checkEnabledAfterURLChange"
+
+# Re-check whether Vimium is enabled for a frame when the url changes without a reload.
+chrome.webNavigation.onHistoryStateUpdated.addListener onURLChange # history.pushState.
+chrome.webNavigation.onReferenceFragmentUpdated.addListener onURLChange # Hash changed.
+
 # Retrieves the help dialog HTML template from a file, and populates it with the latest keybindings.
 # This is called by options.coffee.
 root.helpDialogHtml = (showUnboundCommands, showCommandNames, customTitle) ->
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index ec39ccde..2b90fe95 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -12,7 +12,7 @@ findModeInitialRange = null
 isShowingHelpDialog = false
 keyPort = null
 isEnabledForUrl = true
-isIncognitoMode = false
+isIncognitoMode = chrome.extension.inIncognitoContext
 passKeys = null
 keyQueue = null
 # The user's operating system.
@@ -187,8 +187,9 @@ initializePreDomReady = ->
     currentKeyQueue: (request) ->
       keyQueue = request.keyQueue
       handlerStack.bubbleEvent "registerKeyQueue", { keyQueue: keyQueue }
-    # A frame has received the focus.  We don't care, here (the Vomnibar/UI-component handles this).
+    # A frame has received the focus.  We don't care here (the Vomnibar/UI-component handles this).
     frameFocused: ->
+    checkEnabledAfterURLChange: checkEnabledAfterURLChange
 
   chrome.runtime.onMessage.addListener (request, sender, sendResponse) ->
     # In the options page, we will receive requests from both content and background scripts. ignore those
@@ -196,10 +197,11 @@ initializePreDomReady = ->
     return if sender.tab and not sender.tab.url.startsWith 'chrome-extension://'
     # These requests are delivered to the options page, but there are no handlers there.
     return if request.handler in [ "registerFrame", "frameFocused", "unregisterFrame" ]
-    # We handle the message if we're enabled, or if it's one of these listed message types.
-    shouldHandleRequest = isEnabledForUrl or request.name in [ "executePageCommand" ]
-    # Requests with a frameId of zero should only be handled in the main/top frame (regardless of whether
-    # Vimium is enabled there).
+    shouldHandleRequest = isEnabledForUrl
+    # We always handle the message if it's one of these listed message types.
+    shouldHandleRequest ||= request.name in [ "executePageCommand", "checkEnabledAfterURLChange" ]
+    # Requests with a frameId of zero should always and only be handled in the main/top frame (regardless of
+    # whether Vimium is enabled there).
     if request.frameId == 0 and DomUtils.isTopFrame()
       request.frameId = frameId
       shouldHandleRequest = true
@@ -217,9 +219,11 @@ installListener = (element, event, callback) ->
 # Installing or uninstalling listeners is error prone. Instead we elect to check isEnabledForUrl each time so
 # we know whether the listener should run or not.
 # Run this as early as possible, so the page can't register any event handlers before us.
+# Note: We install the listeners even if Vimium is disabled.  See comment in commit
+# 6446cf04c7b44c3d419dc450a73b60bcaf5cdf02.
 #
 installedListeners = false
-window.initializeWhenEnabled = ->
+window.installListeners = ->
   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.
@@ -583,28 +587,29 @@ onKeyup = (event) ->
 
 checkIfEnabledForUrl = ->
   url = window.location.toString()
-
   chrome.runtime.sendMessage { handler: "isEnabledForUrl", url: url }, (response) ->
-    isEnabledForUrl = response.isEnabledForUrl
-    passKeys = response.passKeys
-    isIncognitoMode = response.incognito
-    if isEnabledForUrl
-      initializeWhenEnabled()
-    else if HUD.isReady()
+    { isEnabledForUrl, passKeys } = response
+    installListeners() # But only if they have not been installed already.
+    if HUD.isReady() and not isEnabledForUrl
       # Quickly hide any HUD we might already be showing, e.g. if we entered insert mode on page load.
       HUD.hide()
     handlerStack.bubbleEvent "registerStateChange",
       enabled: isEnabledForUrl
       passKeys: passKeys
     # Update the page icon, if necessary.
-    if document.hasFocus()
+    if windowIsFocused()
       chrome.runtime.sendMessage
         handler: "setIcon"
         icon:
           if isEnabledForUrl and not passKeys then "enabled"
           else if isEnabledForUrl then "partial"
           else "disabled"
+    null
 
+# When we're informed by the background page that a URL in this tab has changed, we check if we have the
+# correct enabled state (but only if this frame has the focus).
+checkEnabledAfterURLChange = ->
+  checkIfEnabledForUrl() if windowIsFocused()
 
 # Exported to window, but only for DOM tests.
 window.refreshCompletionKeys = (response) ->
diff --git a/manifest.json b/manifest.json
index 4ba222fb..e8e51407 100644
--- a/manifest.json
+++ b/manifest.json
@@ -28,6 +28,7 @@
     "storage",
     "sessions",
     "notifications",
+    "webNavigation",
     "<all_urls>"
   ],
   "content_scripts": [
diff --git a/tests/dom_tests/chrome.coffee b/tests/dom_tests/chrome.coffee
index 5f276649..4c9bfa52 100644
--- a/tests/dom_tests/chrome.coffee
+++ b/tests/dom_tests/chrome.coffee
@@ -29,3 +29,5 @@ root.chrome =
       set: ->
     onChanged:
       addListener: ->
+  extension:
+    inIncognitoContext: false
diff --git a/tests/dom_tests/dom_tests.coffee b/tests/dom_tests/dom_tests.coffee
index f81982ac..bb09a0a8 100644
--- a/tests/dom_tests/dom_tests.coffee
+++ b/tests/dom_tests/dom_tests.coffee
@@ -1,6 +1,6 @@
 
 # Install frontend event handlers.
-initializeWhenEnabled()
+installListeners()
 
 installListener = (element, event, callback) ->
   element.addEventListener event, (-> callback.apply(this, arguments)), true
diff --git a/tests/unit_tests/exclusion_test.coffee b/tests/unit_tests/exclusion_test.coffee
index 287d699d..b3ed7194 100644
--- a/tests/unit_tests/exclusion_test.coffee
+++ b/tests/unit_tests/exclusion_test.coffee
@@ -21,10 +21,6 @@ extend(global, require "../../background_scripts/exclusions.js")
 extend(global, require "../../background_scripts/commands.js")
 extend(global, require "../../background_scripts/main.js")
 
-dummyTab =
-  tab:
-    incognito: false
-
 # These tests cover only the most basic aspects of excluded URLs and passKeys.
 #
 context "Excluded URLs and pass keys",
@@ -40,22 +36,22 @@ context "Excluded URLs and pass keys",
       ])
 
   should "be disabled for excluded sites", ->
-    rule = isEnabledForUrl({ url: 'http://mail.google.com/calendar/page' }, dummyTab)
+    rule = isEnabledForUrl({ url: 'http://mail.google.com/calendar/page' })
     assert.isFalse rule.isEnabledForUrl
     assert.isFalse rule.passKeys
 
   should "be disabled for excluded sites, one exclusion", ->
-    rule = isEnabledForUrl({ url: 'http://www.bbc.com/calendar/page' }, dummyTab)
+    rule = isEnabledForUrl({ url: 'http://www.bbc.com/calendar/page' })
     assert.isFalse rule.isEnabledForUrl
     assert.isFalse rule.passKeys
 
   should "be enabled, but with pass keys", ->
-    rule = isEnabledForUrl({ url: 'https://www.facebook.com/something' }, dummyTab)
+    rule = isEnabledForUrl({ url: 'https://www.facebook.com/something' })
     assert.isTrue rule.isEnabledForUrl
     assert.equal rule.passKeys, 'abcd'
 
   should "be enabled", ->
-    rule = isEnabledForUrl({ url: 'http://www.twitter.com/pages' }, dummyTab)
+    rule = isEnabledForUrl({ url: 'http://www.twitter.com/pages' })
     assert.isTrue rule.isEnabledForUrl
     assert.isFalse rule.passKeys
 
diff --git a/tests/unit_tests/test_chrome_stubs.coffee b/tests/unit_tests/test_chrome_stubs.coffee
index bc50521a..60f3a890 100644
--- a/tests/unit_tests/test_chrome_stubs.coffee
+++ b/tests/unit_tests/test_chrome_stubs.coffee
@@ -38,6 +38,12 @@ exports.chrome =
       addListener: () -> true
     query: () -> true
 
+  webNavigation:
+    onHistoryStateUpdated:
+      addListener: () ->
+    onReferenceFragmentUpdated:
+      addListener: () ->
+
   windows:
     onRemoved:
       addListener: () -> true