Rework UI component focus handling.

The code to handle the focus for UI components has been tweaked and
adapted over time, and has become quite complicated (and brittle).  This
reworks it from scratch, and co-locates similar code which does related
things.

Fixes #2099.

10 files changed, 93 insertions, 136 deletions

diff --git a/content_scripts/hud.coffee b/content_scripts/hud.coffee
index bbf7c5e9..fb1fd657 100644
--- a/content_scripts/hud.coffee
+++ b/content_scripts/hud.coffee
@@ -25,12 +25,12 @@ HUD =
   show: (text) ->
     return unless @isReady()
     clearTimeout(@_showForDurationTimerId)
-    @hudUI.show {name: "show", text}
+    @hudUI.activate {name: "show", text}
     @tween.fade 1.0, 150
 
   showFindMode: (@findMode = null) ->
     return unless @isReady()
-    @hudUI.show {name: "showFindMode", text: ""}
+    @hudUI.activate name: "showFindMode", text: ""
     @tween.fade 1.0, 150
 
   search: (data) ->
@@ -50,9 +50,7 @@ HUD =
     clearTimeout(@_showForDurationTimerId)
     @tween.stop()
     if immediate
-      unless updateIndicator
-        @hudUI.hide()
-        @hudUI.postMessage {name: "hide"}
+      @hudUI.hide()
       Mode.setIndicator() if updateIndicator
     else
       @tween.fade 0, 150, => @hide true, updateIndicator
@@ -60,9 +58,9 @@ HUD =
   hideFindMode: (data) ->
     @findMode.checkReturnToViewPort()
 
-    # An element element won't receive a focus event if the search landed on it while we were in the HUD
-    # iframe. To end up with the correct modes active, we create a focus/blur event manually after refocusing
-    # this window.
+    # An element won't receive a focus event if the search landed on it while we were in the HUD iframe. To
+    # end up with the correct modes active, we create a focus/blur event manually after refocusing this
+    # window.
     window.focus()
 
     focusNode = DomUtils.getSelectionFocusElement()
diff --git a/content_scripts/mode_key_handler.coffee b/content_scripts/mode_key_handler.coffee
index 08222d98..0b8145fc 100644
--- a/content_scripts/mode_key_handler.coffee
+++ b/content_scripts/mode_key_handler.coffee
@@ -42,9 +42,9 @@ class KeyHandlerMode extends Mode
       @reset()
       @suppressEvent
     # If the help dialog loses the focus, then Escape should hide it; see point 2 in #2045.
-    else if isEscape and HelpDialog?.showing
+    else if isEscape and HelpDialog?.isShowing()
       @keydownEvents[event.keyCode] = true
-      HelpDialog.hide()
+      HelpDialog.toggle()
       @suppressEvent
     else if isEscape
       @continueBubbling
diff --git a/content_scripts/ui_component.coffee b/content_scripts/ui_component.coffee
index d7bdf2a1..bb350ccc 100644
--- a/content_scripts/ui_component.coffee
+++ b/content_scripts/ui_component.coffee
@@ -2,7 +2,8 @@ class UIComponent
   uiComponentIsReady: false
   iframeElement: null
   iframePort: null
-  showing: null
+  iframeFrameId: null
+  showing: false
   options: null
   shadowDOM: null
   styleSheetGetter: null
@@ -27,10 +28,8 @@ class UIComponent
     @shadowDOM = shadowWrapper.createShadowRoot?() ? shadowWrapper
     @shadowDOM.appendChild styleSheet
     @shadowDOM.appendChild @iframeElement
-
-    @showing = true # The iframe is visible now.
-    # Hide the iframe, but don't interfere with the focus.
-    @hide false
+    @iframeElement.classList.remove "vimiumUIComponentVisible"
+    @iframeElement.classList.add "vimiumUIComponentHidden"
 
     # Open a port and pass it to the iframe via window.postMessage.  We use an AsyncDataFetcher to handle
     # requests which arrive before the iframe (and its message handlers) have completed initialization.  See
@@ -45,74 +44,59 @@ class UIComponent
         # Get vimiumSecret so the iframe can determine that our message isn't the page impersonating us.
         chrome.storage.local.get "vimiumSecret", ({ vimiumSecret }) =>
           { port1, port2 } = new MessageChannel
-          port1.onmessage = (event) =>
-            if event?.data == "uiComponentIsReady" then @uiComponentIsReady = true else @handleMessage event
           @iframeElement.contentWindow.postMessage vimiumSecret, chrome.runtime.getURL(""), [ port2 ]
-          setIframePort port1
-
-    chrome.runtime.onMessage.addListener (request) =>
-      if @showing and request.name == "frameFocused" and request.focusFrameId != frameId
-        @postMessage name: "frameFocused", focusFrameId: request.focusFrameId
-      false # Free up the sendResponse handler.
 
-  # Posts a message (if one is provided), then calls continuation (if provided).  The continuation is only
-  # ever called *after* the message has been posted.
+          port1.onmessage = (event) =>
+            switch event?.data?.name ? event?.data
+              when "uiComponentIsReady"
+                # If any other frame receives the focus, then hide the UI component.
+                chrome.runtime.onMessage.addListener ({name, focusFrameId}) =>
+                  if name == "frameFocused" and @options?.focus and focusFrameId not in [frameId, @iframeFrameId]
+                    @hide false
+                  false # We will not be calling sendResponse.
+
+                # If this frame receives the focus, then hide the UI component.
+                window.addEventListener "focus", (event) =>
+                  if event.target == window and @options?.focus and not @options?.allowBlur
+                    @hide false
+                  true # Continue propagating the event.
+
+                @uiComponentIsReady = true
+                setIframePort port1
+
+              when "setIframeFrameId" then @iframeFrameId = event.data.iframeFrameId
+              when "hide" then @hide()
+              else @handleMessage event
+
+  # Post a message (if provided), then call continuation (if provided).
   postMessage: (message = null, continuation = null) ->
     @iframePort.use (port) =>
       port.postMessage message if message?
       continuation?()
 
-  activate: (@options) ->
+  activate: (@options = null) ->
     @postMessage @options, =>
-      @show() unless @showing
-      @iframeElement.focus()
-
-  show: (message) ->
-    @postMessage message, =>
       @iframeElement.classList.remove "vimiumUIComponentHidden"
       @iframeElement.classList.add "vimiumUIComponentVisible"
-      # The window may not have the focus.  We focus it now, to prevent the "focus" listener below from firing
-      # immediately.
-      window.focus()
-      window.addEventListener "focus", @onFocus = (event) =>
-        if event.target == window
-          window.removeEventListener "focus", @onFocus
-          @onFocus = null
-          @postMessage "frameFocused"
+      @iframeElement.focus() if @options?.focus
       @showing = true
 
-  hide: (focusWindow = true)->
-    @refocusSourceFrame @options?.sourceFrameId if focusWindow
-    window.removeEventListener "focus", @onFocus if @onFocus
-    @onFocus = null
-    @iframeElement.classList.remove "vimiumUIComponentVisible"
-    @iframeElement.classList.add "vimiumUIComponentHidden"
-    @iframeElement.blur()
-    @options = null
-    @showing = false
-
-  # Refocus the frame from which the UI component was opened.  This may be different from the current frame.
-  # After hiding the UI component, Chrome refocuses the containing frame. To avoid a race condition, we need
-  # to wait until that frame first receives the focus, before then focusing the frame which should now have
-  # the focus.
-  refocusSourceFrame: (sourceFrameId) ->
-    if @showing and sourceFrameId? and sourceFrameId != frameId
-      refocusSourceFrame = ->
-        chrome.runtime.sendMessage
-          handler: "sendMessageToFrames"
-          message:
-            name: "focusFrame"
-            frameId: sourceFrameId
-
-      if windowIsFocused()
-        # We already have the focus.
-        refocusSourceFrame()
-      else
-        # We don't yet have the focus (but we'll be getting it soon).
-        window.addEventListener "focus", handler = (event) ->
-          if event.target == window
-            window.removeEventListener "focus", handler
-            refocusSourceFrame()
+  hide: (shouldRefocusOriginalFrame = true) ->
+    if @showing
+      @showing = false
+      @iframeElement.classList.remove "vimiumUIComponentVisible"
+      @iframeElement.classList.add "vimiumUIComponentHidden"
+      if @options?.focus and shouldRefocusOriginalFrame
+        @iframeElement.blur()
+        if @options?.sourceFrameId?
+          chrome.runtime.sendMessage
+            handler: "sendMessageToFrames",
+            message: name: "focusFrame", frameId: @options.sourceFrameId, forceFocusThisFrame: true
+        else
+          window.focus()
+      @options = null
+      # Inform the UI component that it is hidden.
+      Utils.nextTick => @postMessage "hidden"
 
   # Fetch a Vimium file/resource (such as "content_scripts/vimium.css").
   # We try making an XMLHttpRequest request.  That can fail (see #1817), in which case we fetch the
@@ -135,6 +119,5 @@ class UIComponent
     request.open "GET", (chrome.runtime.getURL file), true
     request.send()
 
-
 root = exports ? window
 root.UIComponent = UIComponent
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index 4dcdfe7d..537dbaa9 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -119,7 +119,10 @@ class NormalMode extends KeyHandlerMode
         You have asked Vimium to perform #{count} repetitions of the command: #{registryEntry.description}.\n
         Are you sure you want to continue?"""
 
-    if registryEntry.topFrame
+    if registryEntry.topFrame and window.isVimiumUIComponent? and window.isVimiumUIComponent
+      # We cannot use "topFrame" commands, most notably the Vomnibar, from within a UI component.
+      HUD.showForDuration "#{registryEntry.command} cannot be used here.", 2000
+    else if registryEntry.topFrame
       chrome.runtime.sendMessage
         handler: "sendMessageToFrames", message: {name: "runInTopFrame", sourceFrameId: frameId, registryEntry}
     else if registryEntry.background
@@ -156,7 +159,7 @@ initializePreDomReady = ->
     getScrollPosition: (ignoredA, ignoredB, sendResponse) ->
       sendResponse scrollX: window.scrollX, scrollY: window.scrollY if frameId == 0
     setScrollPosition: setScrollPosition
-    # 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 (UI components handle this).
     frameFocused: ->
     checkEnabledAfterURLChange: checkEnabledAfterURLChange
     runInTopFrame: ({sourceFrameId, registryEntry}) ->
@@ -626,29 +629,16 @@ enterFindMode = ->
 # If we are in the help dialog iframe, HelpDialog is already defined with the necessary functions.
 window.HelpDialog ?=
   helpUI: null
-  container: null
-  showing: false
-
-  init: ->
-    return if @helpUI?
-
-    @helpUI = new UIComponent "pages/help_dialog.html", "vimiumHelpDialogFrame", (event) =>
-      @hide() if event.data == "hide"
-
-  isReady: -> @helpUI
-
-  show: (html) ->
-    @init()
-    return if @showing or !@isReady()
-    @showing = true
-    @helpUI.activate html
-
-  hide: ->
-    @showing = false
-    @helpUI.hide()
+  isShowing: -> @helpUI?.showing
 
   toggle: (html) ->
-    if @showing then @hide() else @show html
+    @helpUI ?= new UIComponent "pages/help_dialog.html", "vimiumHelpDialogFrame", ->
+    if @isShowing()
+      @helpUI.hide()
+    else
+      # On the options page, we allow the help dialog to lose the focus, elsewhere we do not.  This allows
+      # users to view the help dialog while typing in the key-mappings input.
+      @helpUI.activate {name: "activate", html, focus: true, allowBlur: window.isVimiumOptionsPage ? false}
 
 initializePreDomReady()
 DomUtils.documentReady initializeOnDomReady
diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee
index c23a4e6f..646fda43 100644
--- a/content_scripts/vomnibar.coffee
+++ b/content_scripts/vomnibar.coffee
@@ -51,10 +51,6 @@ Vomnibar =
     unless @vomnibarUI?
       @vomnibarUI = new UIComponent "pages/vomnibar.html", "vomnibarFrame", (event) =>
         @vomnibarUI.hide() if event.data == "hide"
-      # Whenever the window receives the focus, we tell the Vomnibar UI that it has been hidden (regardless of
-      # whether it was previously visible).
-      window.addEventListener "focus", (event) =>
-        @vomnibarUI.postMessage "hidden" if event.target == window; true
 
 
   # This function opens the vomnibar. It accepts options, a map with the values:
@@ -65,7 +61,7 @@ Vomnibar =
   open: (sourceFrameId, options) ->
     @init()
     if @vomnibarUI?.uiComponentIsReady
-      @vomnibarUI.activate extend options, { sourceFrameId }
+      @vomnibarUI.activate extend options, { name: "activate", sourceFrameId, focus: true }
 
 root = exports ? window
 root.Vomnibar = Vomnibar
diff --git a/pages/help_dialog.coffee b/pages/help_dialog.coffee
index 0e4a8973..0300ec00 100644
--- a/pages/help_dialog.coffee
+++ b/pages/help_dialog.coffee
@@ -6,7 +6,7 @@
 #   top-level frame), and then we don't need to be concerned about nested help dialog frames.
 HelpDialog =
   dialogElement: null
-  showing: true
+  isShowing: -> true
 
   # This setting is pulled out of local storage. It's false by default.
   getShowAdvancedCommands: -> Settings.get("helpDialog_showAdvancedCommands")
@@ -30,9 +30,7 @@ HelpDialog =
       @hide() unless @dialogElement.contains event.target
     , false
 
-  isReady: -> true
-
-  show: (html) ->
+  show: ({html}) ->
     for own placeholder, htmlString of html
       @dialogElement.querySelector("#help-dialog-#{placeholder}").innerHTML = htmlString
 
@@ -48,15 +46,8 @@ HelpDialog =
           chrome.runtime.sendMessage handler: "copyToClipboard", data: commandName
           HUD.showForDuration("Yanked #{commandName}.", 2000)
 
-    @exitOnEscape = new Mode name: "help-page-escape", exitOnEscape: true
-    @exitOnEscape.onExit (event) => @hide() if event?.type == "keydown"
-
-  hide: ->
-    @exitOnEscape?.exit()
-    UIComponentServer.postMessage "hide"
-
-  toggle: (html) ->
-    if @showing then @hide() else @show html
+  hide: -> UIComponentServer.hide()
+  toggle: (html) -> @hide()
 
   #
   # Advanced commands are hidden by default so they don't overwhelm new and casual users.
@@ -77,13 +68,8 @@ HelpDialog =
 
 UIComponentServer.registerHandler (event) ->
   switch event.data.name ? event.data
-    when "frameFocused"
-      # We normally close when we lose the focus.  However, we do not close on the options page.  This allows
-      # users to view the help dialog while typing in the key-mappings input.
-      HelpDialog.hide() unless event.data.focusFrameId == frameId or try window.top.isVimiumOptionsPage
-    when "hide"
-      HelpDialog.hide()
-    else
+    when "hide" then HelpDialog.hide()
+    when "activate"
       HelpDialog.init()
       HelpDialog.show event.data
 
diff --git a/pages/hud.coffee b/pages/hud.coffee
index 17ac52be..d2e35cec 100644
--- a/pages/hud.coffee
+++ b/pages/hud.coffee
@@ -50,7 +50,7 @@ handlers =
     document.getElementById("hud").innerText = data.text
     document.getElementById("hud").classList.add "vimiumUIComponentVisible"
     document.getElementById("hud").classList.remove "vimiumUIComponentHidden"
-  hide: ->
+  hidden: ->
     # We get a flicker when the HUD later becomes visible again (with new text) unless we reset its contents
     # here.
     document.getElementById("hud").innerText = ""
@@ -96,8 +96,7 @@ handlers =
       " (No matches)"
     countElement.textContent = if showMatchText then countText else ""
 
-UIComponentServer.registerHandler (event) ->
-  {data} = event
-  handlers[data.name]? data
+UIComponentServer.registerHandler ({data}) ->
+  handlers[data.name ? data]? data
 
 FindModeHistory.init()
diff --git a/pages/options.coffee b/pages/options.coffee
index c708efa7..3e1843a7 100644
--- a/pages/options.coffee
+++ b/pages/options.coffee
@@ -234,7 +234,7 @@ initOptionsPage = ->
     event.preventDefault()
 
   activateHelpDialog = ->
-    HelpDialog.show chrome.extension.getBackgroundPage().helpDialogHtml(true, true, "Command Listing"), frameId
+    HelpDialog.toggle chrome.extension.getBackgroundPage().helpDialogHtml true, true, "Command Listing"
 
   saveOptions = ->
     Option.saveOptions()
diff --git a/pages/ui_component_server.coffee b/pages/ui_component_server.coffee
index 4210a60e..9f72dd92 100644
--- a/pages/ui_component_server.coffee
+++ b/pages/ui_component_server.coffee
@@ -2,14 +2,12 @@
 # Fetch the Vimium secret, register the port received from the parent window, and stop listening for messages
 # on the window object. vimiumSecret is accessible only within the current instance of Vimium.  So a
 # malicious host page trying to register its own port can do no better than guessing.
-registerPort = (event) ->
+window.addEventListener "message", registerPort = (event) ->
   chrome.storage.local.get "vimiumSecret", ({vimiumSecret: secret}) ->
     return unless event.source == window.parent and event.data == secret
     UIComponentServer.portOpen event.ports[0]
     window.removeEventListener "message", registerPort
 
-window.addEventListener "message", registerPort
-
 UIComponentServer =
   ownerPagePort: null
   handleMessage: null
@@ -23,6 +21,9 @@ UIComponentServer =
   postMessage: (message) ->
     @ownerPagePort?.postMessage message
 
+  hide: ->
+    @postMessage "hide"
+
   # We require both that the DOM is ready and that the port has been opened before the UI component is ready.
   # These events can happen in either order.  We count them, and notify the content script when we've seen
   # both.
@@ -34,7 +35,11 @@ UIComponentServer =
       else
         1
 
-    -> @postMessage "uiComponentIsReady" if ++uiComponentIsReadyCount == 2
+    ->
+      if ++uiComponentIsReadyCount == 2
+        @postMessage {name: "setIframeFrameId", iframeFrameId: window.frameId} if window.frameId?
+        @postMessage "uiComponentIsReady"
 
 root = exports ? window
 root.UIComponentServer = UIComponentServer
+root.isVimiumUIComponent = true
diff --git a/pages/vomnibar.coffee b/pages/vomnibar.coffee
index 0332b12f..449c0bac 100644
--- a/pages/vomnibar.coffee
+++ b/pages/vomnibar.coffee
@@ -38,7 +38,7 @@ Vomnibar =
 class VomnibarUI
   constructor: ->
     @refreshInterval = 0
-    @postHideCallback = null
+    @onHiddenCallback = null
     @initDom()
 
   setQuery: (query) -> @input.value = query
@@ -56,14 +56,14 @@ class VomnibarUI
   # 3. Only once the "hidden" message is received here is any required action  invoked (in onHidden).
   # This ensures that the vomnibar is actually hidden before any new tab is created, and avoids flicker after
   # opening a link in a new tab then returning to the original tab (see #1485).
-  hide: (@postHideCallback = null) ->
+  hide: (@onHiddenCallback = null) ->
     UIComponentServer.postMessage "hide"
     @reset()
-    @completer?.reset()
 
   onHidden: ->
-    @postHideCallback?()
-    @postHideCallback = null
+    @onHiddenCallback?()
+    @onHiddenCallback = null
+    @reset()
 
   reset: ->
     @clearUpdateTimer()
@@ -75,6 +75,7 @@ class VomnibarUI
     @selection = @initialSelectionValue
     @keywords = []
     @seenTabToOpenCompletionList = false
+    @completer?.reset()
 
   updateSelection: ->
     # For custom search engines, we suppress the leading term (e.g. the "w" of "w query terms") within the
@@ -331,10 +332,9 @@ class BackgroundCompleter
 
 UIComponentServer.registerHandler (event) ->
   switch event.data.name ? event.data
-    when "frameFocused" then Vomnibar.hide()
     when "hide" then Vomnibar.hide()
     when "hidden" then Vomnibar.onHidden()
-    else Vomnibar.activate event.data
+    when "activate" then Vomnibar.activate event.data
 
 root = exports ? window
 root.Vomnibar = Vomnibar