From 94a32b9ddcec58439a05179d9b6aeec135b46fb2 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 15 Mar 2015 12:13:52 +0000 Subject: Fix frame-focus detection. In b05276ed8264e5a71f20a7068690ba2a414ee6d8, I inadvertently moved the focus handler from window to document. As a consequence, detectFocus (now registerFocus) wasn't being called when expected. --- content_scripts/vimium_frontend.coffee | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index bc56e175..429657fc 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -209,7 +209,7 @@ window.initializeWhenEnabled = -> 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 - installListener document, "focus", detectFocus + installListener window, "focus", registerFocus installedListeners = true FindModeHistory.init() @@ -229,7 +229,7 @@ getActiveState = -> # # The backend needs to know which frame has focus. # -detectFocus = -> +registerFocus = -> # settings may have changed since the frame last had focus settings.load() chrome.runtime.sendMessage({ handler: "frameFocused", frameId: frameId }) -- cgit v1.2.3 From 3876a4f06de77d190fdaecc6cf99eb57134d0372 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 15 Mar 2015 12:32:42 +0000 Subject: Populate popup with frame's URL. Previously, we have been populating the suggested exclusion URL in the page popup with the tab's URL. This uses the active frame's URL instead: - because this is the URL which will affect the current frame (without this, a user can naively add an exclusion rule and it has no effect), and - because, without this, the user has no reasonable way to add exclusion rules for frames such as the hangouts frame on gmail (for which, the URL is not displayed in the address bar). --- content_scripts/vimium_frontend.coffee | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 429657fc..757cc0ad 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -190,6 +190,7 @@ initializePreDomReady = -> # Ensure the sendResponse callback is freed. false + # Wrapper to install event listeners. Syntactic sugar. installListener = (element, event, callback) -> element.addEventListener(event, -> @@ -227,12 +228,12 @@ getActiveState = -> return { enabled: isEnabledForUrl, passKeys: passKeys } # -# The backend needs to know which frame has focus. +# The backend needs to know which frame has focus, and the active URL. # registerFocus = -> # settings may have changed since the frame last had focus settings.load() - chrome.runtime.sendMessage({ handler: "frameFocused", frameId: frameId }) + chrome.runtime.sendMessage handler: "frameFocused", frameId: frameId, url: window.location.toString() # # Initialization tasks that must wait for the document to be ready. -- cgit v1.2.3 From ea05de3f114dcc81a91bf863538f20754641cadd Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 15 Mar 2015 13:14:26 +0000 Subject: Propagate exclusion rules to all frames. When the active tab changes, we call updateActiveState to update the icon and propagate any changed exclusion-rule state to the tab. All frames received the request. However, only one response is received by the background page. Therefore, new exclusion rules are only propagated to one frame. Here's what can go wrong... On gmail, open the hangouts frame. Add an exclusion rule sepcific to the hangouts frame. Save it. The update is propagated only to the main frame. The new exclusion rule is not in effect in the hangouts frame. That's wierd and obviously wrong. In this commit, every frame receiving the getActiveState request also calls checkIfEnabledForUrl to ensure that any new exclusion-rule state is propagated. This is overkill, to some extent. We should really move settings to chrome.storage and have each frame check locally for changes affecting it. --- content_scripts/vimium_frontend.coffee | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 757cc0ad..3577e6f3 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -150,7 +150,7 @@ initializePreDomReady = -> settings.load() initializeModes() - checkIfEnabledForUrl() + checkIfEnabledForUrl true # true means checkIfEnabledForUrl is being called on start up. refreshCompletionKeys() # Send the key to the key handler in the background page. @@ -225,7 +225,12 @@ setState = (request) -> getActiveState = -> Mode.updateBadge() - return { enabled: isEnabledForUrl, passKeys: passKeys } + # getActiveState is called in each frame within the tab. However, only the response from the first frame is + # handled on the background page. Therefore, exclusion rule changes are not propagated to other frames. + # So, we force a state update for this frame, just in case. + # FIXME(smblott): This could be avoided if settings were propagated via chrome.storage. + checkIfEnabledForUrl() + return enabled: isEnabledForUrl, passKeys: passKeys, url: window.location.toString() # # The backend needs to know which frame has focus, and the active URL. @@ -557,7 +562,7 @@ onKeyup = (event) -> DomUtils.suppressPropagation(event) @stopBubblingAndTrue -checkIfEnabledForUrl = -> +checkIfEnabledForUrl = (onStartUp = false) -> url = window.location.toString() chrome.runtime.sendMessage { handler: "isEnabledForUrl", url: url }, (response) -> @@ -566,7 +571,7 @@ checkIfEnabledForUrl = -> isIncognitoMode = response.incognito if isEnabledForUrl initializeWhenEnabled() - else if (HUD.isReady()) + else if onStartUp and HUD.isReady() # Quickly hide any HUD we might already be showing, e.g. if we entered insert mode on page load. HUD.hide() handlerStack.bubbleEvent "registerStateChange", -- cgit v1.2.3 From fb24c770645ce1b8178729d4be612d58497ffa5f Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 15 Mar 2015 15:04:23 +0000 Subject: Rework page icon handling. Setting the page icon is now driven from the corrently-active frame. - Eliminates a race condition. - Icon matches actual frame state (not tab URL state). - Exclusion-rule changes propagate to all frames. --- content_scripts/vimium_frontend.coffee | 48 +++++++++++++++------------------- 1 file changed, 21 insertions(+), 27 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index 3577e6f3..b96157c1 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -162,6 +162,7 @@ initializePreDomReady = -> isEnabledForUrl = false chrome.runtime.sendMessage = -> chrome.runtime.connect = -> + window.removeEventListener "focus", onFocus requestHandlers = hideUpgradeNotification: -> HUD.hideUpgradeNotification() @@ -173,8 +174,6 @@ initializePreDomReady = -> getScrollPosition: -> scrollX: window.scrollX, scrollY: window.scrollY setScrollPosition: (request) -> setScrollPosition request.scrollX, request.scrollY executePageCommand: executePageCommand - getActiveState: getActiveState - setState: setState currentKeyQueue: (request) -> keyQueue = request.keyQueue handlerStack.bubbleEvent "registerKeyQueue", { keyQueue: keyQueue } @@ -183,7 +182,7 @@ initializePreDomReady = -> # In the options page, we will receive requests from both content and background scripts. ignore those # from the former. return if sender.tab and not sender.tab.url.startsWith 'chrome-extension://' - return unless isEnabledForUrl or request.name == 'getActiveState' or request.name == 'setState' + return unless isEnabledForUrl # These requests are delivered to the options page, but there are no handlers there. return if request.handler in [ "registerFrame", "frameFocused", "unregisterFrame" ] sendResponse requestHandlers[request.name](request, sender) @@ -210,35 +209,21 @@ window.initializeWhenEnabled = -> 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 - installListener window, "focus", registerFocus installedListeners = true FindModeHistory.init() -setState = (request) -> - isEnabledForUrl = request.enabled - passKeys = request.passKeys - isIncognitoMode = request.incognito - initializeWhenEnabled() if isEnabledForUrl - handlerStack.bubbleEvent "registerStateChange", - enabled: isEnabledForUrl - passKeys: passKeys - -getActiveState = -> - Mode.updateBadge() - # getActiveState is called in each frame within the tab. However, only the response from the first frame is - # handled on the background page. Therefore, exclusion rule changes are not propagated to other frames. - # So, we force a state update for this frame, just in case. - # FIXME(smblott): This could be avoided if settings were propagated via chrome.storage. - checkIfEnabledForUrl() - return enabled: isEnabledForUrl, passKeys: passKeys, url: window.location.toString() - # -# The backend needs to know which frame has focus, and the active URL. +# Whenever we get the focus: +# - Reload settings (they may have changed). +# - Tell the background page this frame's URL. +# - Check if we should be enabled. # -registerFocus = -> - # settings may have changed since the frame last had focus - settings.load() - chrome.runtime.sendMessage handler: "frameFocused", frameId: frameId, url: window.location.toString() +onFocus = (event) -> + if event.target == window + settings.load() + chrome.runtime.sendMessage handler: "frameFocused", frameId: frameId, url: window.location.toString() + checkIfEnabledForUrl() +window.addEventListener "focus", onFocus # # Initialization tasks that must wait for the document to be ready. @@ -577,6 +562,15 @@ checkIfEnabledForUrl = (onStartUp = false) -> handlerStack.bubbleEvent "registerStateChange", enabled: isEnabledForUrl passKeys: passKeys + # Update the page icon, if necessary. + if document.hasFocus() + chrome.runtime.sendMessage + handler: "setIcon" + icon: + if isEnabledForUrl and not passKeys then "enabled" + else if isEnabledForUrl then "partial" + else "disabled" + # Exported to window, but only for DOM tests. window.refreshCompletionKeys = (response) -> -- cgit v1.2.3 From 5b84f5cf94d32b6e8be75448f9c06aa7f2068582 Mon Sep 17 00:00:00 2001 From: Stephen Blott Date: Sun, 15 Mar 2015 15:38:36 +0000 Subject: Tidy up exclusion rules/propagation. --- content_scripts/vimium_frontend.coffee | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'content_scripts') diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee index b96157c1..d0fc158c 100644 --- a/content_scripts/vimium_frontend.coffee +++ b/content_scripts/vimium_frontend.coffee @@ -150,7 +150,7 @@ initializePreDomReady = -> settings.load() initializeModes() - checkIfEnabledForUrl true # true means checkIfEnabledForUrl is being called on start up. + checkIfEnabledForUrl() refreshCompletionKeys() # Send the key to the key handler in the background page. @@ -547,7 +547,7 @@ onKeyup = (event) -> DomUtils.suppressPropagation(event) @stopBubblingAndTrue -checkIfEnabledForUrl = (onStartUp = false) -> +checkIfEnabledForUrl = -> url = window.location.toString() chrome.runtime.sendMessage { handler: "isEnabledForUrl", url: url }, (response) -> @@ -556,7 +556,7 @@ checkIfEnabledForUrl = (onStartUp = false) -> isIncognitoMode = response.incognito if isEnabledForUrl initializeWhenEnabled() - else if onStartUp and HUD.isReady() + else if HUD.isReady() # Quickly hide any HUD we might already be showing, e.g. if we entered insert mode on page load. HUD.hide() handlerStack.bubbleEvent "registerStateChange", -- cgit v1.2.3