From 5bfe6dc5d1e0aeb1ab3e372821997d83ba5c9164 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Fri, 15 Apr 2016 16:51:18 +0100
Subject: 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.
---
 content_scripts/hud.coffee              |  14 ++---
 content_scripts/mode_key_handler.coffee |   4 +-
 content_scripts/ui_component.coffee     | 107 ++++++++++++++------------------
 content_scripts/vimium_frontend.coffee  |  36 ++++-------
 content_scripts/vomnibar.coffee         |   6 +-
 pages/help_dialog.coffee                |  26 ++------
 pages/hud.coffee                        |   7 +--
 pages/options.coffee                    |   2 +-
 pages/ui_component_server.coffee        |  13 ++--
 pages/vomnibar.coffee                   |  14 ++---
 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
-- 
cgit v1.2.3


From f3aba1914dc645d4de15ff6e6a9104dcb99018ee Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 16 Apr 2016 14:30:05 +0100
Subject: Remove dead code from the HUD initialization.

---
 content_scripts/hud.coffee | 2 +-
 pages/hud.coffee           | 4 ----
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/content_scripts/hud.coffee b/content_scripts/hud.coffee
index fb1fd657..32662861 100644
--- a/content_scripts/hud.coffee
+++ b/content_scripts/hud.coffee
@@ -30,7 +30,7 @@ HUD =
 
   showFindMode: (@findMode = null) ->
     return unless @isReady()
-    @hudUI.activate name: "showFindMode", text: ""
+    @hudUI.activate name: "showFindMode"
     @tween.fade 1.0, 150
 
   search: (data) ->
diff --git a/pages/hud.coffee b/pages/hud.coffee
index d2e35cec..e35e587e 100644
--- a/pages/hud.coffee
+++ b/pages/hud.coffee
@@ -63,7 +63,6 @@ handlers =
 
     inputElement = document.createElement "span"
     inputElement.contentEditable = "plaintext-only"
-    setTextInInputElement inputElement, data.text if data.text
     inputElement.id = "hud-find-input"
     hud.appendChild inputElement
 
@@ -77,9 +76,6 @@ handlers =
     hud.appendChild countElement
     inputElement.focus()
 
-    # Replace \u00A0 ( ) with a normal space.
-    UIComponentServer.postMessage {name: "search", query: inputElement.textContent.replace "\u00A0", " "}
-
     findMode =
       historyIndex: -1
       partialQuery: ""
-- 
cgit v1.2.3


From b94caa42ec2433ec61132be30bec2fbe0bd22aa5 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 16 Apr 2016 14:37:05 +0100
Subject: We cannot wait for nextTick() here...

It interferes with the HUD's timing.
---
 content_scripts/ui_component.coffee | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/content_scripts/ui_component.coffee b/content_scripts/ui_component.coffee
index bb350ccc..675b46c7 100644
--- a/content_scripts/ui_component.coffee
+++ b/content_scripts/ui_component.coffee
@@ -96,7 +96,7 @@ class UIComponent
           window.focus()
       @options = null
       # Inform the UI component that it is hidden.
-      Utils.nextTick => @postMessage "hidden"
+      @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
-- 
cgit v1.2.3


From 59d6ce006da3cb125252dfd98819daf65650781e Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 16 Apr 2016 14:43:15 +0100
Subject: Self code review re. ui-component focus handling.

---
 content_scripts/hud.coffee          | 3 +--
 content_scripts/ui_component.coffee | 3 ++-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/content_scripts/hud.coffee b/content_scripts/hud.coffee
index 32662861..0bd1df3e 100644
--- a/content_scripts/hud.coffee
+++ b/content_scripts/hud.coffee
@@ -50,8 +50,7 @@ HUD =
     clearTimeout(@_showForDurationTimerId)
     @tween.stop()
     if immediate
-      @hudUI.hide()
-      Mode.setIndicator() if updateIndicator
+      if updateIndicator then Mode.setIndicator() else @hudUI.hide()
     else
       @tween.fade 0, 150, => @hide true, updateIndicator
 
diff --git a/content_scripts/ui_component.coffee b/content_scripts/ui_component.coffee
index 675b46c7..6aa22119 100644
--- a/content_scripts/ui_component.coffee
+++ b/content_scripts/ui_component.coffee
@@ -2,8 +2,8 @@ class UIComponent
   uiComponentIsReady: false
   iframeElement: null
   iframePort: null
-  iframeFrameId: null
   showing: false
+  iframeFrameId: null
   options: null
   shadowDOM: null
   styleSheetGetter: null
@@ -119,5 +119,6 @@ class UIComponent
     request.open "GET", (chrome.runtime.getURL file), true
     request.send()
 
+
 root = exports ? window
 root.UIComponent = UIComponent
-- 
cgit v1.2.3


From e60066218806ca42ebe94fd1ce0042993909658d Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 16 Apr 2016 15:20:45 +0100
Subject: Reinstate vomnibar from within help dialog.

---
 content_scripts/vimium_frontend.coffee | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index 537dbaa9..023952c1 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -119,12 +119,12 @@ 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 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
+    if registryEntry.topFrame
+      # The Vomnibar (a top-frame command) cannot coexist with the help dialog (it causes focus issues).
+      sourceFrameId = if HelpDialog.isShowing() and window.isVimiumUIComponent then 0 else frameId
+      HelpDialog.toggle() if HelpDialog.isShowing()
       chrome.runtime.sendMessage
-        handler: "sendMessageToFrames", message: {name: "runInTopFrame", sourceFrameId: frameId, registryEntry}
+        handler: "sendMessageToFrames", message: {name: "runInTopFrame", sourceFrameId, registryEntry}
     else if registryEntry.background
       chrome.runtime.sendMessage {handler: "runBackgroundCommand", registryEntry, count}
     else
-- 
cgit v1.2.3


From ae1a54157291c0bcc69a6e8652bc15e69b72b4e4 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 16 Apr 2016 16:36:35 +0100
Subject: Do not focus a hidden help dialog.

When the help dialog UI component has been created but subsequently
hidden, `gf` was nevertheless selecting it.
---
 content_scripts/vimium_frontend.coffee | 8 +++++---
 pages/help_dialog.coffee               | 5 ++++-
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index 023952c1..56f7742e 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -291,9 +291,11 @@ DomUtils.documentReady ->
 #
 focusThisFrame = (request) ->
   unless request.forceFocusThisFrame
-    if DomUtils.windowIsTooSmall() or document.body?.tagName.toLowerCase() == "frameset"
-      # This frame is too small to focus or it's a frameset. Cancel and tell the background page to focus the
-      # next frame instead.  This affects sites like Google Inbox, which have many tiny iframes. See #1317.
+    skipThisFrame = DomUtils.windowIsTooSmall() # Frame is too small; see #1317.
+    skipThisFrame ||= document.body?.tagName.toLowerCase() == "frameset"
+    skipThisFrame ||= window.isVimiumUIComponent and not HelpDialog.showing
+    if skipThisFrame
+      # Cancel and tell the background page to focus the next frame instead.
       chrome.runtime.sendMessage handler: "nextFrame"
       return
   window.focus()
diff --git a/pages/help_dialog.coffee b/pages/help_dialog.coffee
index 0300ec00..7fee8bf9 100644
--- a/pages/help_dialog.coffee
+++ b/pages/help_dialog.coffee
@@ -6,7 +6,8 @@
 #   top-level frame), and then we don't need to be concerned about nested help dialog frames.
 HelpDialog =
   dialogElement: null
-  isShowing: -> true
+  showing: false
+  isShowing: -> @showing
 
   # This setting is pulled out of local storage. It's false by default.
   getShowAdvancedCommands: -> Settings.get("helpDialog_showAdvancedCommands")
@@ -69,8 +70,10 @@ HelpDialog =
 UIComponentServer.registerHandler (event) ->
   switch event.data.name ? event.data
     when "hide" then HelpDialog.hide()
+    when "hidden" then HelpDialog.showing = false
     when "activate"
       HelpDialog.init()
+      HelpDialog.showing = true
       HelpDialog.show event.data
 
 root = exports ? window
-- 
cgit v1.2.3


From e07a252508eea45c4dcadc92cdad02d42df2f5f8 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sat, 16 Apr 2016 16:54:12 +0100
Subject: Minor code review.

---
 content_scripts/ui_component.coffee    | 4 +---
 content_scripts/vimium_frontend.coffee | 2 +-
 content_scripts/vomnibar.coffee        | 4 +---
 pages/help_dialog.coffee               | 2 +-
 pages/hud.coffee                       | 4 +---
 pages/ui_component_server.coffee       | 7 ++-----
 6 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/content_scripts/ui_component.coffee b/content_scripts/ui_component.coffee
index 6aa22119..7792bfba 100644
--- a/content_scripts/ui_component.coffee
+++ b/content_scripts/ui_component.coffee
@@ -95,8 +95,7 @@ class UIComponent
         else
           window.focus()
       @options = null
-      # Inform the UI component that it is hidden.
-      @postMessage "hidden"
+      @postMessage "hidden" # Inform the UI component that it is 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
@@ -119,6 +118,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 56f7742e..986fd322 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -121,7 +121,7 @@ class NormalMode extends KeyHandlerMode
 
     if registryEntry.topFrame
       # The Vomnibar (a top-frame command) cannot coexist with the help dialog (it causes focus issues).
-      sourceFrameId = if HelpDialog.isShowing() and window.isVimiumUIComponent then 0 else frameId
+      sourceFrameId = if window.isVimiumUIComponent then 0 else frameId
       HelpDialog.toggle() if HelpDialog.isShowing()
       chrome.runtime.sendMessage
         handler: "sendMessageToFrames", message: {name: "runInTopFrame", sourceFrameId, registryEntry}
diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee
index 646fda43..02ce97c5 100644
--- a/content_scripts/vomnibar.coffee
+++ b/content_scripts/vomnibar.coffee
@@ -49,9 +49,7 @@ Vomnibar =
 
   init: ->
     unless @vomnibarUI?
-      @vomnibarUI = new UIComponent "pages/vomnibar.html", "vomnibarFrame", (event) =>
-        @vomnibarUI.hide() if event.data == "hide"
-
+      @vomnibarUI = new UIComponent "pages/vomnibar.html", "vomnibarFrame", ->
 
   # This function opens the vomnibar. It accepts options, a map with the values:
   #   completer   - The completer to fetch results from.
diff --git a/pages/help_dialog.coffee b/pages/help_dialog.coffee
index 7fee8bf9..2ea4353c 100644
--- a/pages/help_dialog.coffee
+++ b/pages/help_dialog.coffee
@@ -48,7 +48,7 @@ HelpDialog =
           HUD.showForDuration("Yanked #{commandName}.", 2000)
 
   hide: -> UIComponentServer.hide()
-  toggle: (html) -> @hide()
+  toggle: -> @hide()
 
   #
   # Advanced commands are hidden by default so they don't overwhelm new and casual users.
diff --git a/pages/hud.coffee b/pages/hud.coffee
index e35e587e..fcc7b1dd 100644
--- a/pages/hud.coffee
+++ b/pages/hud.coffee
@@ -92,7 +92,5 @@ handlers =
       " (No matches)"
     countElement.textContent = if showMatchText then countText else ""
 
-UIComponentServer.registerHandler ({data}) ->
-  handlers[data.name ? data]? data
-
+UIComponentServer.registerHandler ({data}) -> handlers[data.name ? data]? data
 FindModeHistory.init()
diff --git a/pages/ui_component_server.coffee b/pages/ui_component_server.coffee
index 9f72dd92..488ff0ed 100644
--- a/pages/ui_component_server.coffee
+++ b/pages/ui_component_server.coffee
@@ -18,11 +18,8 @@ UIComponentServer =
 
   registerHandler: (@handleMessage) ->
 
-  postMessage: (message) ->
-    @ownerPagePort?.postMessage message
-
-  hide: ->
-    @postMessage "hide"
+  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
-- 
cgit v1.2.3


From 025b7f930205e9dfbae5f2dff5c7c1fd4a45e4c1 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sun, 17 Apr 2016 10:18:08 +0100
Subject: Use register/unregister frame in help dialog.

Remove special-purpose code from `gf`.

Instead, register the help dialog frame when it launches, and unregister
it when it's hidden.

This way, when the helpd-dialog frame is hidden, it simply isn't
available for `gf` and for link hints.
---
 content_scripts/vimium_frontend.coffee | 1 -
 pages/help_dialog.coffee               | 9 +++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index 986fd322..3c429115 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -293,7 +293,6 @@ focusThisFrame = (request) ->
   unless request.forceFocusThisFrame
     skipThisFrame = DomUtils.windowIsTooSmall() # Frame is too small; see #1317.
     skipThisFrame ||= document.body?.tagName.toLowerCase() == "frameset"
-    skipThisFrame ||= window.isVimiumUIComponent and not HelpDialog.showing
     if skipThisFrame
       # Cancel and tell the background page to focus the next frame instead.
       chrome.runtime.sendMessage handler: "nextFrame"
diff --git a/pages/help_dialog.coffee b/pages/help_dialog.coffee
index 2ea4353c..111ef73e 100644
--- a/pages/help_dialog.coffee
+++ b/pages/help_dialog.coffee
@@ -6,8 +6,7 @@
 #   top-level frame), and then we don't need to be concerned about nested help dialog frames.
 HelpDialog =
   dialogElement: null
-  showing: false
-  isShowing: -> @showing
+  isShowing: -> true
 
   # This setting is pulled out of local storage. It's false by default.
   getShowAdvancedCommands: -> Settings.get("helpDialog_showAdvancedCommands")
@@ -70,11 +69,13 @@ HelpDialog =
 UIComponentServer.registerHandler (event) ->
   switch event.data.name ? event.data
     when "hide" then HelpDialog.hide()
-    when "hidden" then HelpDialog.showing = false
     when "activate"
       HelpDialog.init()
-      HelpDialog.showing = true
       HelpDialog.show event.data
+      Frame.postMessage "registerFrame"
+    when "hidden"
+      # Unregister the frame, so that it's not available for `gf` or linkk hints.
+      Frame.postMessage "unregisterFrame"
 
 root = exports ? window
 root.HelpDialog = HelpDialog
-- 
cgit v1.2.3


From d43f18bd5ce7c39e8e663a657027b233707e2926 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sun, 17 Apr 2016 11:13:27 +0100
Subject: More code review of UI-component focus handling.

---
 content_scripts/ui_component.coffee    | 33 ++++++++++++++++-----------------
 content_scripts/vimium_frontend.coffee | 10 ++++------
 pages/help_dialog.coffee               |  2 +-
 3 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/content_scripts/ui_component.coffee b/content_scripts/ui_component.coffee
index 7792bfba..0989bbc9 100644
--- a/content_scripts/ui_component.coffee
+++ b/content_scripts/ui_component.coffee
@@ -8,6 +8,10 @@ class UIComponent
   shadowDOM: null
   styleSheetGetter: null
 
+  toggleIframeElementClasses: (removeClass, addClass) ->
+    @iframeElement.classList.remove removeClass
+    @iframeElement.classList.add addClass
+
   constructor: (iframeUrl, className, @handleMessage) ->
     styleSheet = DomUtils.createElement "style"
     styleSheet.type = "text/css"
@@ -28,8 +32,7 @@ class UIComponent
     @shadowDOM = shadowWrapper.createShadowRoot?() ? shadowWrapper
     @shadowDOM.appendChild styleSheet
     @shadowDOM.appendChild @iframeElement
-    @iframeElement.classList.remove "vimiumUIComponentVisible"
-    @iframeElement.classList.add "vimiumUIComponentHidden"
+    @toggleIframeElementClasses "vimiumUIComponentVisible", "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,7 +48,6 @@ class UIComponent
         chrome.storage.local.get "vimiumSecret", ({ vimiumSecret }) =>
           { port1, port2 } = new MessageChannel
           @iframeElement.contentWindow.postMessage vimiumSecret, chrome.runtime.getURL(""), [ port2 ]
-
           port1.onmessage = (event) =>
             switch event?.data?.name ? event?.data
               when "uiComponentIsReady"
@@ -54,16 +56,14 @@ class UIComponent
                   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.
-
+                # Register the UI component as ready and post its iframe port.
                 @uiComponentIsReady = true
                 setIframePort port1
-
               when "setIframeFrameId" then @iframeFrameId = event.data.iframeFrameId
               when "hide" then @hide()
               else @handleMessage event
@@ -76,24 +76,23 @@ class UIComponent
 
   activate: (@options = null) ->
     @postMessage @options, =>
-      @iframeElement.classList.remove "vimiumUIComponentHidden"
-      @iframeElement.classList.add "vimiumUIComponentVisible"
+      @toggleIframeElementClasses "vimiumUIComponentHidden", "vimiumUIComponentVisible"
       @iframeElement.focus() if @options?.focus
       @showing = true
 
   hide: (shouldRefocusOriginalFrame = true) ->
     if @showing
       @showing = false
-      @iframeElement.classList.remove "vimiumUIComponentVisible"
-      @iframeElement.classList.add "vimiumUIComponentHidden"
-      if @options?.focus and shouldRefocusOriginalFrame
+      @toggleIframeElementClasses "vimiumUIComponentVisible", "vimiumUIComponentHidden"
+      if @options?.focus
         @iframeElement.blur()
-        if @options?.sourceFrameId?
-          chrome.runtime.sendMessage
-            handler: "sendMessageToFrames",
-            message: name: "focusFrame", frameId: @options.sourceFrameId, forceFocusThisFrame: true
-        else
-          window.focus()
+        if shouldRefocusOriginalFrame
+          if @options?.sourceFrameId?
+            chrome.runtime.sendMessage
+              handler: "sendMessageToFrames",
+              message: name: "focusFrame", frameId: @options.sourceFrameId, forceFocusThisFrame: true
+          else
+            window.focus()
       @options = null
       @postMessage "hidden" # Inform the UI component that it is hidden.
 
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index 3c429115..7d6fa9a0 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -159,8 +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 (UI components handle this).
-    frameFocused: ->
+    frameFocused: -> # A frame has received the focus; we don't care here (UI components handle this).
     checkEnabledAfterURLChange: checkEnabledAfterURLChange
     runInTopFrame: ({sourceFrameId, registryEntry}) ->
       Utils.invokeCommandString registryEntry.command, sourceFrameId, registryEntry if DomUtils.isTopFrame()
@@ -291,10 +290,9 @@ DomUtils.documentReady ->
 #
 focusThisFrame = (request) ->
   unless request.forceFocusThisFrame
-    skipThisFrame = DomUtils.windowIsTooSmall() # Frame is too small; see #1317.
-    skipThisFrame ||= document.body?.tagName.toLowerCase() == "frameset"
-    if skipThisFrame
-      # Cancel and tell the background page to focus the next frame instead.
+    if DomUtils.windowIsTooSmall() or document.body?.tagName.toLowerCase() == "frameset"
+      # This frame is too small to focus or it's a frameset. Cancel and tell the background page to focus the
+      # next frame instead.  This affects sites like Google Inbox, which have many tiny iframes. See #1317.
       chrome.runtime.sendMessage handler: "nextFrame"
       return
   window.focus()
diff --git a/pages/help_dialog.coffee b/pages/help_dialog.coffee
index 111ef73e..990fa063 100644
--- a/pages/help_dialog.coffee
+++ b/pages/help_dialog.coffee
@@ -74,7 +74,7 @@ UIComponentServer.registerHandler (event) ->
       HelpDialog.show event.data
       Frame.postMessage "registerFrame"
     when "hidden"
-      # Unregister the frame, so that it's not available for `gf` or linkk hints.
+      # Unregister the frame, so that it's not available for `gf` or link hints.
       Frame.postMessage "unregisterFrame"
 
 root = exports ? window
-- 
cgit v1.2.3


From 5c3e4bda2968486e23f8cc3410e776de67fec849 Mon Sep 17 00:00:00 2001
From: Stephen Blott
Date: Sun, 17 Apr 2016 11:54:28 +0100
Subject: Better window focus handling.

This fixes two issues:

- We cannot set windowHasFocus until the DOM is ready (because
  document.hasFocus isn't set until then.
- We should use windowhasFocus in prefernce to document.hasFocus
  because, for framesets, document.hasFocus is true for both the
  frameset and a focused contained frame.
---
 content_scripts/link_hints.coffee      | 6 +++---
 content_scripts/vimium_frontend.coffee | 3 ++-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/content_scripts/link_hints.coffee b/content_scripts/link_hints.coffee
index f2b9cd0f..702ff69d 100644
--- a/content_scripts/link_hints.coffee
+++ b/content_scripts/link_hints.coffee
@@ -334,7 +334,7 @@ class LinkHintsMode
         flashEl = DomUtils.addFlashRect linkMatched.rect
         HintCoordinator.onExit.push -> DomUtils.removeElement flashEl
 
-      if document.hasFocus()
+      if windowIsFocused()
         startKeyboardBlocker (isSuccess) -> HintCoordinator.sendMessage "exit", {isSuccess}
 
     # If we're using a keyboard blocker, then the frame with the focus sends the "exit" message, otherwise the
@@ -613,12 +613,12 @@ LocalHints =
         isClickable ||= element.control? and (@getVisibleClickable element.control).length == 0
       when "body"
         isClickable ||=
-          if element == document.body and not document.hasFocus() and
+          if element == document.body and not windowIsFocused() and
               window.innerWidth > 3 and window.innerHeight > 3 and
               document.body?.tagName.toLowerCase() != "frameset"
             reason = "Frame."
         isClickable ||=
-          if element == document.body and document.hasFocus() and Scroller.isScrollableElement element
+          if element == document.body and windowIsFocused() and Scroller.isScrollableElement element
             reason = "Scroll."
       when "div", "ol", "ul"
         isClickable ||=
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index 7d6fa9a0..39e8e5d8 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -8,7 +8,8 @@ normalMode = null
 
 # We track whther the current window has the focus or not.
 windowIsFocused = do ->
-  windowHasFocus = document.hasFocus()
+  windowHasFocus = null
+  DomUtils.documentReady -> windowHasFocus = document.hasFocus()
   window.addEventListener "focus", (event) -> windowHasFocus = true if event.target == window; true
   window.addEventListener "blur", (event) -> windowHasFocus = false if event.target == window; true
   -> windowHasFocus
-- 
cgit v1.2.3