diff options
| -rw-r--r-- | background_scripts/completion.coffee | 166 | ||||
| -rw-r--r-- | background_scripts/settings.coffee | 1 | ||||
| -rw-r--r-- | pages/options.html | 3 | ||||
| -rw-r--r-- | pages/vomnibar.coffee | 14 | ||||
| -rw-r--r-- | pages/vomnibar.css | 1 | ||||
| -rw-r--r-- | tests/unit_tests/completion_test.coffee | 8 |
6 files changed, 91 insertions, 102 deletions
diff --git a/background_scripts/completion.coffee b/background_scripts/completion.coffee index 02c5478a..bae73b8d 100644 --- a/background_scripts/completion.coffee +++ b/background_scripts/completion.coffee @@ -43,23 +43,33 @@ class Suggestion # or @relevancyFunction. @relevancy ?= @relevancyFunction this - generateHtml: -> + generateHtml: (request) -> return @html if @html relevancyHtml = if @showRelevancy then "<span class='relevancy'>#{@computeRelevancy()}</span>" else "" insertTextClass = if @insertText then "vomnibarInsertText" else "vomnibarNoInsertText" insertTextIndicator = "↪" # A right hooked arrow. + @title = @insertText if @insertText and request.isCustomSearch # NOTE(philc): We're using these vimium-specific class names so we don't collide with the page's CSS. @html = - """ - <div class="vimiumReset vomnibarTopHalf"> - <span class="vimiumReset vomnibarSource #{insertTextClass}">#{insertTextIndicator}</span><span class="vimiumReset vomnibarSource">#{@type}</span> - <span class="vimiumReset vomnibarTitle">#{@highlightQueryTerms Utils.escapeHtml @title}</span> - </div> - <div class="vimiumReset vomnibarBottomHalf"> - <span class="vimiumReset vomnibarSource vomnibarNoInsertText">#{insertTextIndicator}</span><span class="vimiumReset vomnibarUrl">#{@highlightUrlTerms Utils.escapeHtml @shortenUrl()}</span> - #{relevancyHtml} - </div> - """ + if request.isCustomSearch + """ + <div class="vimiumReset vomnibarTopHalf"> + <span class="vimiumReset vomnibarSource #{insertTextClass}">#{insertTextIndicator}</span><span class="vimiumReset vomnibarSource">#{@type}</span> + <span class="vimiumReset vomnibarTitle">#{@highlightQueryTerms Utils.escapeHtml @title}</span> + #{relevancyHtml} + </div> + """ + else + """ + <div class="vimiumReset vomnibarTopHalf"> + <span class="vimiumReset vomnibarSource #{insertTextClass}">#{insertTextIndicator}</span><span class="vimiumReset vomnibarSource">#{@type}</span> + <span class="vimiumReset vomnibarTitle">#{@highlightQueryTerms Utils.escapeHtml @title}</span> + </div> + <div class="vimiumReset vomnibarBottomHalf"> + <span class="vimiumReset vomnibarSource vomnibarNoInsertText">#{insertTextIndicator}</span><span class="vimiumReset vomnibarUrl">#{@highlightUrlTerms Utils.escapeHtml @shortenUrl()}</span> + #{relevancyHtml} + </div> + """ # Use neat trick to snatch a domain (http://stackoverflow.com/a/8498668). getUrlRoot: (url) -> @@ -293,7 +303,7 @@ class DomainCompleter queryTerms: queryTerms type: "domain" url: domains[0]?[0] ? "" # This is the URL or an empty string, but not null. - relevancy: 1 + relevancy: 2.0 ].filter (s) -> 0 < s.url.length # Returns a list of domains of the form: [ [domain, relevancy], ... ] @@ -414,7 +424,7 @@ class SearchEngineCompleter preprocessRequest: (request) -> @searchEngines.use (engines) => { queryTerms, query } = request - request.searchEngines = engines + extend request, searchEngines: engines, keywords: key for own key of engines keyword = queryTerms[0] # Note. For a keyword "w", we match "w search terms" and "w ", but not "w" on its own. if keyword and engines[keyword] and (1 < queryTerms.length or /\S\s/.test query) @@ -422,6 +432,7 @@ class SearchEngineCompleter queryTerms: queryTerms[1..] keyword: keyword engine: engines[keyword] + isCustomSearch: true refresh: (port) -> @previousSuggestions = {} @@ -454,92 +465,77 @@ class SearchEngineCompleter filter: (request, onComplete) -> { queryTerms, query, engine } = request + return onComplete [] unless engine - { custom, searchUrl, description } = - if engine - { keyword, searchUrl, description } = engine - extend request, { searchUrl, customSearchMode: true } - custom: true - searchUrl: searchUrl - description: description - else - custom: false - searchUrl: Settings.get "searchUrl" - description: "search" - - return onComplete [] unless custom or 0 < queryTerms.length + { keyword, searchUrl, description } = engine + extend request, searchUrl, customSearchMode: true - factor = Math.max 0.0, Math.min 1.0, Settings.get "omniSearchWeight" - haveCompletionEngine = (0.0 < factor or custom) and CompletionSearch.haveCompletionEngine searchUrl + factor = 0.5 + haveCompletionEngine = CompletionSearch.haveCompletionEngine searchUrl # This filter is applied to all of the suggestions from all of the completers, after they have been # aggregated by the MultiCompleter. filter = (suggestions) -> - if custom and haveCompletionEngine - # We only accept suggestions: - # - from this completer, or - # - from other completers, but then only if their URL matches this search engine and matches this - # query (that is only if their URL could have been generated by this search engine). - suggestions.filter (suggestion) -> - suggestion.type == description or - # This is a suggestion for the same search engine. - (suggestion.url.startsWith(engine.searchUrlPrefix) and - # And the URL suffix (which must contain the query part) matches the current query. - RankingUtils.matches queryTerms, suggestion.url[engine.searchUrlPrefix.length..]) - - else if not custom - # Filter out any suggestion which is just what the user would get if they hit <Enter> anyway. For - # example, don't offer "https://www.google.com/search?q=vimium" if the query is "vimium". - defaultUrl = Utils.createSearchUrl queryTerms, searchUrl - defaultQuery = queryTerms.join " " - suggestions.filter (suggestion) -> Utils.extractQuery(searchUrl, suggestion.url) != defaultQuery - else - suggestions + suggestions.filter (suggestion) -> + # We only keep suggestions which either *were* generated by this search engine, or *could have + # been* generated by this search engine (and match the current query). + suggestion.isSearchSuggestion or suggestion.isCustomSearch or + ( + terms = Utils.extractQuery searchUrl, suggestion.url + terms and RankingUtils.matches queryTerms, terms + ) # If a previous suggestion still matches the query, then we keep it (even if the completion engine may not # return it for the current query). This allows the user to pick suggestions by typing fragments of their # text, without regard to whether the completion engine can complete the actual text of the query. previousSuggestions = - for url, suggestion of @previousSuggestions - continue unless RankingUtils.matches queryTerms, suggestion.title - # Reset various fields, they may not be correct wrt. the current query. - extend suggestion, relevancy: null, html: null, queryTerms: queryTerms - suggestion.relevancy = null - suggestion + if queryTerms.length == 0 + [] + else + for url, suggestion of @previousSuggestions + continue unless RankingUtils.matches queryTerms, suggestion.title + # Reset various fields, they may not be correct wrt. the current query. + extend suggestion, relevancy: null, html: null, queryTerms: queryTerms + suggestion.relevancy = null + suggestion primarySuggestion = new Suggestion queryTerms: queryTerms type: description url: Utils.createSearchUrl queryTerms, searchUrl title: queryTerms.join " " - relevancy: 1 - autoSelect: custom - highlightTerms: not haveCompletionEngine + relevancy: 2.0 + autoSelect: true + highlightTerms: false isSearchSuggestion: true + isPrimarySuggestion: true - mkSuggestion = (suggestion) => - url = Utils.createSearchUrl suggestion, searchUrl - @previousSuggestions[url] = new Suggestion - queryTerms: queryTerms - type: description - url: url - title: suggestion - insertText: suggestion - highlightTerms: false - highlightTermsExcludeUrl: true - isCustomSearch: custom - relevancyFunction: @computeRelevancy - relevancyData: factor + return onComplete [ primarySuggestion ], { filter } if queryTerms.length == 0 + + mkSuggestion = do => + count = 0 + (suggestion) => + url = Utils.createSearchUrl suggestion, searchUrl + @previousSuggestions[url] = new Suggestion + queryTerms: queryTerms + type: description + url: url + title: suggestion + insertText: suggestion + highlightTerms: false + highlightTermsExcludeUrl: true + isCustomSearch: true + relevancy: if ++count == 1 then 1.0 else null + relevancyFunction: @computeRelevancy cachedSuggestions = if haveCompletionEngine then CompletionSearch.complete searchUrl, queryTerms else null suggestions = previousSuggestions - suggestions.push primarySuggestion if custom - suggestions.push cachedSuggestions.map(mkSuggestion)... if custom and cachedSuggestions? + suggestions.push primarySuggestion if queryTerms.length == 0 or cachedSuggestions? or not haveCompletionEngine - # There is no prospect of adding further completions. + # There is no prospect of adding further completions, so we're done. suggestions.push cachedSuggestions.map(mkSuggestion)... if cachedSuggestions? onComplete suggestions, { filter, continuation: null } else @@ -547,20 +543,7 @@ class SearchEngineCompleter # continuation. onComplete suggestions, filter: filter - continuation: (suggestions, onComplete) => - - # We can skip querying the completion engine if any new suggestions we propose will not score highly - # enough to make the list anyway. We construct a suggestion which perfectly matches the query, and - # ask the relevancy function what score it would get. If that score is less than the score of the - # lowest-ranked suggestion from another completer (and there are already 10 suggestions), then - # there's no need to query the completion engine. - perfectRelevancyScore = @computeRelevancy new Suggestion - queryTerms: queryTerms, title: queryTerms.join(" "), relevancyData: factor - - if 10 <= suggestions.length and perfectRelevancyScore < suggestions[suggestions.length-1].relevancy - console.log "skip (cannot make the grade):", suggestions.length, query if SearchEngineCompleter.debug - return onComplete [] - + continuation: (onComplete) => CompletionSearch.complete searchUrl, queryTerms, (suggestions = []) => console.log "fetched suggestions:", suggestions.length, query if SearchEngineCompleter.debug onComplete suggestions.map mkSuggestion @@ -571,7 +554,7 @@ class SearchEngineCompleter # scores here, and those provided by other completers. # - Relevancy depends only on the title (which is the search terms), and not on the URL. Suggestion.boostRelevancyScore 0.5, - relevancyData * RankingUtils.wordRelevancy queryTerms, title, title + 0.7 * RankingUtils.wordRelevancy queryTerms, title, title postProcessSuggestions: (request, suggestions) -> return unless request.searchEngines @@ -586,9 +569,6 @@ class SearchEngineCompleter # suggestion, then custom search-engine mode should be activated. suggestion.customSearchMode = engine.keyword suggestion.title ||= suggestion.insertText - # NOTE(smblott) The following is disabled: experimentation with UI. - # suggestion.highlightTermsExcludeUrl = true - # suggestion.type = engine.description ? "custom search history" break # A completer which calls filter() on many completers, aggregates the results, ranks them, and returns the top @@ -643,7 +623,7 @@ class MultiCompleter if shouldRunContinuations jobs = new JobRunner continuations.map (continuation) -> (callback) -> - continuation suggestions, (newSuggestions) -> + continuation (newSuggestions) -> suggestions.push newSuggestions... callback() @@ -676,7 +656,7 @@ class MultiCompleter completer.postProcessSuggestions? request, suggestions for completer in @completers # Generate HTML for the remaining suggestions and return them. - suggestion.generateHtml() for suggestion in suggestions + suggestion.generateHtml request for suggestion in suggestions suggestions # Utilities which help us compute a relevancy score for a given item. diff --git a/background_scripts/settings.coffee b/background_scripts/settings.coffee index 269b4a2c..d23649ee 100644 --- a/background_scripts/settings.coffee +++ b/background_scripts/settings.coffee @@ -43,7 +43,6 @@ root.Settings = Settings = # or strings defaults: scrollStepSize: 60 - omniSearchWeight: 0.4 smoothScroll: true keyMappings: "# Insert your preferred key mappings here." linkHintCharacters: "sadfjklewcmpgh" diff --git a/pages/options.html b/pages/options.html index 8535823d..0fa5b18d 100644 --- a/pages/options.html +++ b/pages/options.html @@ -238,6 +238,8 @@ b: http://b.com/?q=%s description </tr> <!-- Vimium Labs --> + <!-- + Disabled. But we leave this code here as a template for the next time we need to introduce "Vimium Labs". <tr> <td colspan="2"><header>Vimium Labs</header></td> </tr> @@ -263,6 +265,7 @@ b: http://b.com/?q=%s description <input id="omniSearchWeight" type="number" min="0.0" max="1.0" step="0.05" />(0 to 1) </td> </tr> + --> </tbody> </table> </div> diff --git a/pages/vomnibar.coffee b/pages/vomnibar.coffee index 423ffa59..71f22900 100644 --- a/pages/vomnibar.coffee +++ b/pages/vomnibar.coffee @@ -76,7 +76,7 @@ class VomnibarUI updateSelection: -> # For custom search engines, we suppress the leading term (e.g. the "w" of "w query terms") within the # vomnibar input. - if @lastReponse.customSearchMode and not @customSearchMode? + if @lastReponse.isCustomSearch and not @customSearchMode? queryTerms = @input.value.trim().split /\s+/ @customSearchMode = queryTerms[0] @input.value = queryTerms[1..].join " " @@ -141,15 +141,23 @@ class VomnibarUI @selection = @completions.length - 1 if @selection < @initialSelectionValue @updateSelection() else if (action == "enter") - if @selection == -1 + isCustomSearchPrimarySuggestion = @completions[@selection]?.isPrimarySuggestion and @lastReponse.engine?.searchUrl? + if @selection == -1 or isCustomSearchPrimarySuggestion query = @input.value.trim() # <Enter> on an empty query is a no-op. return unless 0 < query.length + # First case (@selection == -1). # If the user types something and hits enter without selecting a completion from the list, then: # - If a search URL has been provided, then use it. This is custom search engine request. # - Otherwise, send the query to the background page, which will open it as a URL or create a # default search, as appropriate. - query = Utils.createSearchUrl query, @lastReponse.searchUrl if @lastReponse.searchUrl? + # + # Second case (isCustomSearchPrimarySuggestion). + # Alternatively, the selected completion could be the primary selection for a custom search engine. + # Because the the suggestions are updated asynchronously in omni mode, the user may have typed more + # text than that which is included in the URL associated with the primary suggestion. Therefore, to + # avoid a race condition, we construct the query from the actual contents of the input (query). + query = Utils.createSearchUrl query, @lastReponse.engine.searchUrl if isCustomSearchPrimarySuggestion @hide -> chrome.runtime.sendMessage handler: if openInNewTab then "openUrlInNewTab" else "openUrlInCurrentTab" diff --git a/pages/vomnibar.css b/pages/vomnibar.css index b1ed0252..1b19daad 100644 --- a/pages/vomnibar.css +++ b/pages/vomnibar.css @@ -126,7 +126,6 @@ #vomnibar li em { font-style: italic; } #vomnibar li em .vomnibarMatch, #vomnibar li .vomnibarTitle .vomnibarMatch { color: #333; - text-decoration: underline; } #vomnibar li.vomnibarSelected { diff --git a/tests/unit_tests/completion_test.coffee b/tests/unit_tests/completion_test.coffee index 88df0a43..4a0cf746 100644 --- a/tests/unit_tests/completion_test.coffee +++ b/tests/unit_tests/completion_test.coffee @@ -244,7 +244,7 @@ context "suggestions", url: "url" title: "title <span>" relevancyFunction: returns 1 - assert.isTrue suggestion.generateHtml().indexOf("title <span>") >= 0 + assert.isTrue suggestion.generateHtml({}).indexOf("title <span>") >= 0 should "highlight query words", -> suggestion = new Suggestion @@ -254,7 +254,7 @@ context "suggestions", title: "ninjawords" relevancyFunction: returns 1 expected = "<span class='vomnibarMatch'>ninj</span>a<span class='vomnibarMatch'>words</span>" - assert.isTrue suggestion.generateHtml().indexOf(expected) >= 0 + assert.isTrue suggestion.generateHtml({}).indexOf(expected) >= 0 should "highlight query words correctly when whey they overlap", -> suggestion = new Suggestion @@ -264,7 +264,7 @@ context "suggestions", title: "ninjawords" relevancyFunction: returns 1 expected = "<span class='vomnibarMatch'>ninjaword</span>s" - assert.isTrue suggestion.generateHtml().indexOf(expected) >= 0 + assert.isTrue suggestion.generateHtml({}).indexOf(expected) >= 0 should "shorten urls", -> suggestion = new Suggestion @@ -273,7 +273,7 @@ context "suggestions", url: "http://ninjawords.com" title: "ninjawords" relevancyFunction: returns 1 - assert.equal -1, suggestion.generateHtml().indexOf("http://ninjawords.com") + assert.equal -1, suggestion.generateHtml({}).indexOf("http://ninjawords.com") context "RankingUtils.wordRelevancy", should "score higher in shorter URLs", -> |