Activate vomnibar in window.top; fix race condition on close.

2 files changed, 25 insertions, 13 deletions

diff --git a/content_scripts/ui_component.coffee b/content_scripts/ui_component.coffee
index 185d525a..727f6c27 100644
--- a/content_scripts/ui_component.coffee
+++ b/content_scripts/ui_component.coffee
@@ -19,7 +19,7 @@ class UIComponent
     # NOTE(smblott) This is correct for the vomnibar, but might be incorrect (and need to be revisited) for
     # other UI components.
     chrome.runtime.onMessage.addListener (request) =>
-      @hide false if @showing and request.name == "frameFocused" and request.focusFrameId != frameId
+      @postMessage "hide" if @showing and request.name == "frameFocused" and request.focusFrameId != frameId
       false # Free up response handler.
 
   # Open a port and pass it to the iframe via window.postMessage.
@@ -55,19 +55,29 @@ class UIComponent
     @showing = true
 
   hide: (focusWindow = true)->
-    @iframeElement.classList.remove "vimiumUIComponentShowing"
-    @iframeElement.classList.add "vimiumUIComponentHidden"
+    @refocusSourceFrame @options?.sourceFrameId if focusWindow and @options?.sourceFrameId?
     window.removeEventListener "focus", @onFocus if @onFocus
     @onFocus = null
-    if focusWindow and @options?.sourceFrameId?
-      chrome.runtime.sendMessage
-        handler: "sendMessageToFrames"
-        message:
-          name: "focusFrame"
-          frameId: @options.sourceFrameId
-          highlight: true # true for debugging; should be false when live.
+    @iframeElement.classList.remove "vimiumUIComponentShowing"
+    @iframeElement.classList.add "vimiumUIComponentHidden"
     @options = null
     @showing = false
 
+  # Refocus the frame from which the UI component was opened.
+  # After hiding the UI component, Chrome refocuses the containing frame. To avoid a race condition, we need
+  # to wait until that frame receives the focus, before then focusing the frame which should now have the
+  # focus.
+  refocusSourceFrame: (sourceFrameId) ->
+    window.addEventListener "focus", handler = (event) ->
+      if event.target == window
+        window.removeEventListener "focus", handler
+        chrome.runtime.sendMessage
+          handler: "sendMessageToFrames"
+          message:
+            name: "focusFrame"
+            frameId: sourceFrameId
+            highlight: false
+            highlightOnlyIfNotTop: true
+
 root = exports ? window
 root.UIComponent = UIComponent
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index 072789a4..dccdfe76 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -173,7 +173,7 @@ initializePreDomReady = ->
     showUpgradeNotification: (request) -> HUD.showUpgradeNotification(request.version)
     showHUDforDuration: (request) -> HUD.showForDuration request.text, request.duration
     toggleHelpDialog: (request) -> toggleHelpDialog(request.dialogHtml, request.frameId)
-    focusFrame: (request) -> if (frameId == request.frameId) then focusThisFrame(request.highlight)
+    focusFrame: (request) -> if (frameId == request.frameId) then focusThisFrame request
     refreshCompletionKeys: refreshCompletionKeys
     getScrollPosition: -> scrollX: window.scrollX, scrollY: window.scrollY
     setScrollPosition: (request) -> setScrollPosition request.scrollX, request.scrollY
@@ -293,7 +293,7 @@ setScrollPosition = (scrollX, scrollY) ->
 #
 # Called from the backend in order to change frame focus.
 #
-window.focusThisFrame = (shouldHighlight) ->
+window.focusThisFrame = (request) ->
   if window.innerWidth < 3 or window.innerHeight < 3
     # This frame is too small to focus. Cancel and tell the background frame to focus the next one instead.
     # This affects sites like Google Inbox, which have many tiny iframes. See #1317.
@@ -301,7 +301,9 @@ window.focusThisFrame = (shouldHighlight) ->
     chrome.runtime.sendMessage({ handler: "nextFrame", frameId: frameId })
     return
   window.focus()
-  if (document.body && shouldHighlight)
+  shouldHighlight = request.highlight
+  shouldHighlight ||= request.highlightOnlyIfNotTop and not DomUtils.isTopFrame()
+  if document.body and shouldHighlight
     borderWas = document.body.style.border
     document.body.style.border = '5px solid yellow'
     setTimeout((-> document.body.style.border = borderWas), 200)