Merge pull request #2033 from smblott-github/init-ui-components-only-when-enabled

Do not initialise UI components when disabled.

5 files changed, 31 insertions, 11 deletions

diff --git a/content_scripts/hud.coffee b/content_scripts/hud.coffee
index 453d6522..aa8a6bbd 100644
--- a/content_scripts/hud.coffee
+++ b/content_scripts/hud.coffee
@@ -13,9 +13,10 @@ HUD =
   # it doesn't sit on top of horizontal scrollbars like Chrome's HUD does.
 
   init: ->
-    @hudUI = new UIComponent "pages/hud.html", "vimiumHUDFrame", ({data}) =>
-      this[data.name]? data
-    @tween = new Tween "iframe.vimiumHUDFrame.vimiumUIComponentVisible", @hudUI.shadowDOM
+    unless @hudUI?
+      @hudUI = new UIComponent "pages/hud.html", "vimiumHUDFrame", ({data}) =>
+        this[data.name]? data
+      @tween = new Tween "iframe.vimiumHUDFrame.vimiumUIComponentVisible", @hudUI.shadowDOM
 
   showForDuration: (text, duration) ->
     @show(text)
@@ -85,7 +86,7 @@ HUD =
   isReady: do ->
     ready = false
     DomUtils.documentReady -> ready = true
-    -> ready and document.body != null
+    -> ready and document.body != null and @hudUI?
 
   enabled: -> true
 
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index 2e57ee1a..1a882192 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -149,6 +149,7 @@ initializePreDomReady = ->
     # A frame has received the focus.  We don't care here (the Vomnibar/UI-component handles this).
     frameFocused: ->
     checkEnabledAfterURLChange: checkEnabledAfterURLChange
+    initializeTopFrame: initializeTopFrame
     runInTopFrame: ({sourceFrameId, registryEntry}) ->
       Utils.invokeCommandString registryEntry.command, [sourceFrameId, registryEntry] if DomUtils.isTopFrame()
 
@@ -218,9 +219,6 @@ initializeOnDomReady = ->
     isEnabledForUrl = false
     chrome.runtime.sendMessage = ->
     window.removeEventListener "focus", onFocus
-  # We only initialize the vomnibar in the tab's main frame, because it's only ever opened there.
-  Vomnibar.init() if DomUtils.isTopFrame()
-  HUD.init()
 
 registerFrame = ->
   # Don't register frameset containers; focusing them is no use.
@@ -436,6 +434,19 @@ extend window,
               indicator: false
 
 
+initializeTopFrame = (request = null) ->
+  initializeTopFrame = -> # Only do this initialization once.
+  # We only initialize the vomnibar in the tab's top/main frame, because it's only ever opened there.
+  if DomUtils.isTopFrame()
+    DomUtils.documentReady Vomnibar.init.bind Vomnibar
+  else
+    # Ignore requests from other frames (we're not the top frame).
+    unless request?
+      # Tell the top frame to initialize the Vomnibar.  We wait until "DOMContentLoaded" to ensure that the
+      # listener in the main/top frame (which are installed pre-DomReady) is already installed.
+      DomUtils.documentReady ->
+        chrome.runtime.sendMessage handler: "sendMessageToFrames", message: name: "initializeTopFrame"
+
 # Checks if Vimium should be enabled or not in this frame.  As a side effect, it also informs the background
 # page whether this frame has the focus, allowing the background page to track the active frame's URL.
 checkIfEnabledForUrl = (frameIsFocused = windowIsFocused()) ->
@@ -443,7 +454,11 @@ checkIfEnabledForUrl = (frameIsFocused = windowIsFocused()) ->
   chrome.runtime.sendMessage { handler: "isEnabledForUrl", url: url, frameIsFocused: frameIsFocused }, (response) ->
     { isEnabledForUrl, passKeys } = response
     installListeners() # But only if they have not been installed already.
-    if HUD.isReady() and not isEnabledForUrl
+    # Initialize UI components. We only initialize these once we know that Vimium is enabled; see #1838.
+    if isEnabledForUrl
+      initializeTopFrame()
+      DomUtils.documentReady HUD.init.bind HUD if frameIsFocused
+    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()
     normalMode?.setPassKeys passKeys
diff --git a/lib/dom_utils.coffee b/lib/dom_utils.coffee
index aab0a4df..3581bd3f 100644
--- a/lib/dom_utils.coffee
+++ b/lib/dom_utils.coffee
@@ -2,11 +2,13 @@ DomUtils =
   #
   # Runs :callback if the DOM has loaded, otherwise runs it on load
   #
-  documentReady: (func) ->
+  documentReady: (callback) ->
     if document.readyState == "loading"
-      window.addEventListener "DOMContentLoaded", func
+      window.addEventListener "DOMContentLoaded", handler = ->
+        window.removeEventListener "DOMContentLoaded", handler
+        callback()
     else
-      func()
+      callback()
 
   createElement: (tagName) ->
     element = document.createElement tagName
diff --git a/tests/dom_tests/dom_tests.coffee b/tests/dom_tests/dom_tests.coffee
index 0745e5b2..3797a37b 100644
--- a/tests/dom_tests/dom_tests.coffee
+++ b/tests/dom_tests/dom_tests.coffee
@@ -1,6 +1,7 @@
 
 # Install frontend event handlers.
 installListeners()
+HUD.init()
 
 installListener = (element, event, callback) ->
   element.addEventListener event, (-> callback.apply(this, arguments)), true
diff --git a/tests/dom_tests/vomnibar_test.coffee b/tests/dom_tests/vomnibar_test.coffee
index 3eda6234..0898e33a 100644
--- a/tests/dom_tests/vomnibar_test.coffee
+++ b/tests/dom_tests/vomnibar_test.coffee
@@ -1,5 +1,6 @@
 vomnibarFrame = null
 SearchEngines.refresh ""
+Vomnibar.init()
 
 context "Keep selection within bounds",