Merge pull request #2100 from smblott-github/rework-ui-component-focus-handling

Rework ui component focus handling

11 files changed, 102 insertions, 155 deletions

diff --git a/content_scripts/hud.coffee b/content_scripts/hud.coffee
index bbf7c5e9..0bd1df3e 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"
     @tween.fade 1.0, 150
 
   search: (data) ->
@@ -50,19 +50,16 @@ HUD =
     clearTimeout(@_showForDurationTimerId)
     @tween.stop()
     if immediate
-      unless updateIndicator
-        @hudUI.hide()
-        @hudUI.postMessage {name: "hide"}
-      Mode.setIndicator() if updateIndicator
+      if updateIndicator then Mode.setIndicator() else @hudUI.hide()
     else
       @tween.fade 0, 150, => @hide true, updateIndicator
 
   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/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/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..0989bbc9 100644
--- a/content_scripts/ui_component.coffee
+++ b/content_scripts/ui_component.coffee
@@ -2,11 +2,16 @@ class UIComponent
   uiComponentIsReady: false
   iframeElement: null
   iframePort: null
-  showing: null
+  showing: false
+  iframeFrameId: null
   options: null
   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"
@@ -27,10 +32,7 @@ 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
+    @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,74 +47,54 @@ 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.
+                # 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
+
+  # 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"
+      @toggleIframeElementClasses "vimiumUIComponentHidden", "vimiumUIComponentVisible"
+      @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
+      @toggleIframeElementClasses "vimiumUIComponentVisible", "vimiumUIComponentHidden"
+      if @options?.focus
+        @iframeElement.blur()
+        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.
 
   # 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 +117,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..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
@@ -120,8 +121,11 @@ class NormalMode extends KeyHandlerMode
         Are you sure you want to continue?"""
 
     if registryEntry.topFrame
+      # The Vomnibar (a top-frame command) cannot coexist with the help dialog (it causes focus issues).
+      sourceFrameId = if 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
@@ -156,8 +160,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).
-    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()
@@ -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..02ce97c5 100644
--- a/content_scripts/vomnibar.coffee
+++ b/content_scripts/vomnibar.coffee
@@ -49,13 +49,7 @@ Vomnibar =
 
   init: ->
     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
-
+      @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.
@@ -65,7 +59,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..990fa063 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: -> @hide()
 
   #
   # Advanced commands are hidden by default so they don't overwhelm new and casual users.
@@ -77,15 +68,14 @@ 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
+      Frame.postMessage "registerFrame"
+    when "hidden"
+      # Unregister the frame, so that it's not available for `gf` or link hints.
+      Frame.postMessage "unregisterFrame"
 
 root = exports ? window
 root.HelpDialog = HelpDialog
diff --git a/pages/hud.coffee b/pages/hud.coffee
index 17ac52be..fcc7b1dd 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 = ""
@@ -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: ""
@@ -96,8 +92,5 @@ 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..488ff0ed 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
@@ -20,8 +18,8 @@ UIComponentServer =
 
   registerHandler: (@handleMessage) ->
 
-  postMessage: (message) ->
-    @ownerPagePort?.postMessage message
+  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
@@ -34,7 +32,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