Search completion; rework handling when no selection.

This makes the behaviour consistent between custom and non-custom
searches when there is no selection in the vomnibar (omni-mode only).

Approach:
   - When there is no selection in the vomnibar, we *always* send the
     query to to background completer (that is, both for default
     searches and for custom searches) and ask the completer to provide
     only the primary suggestion.  The primary suggestion is just what
     you get if you append the query terms to the search URL (default or
     custom).  We then immediately open the first response.

The round trip for default searches isn't strictly necessary.  However,
this uniform approach disentangles some nasty logic in the vomnibar when
we're trying to handle several cases (default or custom search, with or
without prompted text, with or without any suggestions at all).  The
extra round trip simplifies the logic to such a great extend that it's
worth it.

3 files changed, 73 insertions, 73 deletions

diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee
index 02030444..bf88f10e 100644
--- a/background_scripts/completion.coffee
+++ b/background_scripts/completion.coffee
@@ -402,7 +402,7 @@ class SearchEngineCompleter
         handler: "keywords"
         keywords: key for own key of engines
 
-  filter: ({ queryTerms, query, engine }, onComplete) ->
+  filter: ({ queryTerms, query, engine, fetchOnlyThePrimarySuggestion }, onComplete) ->
     [ primarySuggestion, removePrimarySuggestion ] = [ null, false ]
 
     { custom, searchUrl, description } =
@@ -455,29 +455,27 @@ class SearchEngineCompleter
               # And the URL suffix (which must contain the query part) matches the current query.
               RankingUtils.matches queryTerms, suggestion.url[engine.searchUrlPrefix.length..])
 
-        # If we've delivered suggestions from a completion engine, then we can strip out the primary
-        # suggestion.
-        if removePrimarySuggestion
-          suggestions = suggestions.filter (suggestion) -> suggestion != primarySuggestion
-
-        suggestions
-
-    # For custom search engines, we add a single, top-ranked entry for the unmodified query.  This
-    # suggestion appears at the top of the list.  This is the primary suggestion.
-    if custom
-      primarySuggestion = new Suggestion
-        queryTerms: queryTerms
-        type: description
-        url: Utils.createSearchUrl queryTerms, searchUrl
-        title: queryTerms.join " "
-        relevancy: 1
-        insertText: query
-        # We suppress the leading keyword, for example "w something" becomes "something" in the vomnibar.
-        suppressLeadingKeyword: true
-        # Toggles for the legacy behaviour.
-        autoSelect: not useExclusiveVomnibar
-        forceAutoSelect: not useExclusiveVomnibar
-        highlightTerms: not useExclusiveVomnibar
+        if fetchOnlyThePrimarySuggestion
+          suggestions.filter (suggestion) -> suggestion == primarySuggestion
+        else if removePrimarySuggestion
+          suggestions.filter (suggestion) -> suggestion != primarySuggestion
+        else
+          suggestions
+
+    primarySuggestion = new Suggestion
+      queryTerms: queryTerms
+      type: description
+      url: Utils.createSearchUrl queryTerms, searchUrl
+      title: queryTerms.join " "
+      relevancy: relevancy
+      insertText: if useExclusiveVomnibar then query else null
+      # We suppress the leading keyword for custom search engines; for example, "w query terms" becomes just
+      # "query terms" in the vomnibar.
+      suppressLeadingKeyword: custom
+      # Toggles for the legacy behaviour.
+      autoSelect: not useExclusiveVomnibar
+      forceAutoSelect: not useExclusiveVomnibar
+      highlightTerms: not useExclusiveVomnibar
 
     mkSuggestion = (suggestion) ->
       new Suggestion
@@ -492,27 +490,27 @@ class SearchEngineCompleter
 
     deliverCompletions = (onComplete, completions, args...) ->
       # Make the first suggestion float to the top of the vomnibar (except if we would be competing with the
-      # domain completer).
+      # domain completer, which also assigns a relevancy of 1).
       if 0 < completions.length
-        if custom or (1 < queryTerms.length or /\S\s/.test query)
-          completions[0].relevancy = 1
+        completions[0].relevancy = 1 if custom or (1 < queryTerms.length or /\S\s/.test query)
       onComplete completions, args...
 
     # If we have cached suggestions, then we can bundle them immediately (otherwise we'll have to fetch them
     # asynchronously).
     cachedSuggestions = null
-    cachedSuggestions = CompletionSearch.complete searchUrl, queryTerms if haveCompletionEngine
+    cachedSuggestions = CompletionSearch.complete searchUrl, queryTerms if haveCompletionEngine and not fetchOnlyThePrimarySuggestion
 
     suggestions =
-      if haveCompletionEngine and cachedSuggestions? and 0 < cachedSuggestions.length
+      if haveCompletionEngine and cachedSuggestions? and 0 < cachedSuggestions.length and not fetchOnlyThePrimarySuggestion
         cachedSuggestions.map mkSuggestion
-      else if custom
+      else if custom or fetchOnlyThePrimarySuggestion
         [ primarySuggestion ]
       else
         []
 
-    if queryTerms.length == 0 or cachedSuggestions? or not haveCompletionEngine
-      # There is no prospect of adding further completions.
+    if queryTerms.length == 0 or cachedSuggestions? or not haveCompletionEngine or fetchOnlyThePrimarySuggestion
+      # There is no prospect of adding further completions, or further completions will not be used (eg.
+      # because the vomnibar is closing and we've been asked for the primary suggestion only).
       deliverCompletions onComplete, suggestions, { filter, continuation: null }
     else
       # Post initial suggestions, then deliver further completions asynchronously, as a continuation.
diff --git a/pages/vomnibar.coffee b/pages/vomnibar.coffee
index b301bec3..c17a14f5 100644
--- a/pages/vomnibar.coffee
+++ b/pages/vomnibar.coffee
@@ -63,6 +63,7 @@ class VomnibarUI
     @postHideCallback = null
 
   reset: ->
+    @fetchOnlyThePrimarySuggestion = false
     @clearUpdateTimer()
     @completionList.style.display = ""
     @input.value = ""
@@ -191,32 +192,26 @@ class VomnibarUI
       @updateSelection()
     else if (action == "enter")
       if @selection == -1
-        # <Alt>/<Meta> includes prompted text in the query (normally it is not included).
-        #
-        # FIXME(smblott).  This is a terrible binding. <Ctrl-Enter> would be better, but that's already being
-        # used.  We need a better UX around how to include the prompted text in the query. <Right> then
-        # <Enter> works, but that's ugly too.
-        window.getSelection().collapseToEnd() if event.altKey or event.metaKey
-        # The user has not selected a suggestion.
-        query = @getInputWithoutPromptedText().trim()
-        # <Enter> on an empty vomnibar is a no-op.
-        return unless 0 < query.length
-        if @suppressedLeadingKeyword?
-          # This is a custom search engine completion.  The text in the input might not correspond to any of
-          # the completions.  So we fire off the query to the background page and use the completion at the
-          # top of the list (which will be the right one).
-          @update true, =>
-            if @completions[0]
+        switch @completer.name
+          when "omni"
+            return unless 0 < @getInputWithoutPromptedText().trim().length
+            # We ask the SearchEngineCompleter for its primary suggestion and launch it.  In some cases, this
+            # adds an extra (and not strictly necessary) round trip to the background completer.  However,
+            # this approach allows all of the various search-engine modes to be handled in a uniform way.
+            @fetchOnlyThePrimarySuggestion = true
+            @update true, =>
               completion = @completions[0]
-              @hide -> completion.performAction openInNewTab
-        else
-          # If the user types something and hits enter without selecting a completion from the list, then try
-          # to open their query as a URL directly. If it doesn't look like a URL, then use the default search
-          # engine.
-          @hide ->
-            chrome.runtime.sendMessage
-              handler: if openInNewTab then "openUrlInNewTab" else "openUrlInCurrentTab"
-              url: query
+              @hide -> completion?.performAction openInNewTab
+          else
+            # We're in "bookmark" or "tab" mode.
+            # If the user types something and hits enter without selecting a completion from the list, then try
+            # to open their query as a URL directly. If it doesn't look like a URL, then use the default search
+            # engine.
+            query = @getInputValueAsQuery()
+            @hide ->
+              chrome.runtime.sendMessage
+                handler: if openInNewTab then "openUrlInNewTab" else "openUrlInCurrentTab"
+                url: query
       else
         completion = @completions[@selection]
         @hide -> completion.performAction openInNewTab
@@ -283,17 +278,21 @@ class VomnibarUI
     (if @suppressedLeadingKeyword? then @suppressedLeadingKeyword + " " else "") + @getInputWithoutPromptedText()
 
   updateCompletions: (callback = null) ->
-    @completer.filter @getInputValueAsQuery(), (response) =>
-      { results, mayCacheResults } = response
-      @completions = results
-      # Update completion list with the new suggestions.
-      @completionList.innerHTML = @completions.map((completion) -> "<li>#{completion.html}</li>").join("")
-      @completionList.style.display = if @completions.length > 0 then "block" else ""
-      @selection = Math.min @completions.length - 1, Math.max @initialSelectionValue, @selection
-      @previousAutoSelect = null if @completions[0]?.autoSelect and @completions[0]?.forceAutoSelect
-      @updateSelection()
-      @addPromptedText response
-      callback?()
+    @completer.filter
+      query: @getInputValueAsQuery()
+      fetchOnlyThePrimarySuggestion: @fetchOnlyThePrimarySuggestion
+      mayUseVomnibarCache: not @fetchOnlyThePrimarySuggestion
+      callback: (response) =>
+        { results, mayCacheResults } = response
+        @completions = results
+        # Update completion list with the new suggestions.
+        @completionList.innerHTML = @completions.map((completion) -> "<li>#{completion.html}</li>").join("")
+        @completionList.style.display = if @completions.length > 0 then "block" else ""
+        @selection = Math.min @completions.length - 1, Math.max @initialSelectionValue, @selection
+        @previousAutoSelect = null if @completions[0]?.autoSelect and @completions[0]?.forceAutoSelect
+        @updateSelection()
+        @addPromptedText response
+        callback?()
 
   updateOnInput: =>
     @completer.cancel()
@@ -387,21 +386,24 @@ class BackgroundCompleter
           # Handle the message, but only if it hasn't arrived too late.
           @mostRecentCallback msg if msg.id == @messageId
 
-  filter: (query, @mostRecentCallback) ->
+  filter: (request) ->
+    [ query, mayUseVomnibarCache, @mostRecentCallback ] = [ request.query, request.mayUseVomnibarCache,  request.callback ]
     cacheKey = query.ltrim().split(/\s+/).join " "
 
-    if cacheKey of @cache
+    if cacheKey of @cache and request.mayUseVomnibarCache
       console.log "cache hit:", "-#{cacheKey}-" if @debug
       @mostRecentCallback @cache[cacheKey]
     else
       console.log "cache miss:", "-#{cacheKey}-" if @debug
-      @port.postMessage
+      @port.postMessage extend request,
         handler: "filter"
         name: @name
         id: @messageId = Utils.createUniqueId()
         queryTerms: query.trim().split(/\s+/).filter (s) -> 0 < s.length
-        query: query
         cacheKey: cacheKey
+        # We don't send these keys.
+        callback: null
+        mayUseVomnibarCache: null
 
   reset: ->
     [ @keywords, @cache ] = [ [], {} ]
diff --git a/tests/dom_tests/vomnibar_test.coffee b/tests/dom_tests/vomnibar_test.coffee
index e32c050d..380175f3 100644
--- a/tests/dom_tests/vomnibar_test.coffee
+++ b/tests/dom_tests/vomnibar_test.coffee
@@ -14,7 +14,7 @@ context "Keep selection within bounds",
     oldGetCompleter = vomnibarFrame.Vomnibar.getCompleter.bind vomnibarFrame.Vomnibar
     stub vomnibarFrame.Vomnibar, 'getCompleter', (name) =>
       completer = oldGetCompleter name
-      stub completer, 'filter', (query, callback) => callback results: @completions
+      stub completer, 'filter', ({ callback }) => callback results: @completions
       completer
 
     # Shoulda.js doesn't support async tests, so we have to hack around.