Merge pull request #2101 from smblott-github/rework-help-dialog

Move help dialog to top frame

8 files changed, 30 insertions, 42 deletions

diff --git a/background_scripts/commands.coffee b/background_scripts/commands.coffee
index 2d901b15..2999ebfc 100644
--- a/background_scripts/commands.coffee
+++ b/background_scripts/commands.coffee
@@ -295,7 +295,7 @@ defaultKeyMappings =
 # If the noRepeat and repeatLimit options are both specified, then noRepeat takes precedence.
 commandDescriptions =
   # Navigating the current page
-  showHelp: ["Show help", { background: true, noRepeat: true }]
+  showHelp: ["Show help", { topFrame: true, noRepeat: true }]
   scrollDown: ["Scroll down"]
   scrollUp: ["Scroll up"]
   scrollLeft: ["Scroll left"]
diff --git a/background_scripts/main.coffee b/background_scripts/main.coffee
index 5c6e8e19..4aa2fc41 100644
--- a/background_scripts/main.coffee
+++ b/background_scripts/main.coffee
@@ -82,8 +82,7 @@ chrome.webNavigation.onHistoryStateUpdated.addListener onURLChange # history.pus
 chrome.webNavigation.onReferenceFragmentUpdated.addListener onURLChange # Hash changed.
 
 # Retrieves the help dialog HTML template from a file, and populates it with the latest keybindings.
-# This is called by options.coffee.
-root.helpDialogHtml = (showUnboundCommands, showCommandNames, customTitle) ->
+getHelpDialogHtml = ({showUnboundCommands, showCommandNames, customTitle}) ->
   commandsToKey = {}
   for own key of Commands.keyToCommandRegistry
     command = Commands.keyToCommandRegistry[key].command
@@ -209,8 +208,6 @@ BackgroundCommands =
   openCopiedUrlInCurrentTab: (request) -> TabOperations.openUrlInCurrentTab extend request, url: Clipboard.paste()
   openCopiedUrlInNewTab: (request) -> @createTab extend request, url: Clipboard.paste()
   togglePinTab: ({tab}) -> chrome.tabs.update tab.id, {pinned: !tab.pinned}
-  showHelp: ({tab, frameId}) ->
-    chrome.tabs.sendMessage tab.id, {name: "toggleHelpDialog", dialogHtml: helpDialogHtml(), frameId}
   moveTabLeft: moveTab
   moveTabRight: moveTab
   nextFrame: ({count, frameId, tabId}) ->
@@ -402,6 +399,7 @@ portHandlers =
 
 sendRequestHandlers =
   runBackgroundCommand: (request) -> BackgroundCommands[request.registryEntry.command] request
+  getHelpDialogHtml: getHelpDialogHtml
   # getCurrentTabUrl is used by the content scripts to get their full URL, because window.location cannot help
   # with Chrome-specific URLs like "view-source:http:..".
   getCurrentTabUrl: ({tab}) -> tab.url
diff --git a/content_scripts/hud.coffee b/content_scripts/hud.coffee
index 0bd1df3e..62bcf03f 100644
--- a/content_scripts/hud.coffee
+++ b/content_scripts/hud.coffee
@@ -23,13 +23,13 @@ HUD =
     @_showForDurationTimerId = setTimeout((=> @hide()), duration)
 
   show: (text) ->
-    return unless @isReady()
+    @init()
     clearTimeout(@_showForDurationTimerId)
     @hudUI.activate {name: "show", text}
     @tween.fade 1.0, 150
 
   showFindMode: (@findMode = null) ->
-    return unless @isReady()
+    @init()
     @hudUI.activate name: "showFindMode"
     @tween.fade 1.0, 150
 
@@ -46,7 +46,7 @@ HUD =
   # If :updateIndicator is truthy, then we also refresh the mode indicator.  The only time we don't update the
   # mode indicator, is when hide() is called for the mode indicator itself.
   hide: (immediate = false, updateIndicator = true) ->
-    return unless @tween? and @isReady()
+    return unless @hudUI?.uiComponentIsReady
     clearTimeout(@_showForDurationTimerId)
     @tween.stop()
     if immediate
@@ -80,8 +80,6 @@ HUD =
     @findMode.exit()
     postExit?()
 
-  isReady: -> @hudUI?.uiComponentIsReady
-
 class Tween
   opacity: 0
   intervalId: -1
diff --git a/content_scripts/mode.coffee b/content_scripts/mode.coffee
index 37321660..6508627e 100644
--- a/content_scripts/mode.coffee
+++ b/content_scripts/mode.coffee
@@ -69,8 +69,7 @@ class Mode
         # undefined, then the request propagates to the next mode.
         # The active indicator can also be changed with @setIndicator().
         if @options.indicator?
-          if HUD?.isReady()
-            if @options.indicator then HUD.show @options.indicator else HUD.hide true, false
+          if @options.indicator then HUD.show @options.indicator else HUD.hide true, false
           @passEventToPage
         else @continueBubbling
 
diff --git a/content_scripts/vimium_frontend.coffee b/content_scripts/vimium_frontend.coffee
index 92145303..a80093f9 100644
--- a/content_scripts/vimium_frontend.coffee
+++ b/content_scripts/vimium_frontend.coffee
@@ -121,9 +121,8 @@ 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).
+      # We never return to a UI-component frame (e.g. the help dialog), it might have lost the focus.
       sourceFrameId = if window.isVimiumUIComponent then 0 else frameId
-      HelpDialog.toggle() if HelpDialog.isShowing()
       chrome.runtime.sendMessage
         handler: "sendMessageToFrames", message: {name: "runInTopFrame", sourceFrameId, registryEntry}
     else if registryEntry.background
@@ -142,10 +141,6 @@ installModes = ->
 initializeOnEnabledStateKnown = Utils.makeIdempotent ->
   installModes()
 
-initializeUIComponents = Utils.makeIdempotent -> DomUtils.documentReady ->
- HUD.init()
- Vomnibar.init() if DomUtils.isTopFrame()
-
 #
 # Complete initialization work that should be done prior to DOMReady.
 #
@@ -155,7 +150,6 @@ initializePreDomReady = ->
   checkIfEnabledForUrl document.hasFocus()
 
   requestHandlers =
-    toggleHelpDialog: (request) -> if frameId == request.frameId then HelpDialog.toggle request.dialogHtml
     focusFrame: (request) -> if (frameId == request.frameId) then focusThisFrame request
     getScrollPosition: (ignoredA, ignoredB, sendResponse) ->
       sendResponse scrollX: window.scrollX, scrollY: window.scrollY if frameId == 0
@@ -458,13 +452,8 @@ checkIfEnabledForUrl = do ->
     {isEnabledForUrl, passKeys, frameIsFocused} = response
     initializeOnEnabledStateKnown()
     normalMode.setPassKeys passKeys
-    # Initialize UI components, if necessary. We only initialize these once we know that Vimium is enabled;
-    # see #1838.  We need to check this every time so that we can change state from disabled to enabled.
-    if isEnabledForUrl
-      initializeUIComponents() if frameIsFocused
-    else
-      # Hide the HUD if we're not enabled.
-      HUD.hide() if HUD.isReady()
+    # Hide the HUD if we're not enabled.
+    HUD.hide true, false unless isEnabledForUrl
 
   (frameIsFocused = windowIsFocused()) ->
     Frame.postMessage "isEnabledForUrl", {frameIsFocused, url: window.location.toString()}
@@ -626,19 +615,25 @@ enterFindMode = ->
   Marks.setPreviousPosition()
   new FindMode()
 
-# If we are in the help dialog iframe, HelpDialog is already defined with the necessary functions.
+window.showHelp = (sourceFrameId) ->
+  chrome.runtime.sendMessage handler: "getHelpDialogHtml", (response) ->
+    HelpDialog.toggle {sourceFrameId, html: response}
+
+# If we are in the help dialog iframe, then HelpDialog is already defined with the necessary functions.
 window.HelpDialog ?=
   helpUI: null
   isShowing: -> @helpUI?.showing
+  abort: -> @helpUI.hide false if @isShowing()
 
-  toggle: (html) ->
+  toggle: (request) ->
     @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}
+      # On the options page, we allow the help dialog to blur, elsewhere we do not.  This allows users to view
+      # the help dialog while typing in the key-mappings input.
+      @helpUI.activate extend request,
+        name: "activate", focus: true, allowBlur: window.isVimiumOptionsPage ? false
 
 initializePreDomReady()
 DomUtils.documentReady initializeOnDomReady
diff --git a/content_scripts/vomnibar.coffee b/content_scripts/vomnibar.coffee
index 02ce97c5..cbd2892c 100644
--- a/content_scripts/vomnibar.coffee
+++ b/content_scripts/vomnibar.coffee
@@ -48,8 +48,7 @@ Vomnibar =
   }
 
   init: ->
-    unless @vomnibarUI?
-      @vomnibarUI = new UIComponent "pages/vomnibar.html", "vomnibarFrame", ->
+    @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.
@@ -58,8 +57,9 @@ Vomnibar =
   #   newTab      - Optional, boolean. Whether to open the result in a new tab.
   open: (sourceFrameId, options) ->
     @init()
-    if @vomnibarUI?.uiComponentIsReady
-      @vomnibarUI.activate extend options, { name: "activate", sourceFrameId, focus: true }
+    # The Vomnibar cannot coexist with the help dialog (it causes focus issues).
+    HelpDialog.abort()
+    @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 990fa063..b2d33f95 100644
--- a/pages/help_dialog.coffee
+++ b/pages/help_dialog.coffee
@@ -1,9 +1,5 @@
-# This overrides the HelpDialog implementation in vimium_frontend.coffee, which prevents us from being able
-# to spawn a help dialog within the help dialog UIComponent. As such, we need to provide all the properties
-# that we expect on the normal HelpDialog implementation.
-#
-# NOTE(mrmr1993): In the future, we can move to a single help dialog UIComponent per tab (ie. in the
-#   top-level frame), and then we don't need to be concerned about nested help dialog frames.
+# This overrides the HelpDialog implementation in vimium_frontend.coffee.  We provide aliases for the two
+# HelpDialog methods required by normalMode (isShowing() and toggle()).
 HelpDialog =
   dialogElement: null
   isShowing: -> true
diff --git a/pages/options.coffee b/pages/options.coffee
index 3e1843a7..883405e6 100644
--- a/pages/options.coffee
+++ b/pages/options.coffee
@@ -234,7 +234,9 @@ initOptionsPage = ->
     event.preventDefault()
 
   activateHelpDialog = ->
-    HelpDialog.toggle chrome.extension.getBackgroundPage().helpDialogHtml true, true, "Command Listing"
+    request = showUnboundCommands: true, showCommandNames: true, customTitle: "Command Listing"
+    chrome.runtime.sendMessage extend(request, handler: "getHelpDialogHtml"), (response) ->
+      HelpDialog.toggle {html: response}
 
   saveOptions = ->
     Option.saveOptions()