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 +-
 5 files changed, 67 insertions(+), 100 deletions(-)

(limited to 'content_scripts')

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
-- 
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 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

(limited to 'content_scripts')

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) ->
-- 
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(-)

(limited to 'content_scripts')

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(-)

(limited to 'content_scripts')

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(-)

(limited to 'content_scripts')

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 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

(limited to 'content_scripts')

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()
-- 
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 +---
 3 files changed, 3 insertions(+), 7 deletions(-)

(limited to 'content_scripts')

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.
-- 
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 -
 1 file changed, 1 deletion(-)

(limited to 'content_scripts')

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"
-- 
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 ++++------
 2 files changed, 20 insertions(+), 23 deletions(-)

(limited to 'content_scripts')

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()
-- 
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(-)

(limited to 'content_scripts')

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