author | Harry Twyford <htwyford@mozilla.com> |
Tue, 19 May 2020 13:57:54 +0000 | |
changeset 530825 | e47fbd2bd52d12547adfd826783fc50e7ac2978c |
parent 530824 | e35047570e96b823d3c3c0920ae64dd8a50a7e40 |
child 530826 | 5571b75d82b66bad47fe582fb1d1f671254cd6fa |
push id | 37433 |
push user | dluca@mozilla.com |
push date | Wed, 20 May 2020 03:39:31 +0000 |
treeherder | mozilla-central@855249e545c3 [default view] [failures only] |
perfherder | [talos] [build metrics] [platform microbench] (compared to previous push) |
reviewers | mak |
bugs | 1626891 |
milestone | 78.0a1 |
first release with | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
last release without | nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
|
--- a/browser/components/extensions/test/xpcshell/test_ext_urlbar.js +++ b/browser/components/extensions/test/xpcshell/test_ext_urlbar.js @@ -281,17 +281,19 @@ add_task(async function test_onProviderR type: UrlbarUtils.RESULT_TYPE.SEARCH, source: UrlbarUtils.RESULT_SOURCE.SEARCH, title: "test", heuristic: true, payload: { query: "test", engine: "Test engine", suggestion: undefined, + tailPrefix: undefined, tail: undefined, + tailOffsetIndex: -1, keyword: undefined, isSearchHistory: false, icon: "", keywordOffer: false, }, }, // The second result should be our search suggestion result since the // default muxer sorts search suggestion results before other types.
--- a/browser/components/urlbar/UrlbarProviderSearchSuggestions.jsm +++ b/browser/components/urlbar/UrlbarProviderSearchSuggestions.jsm @@ -364,35 +364,43 @@ class ProviderSearchSuggestions extends if ( !suggestion || suggestion.entry.value == searchString || looksLikeUrl(suggestion.entry.value) ) { continue; } + if (suggestion.entry.tail && suggestion.entry.tailOffsetIndex < 0) { + Cu.reportError( + `Error in tail suggestion parsing. Value: ${suggestion.entry.value}, tail: ${suggestion.entry.tail}.` + ); + continue; + } + + let tail, tailPrefix; + if (UrlbarPrefs.get("richSuggestions.tail")) { + tail = suggestion.entry.tail; + tailPrefix = suggestion.entry.matchPrefix; + } + try { results.push( new UrlbarResult( UrlbarUtils.RESULT_TYPE.SEARCH, UrlbarUtils.RESULT_SOURCE.SEARCH, ...UrlbarResult.payloadAndSimpleHighlights(queryContext.tokens, { engine: [engine.name, UrlbarUtils.HIGHLIGHT.TYPED], suggestion: [ suggestion.entry.value, UrlbarUtils.HIGHLIGHT.SUGGESTED, ], - tail: [ - UrlbarPrefs.get("richSuggestions.tail") && - suggestion.entry.matchPrefix && - suggestion.entry.tail - ? suggestion.entry.matchPrefix + suggestion.entry.tail - : undefined, - UrlbarUtils.HIGHLIGHT.SUGGESTED, - ], + tailPrefix, + tail: [tail, UrlbarUtils.HIGHLIGHT.SUGGESTED], + tailOffsetIndex: suggestion.entry.tailOffsetIndex, keyword: [alias ? alias : undefined, UrlbarUtils.HIGHLIGHT.TYPED], query: [searchString.trim(), UrlbarUtils.HIGHLIGHT.NONE], isSearchHistory: false, icon: [ engine.iconURI && !suggestion.entry.value ? engine.iconURI.spec : "", ],
--- a/browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm +++ b/browser/components/urlbar/UrlbarProviderUnifiedComplete.jsm @@ -204,17 +204,19 @@ function makeUrlbarResult(tokens, info) UrlbarUtils.RESULT_SOURCE.SEARCH, ...UrlbarResult.payloadAndSimpleHighlights(tokens, { engine: [action.params.engineName, UrlbarUtils.HIGHLIGHT.TYPED], suggestion: [ action.params.searchSuggestion, UrlbarUtils.HIGHLIGHT.SUGGESTED, ], // For test interoperabilty with UrlbarProviderSearchSuggestions. + tailPrefix: undefined, tail: undefined, + tailOffsetIndex: -1, keyword: [action.params.alias, UrlbarUtils.HIGHLIGHT.TYPED], query: [ action.params.searchQuery.trim(), UrlbarUtils.HIGHLIGHT.NONE, ], isSearchHistory: !!action.params.isSearchHistory, icon: [info.icon], keywordOffer,
--- a/browser/components/urlbar/UrlbarResult.jsm +++ b/browser/components/urlbar/UrlbarResult.jsm @@ -123,17 +123,17 @@ class UrlbarResult { : [this.payload.url || "", this.payloadHighlights.url || []]; case UrlbarUtils.RESULT_TYPE.SEARCH: switch (this.payload.keywordOffer) { case UrlbarUtils.KEYWORD_OFFER.SHOW: return [this.payload.keyword, this.payloadHighlights.keyword]; case UrlbarUtils.KEYWORD_OFFER.HIDE: return ["", []]; } - if (this.payload.tail) { + if (this.payload.tail && this.payload.tailOffsetIndex >= 0) { return [this.payload.tail, this.payloadHighlights.tail]; } else if (this.payload.suggestion) { return [this.payload.suggestion, this.payloadHighlights.suggestion]; } return [this.payload.query, this.payloadHighlights.query]; default: return ["", []]; }
--- a/browser/components/urlbar/UrlbarUtils.jsm +++ b/browser/components/urlbar/UrlbarUtils.jsm @@ -640,16 +640,22 @@ UrlbarUtils.RESULT_PAYLOAD_SCHEMA = { type: "boolean", }, suggestion: { type: "string", }, tail: { type: "string", }, + tailPrefix: { + type: "string", + }, + tailOffsetIndex: { + type: "number", + }, title: { type: "string", }, url: { type: "string", }, }, },
--- a/browser/components/urlbar/UrlbarView.jsm +++ b/browser/components/urlbar/UrlbarView.jsm @@ -754,16 +754,34 @@ class UrlbarView { favicon.className = "urlbarView-favicon"; noWrap.appendChild(favicon); item._elements.set("favicon", favicon); let typeIcon = this._createElement("span"); typeIcon.className = "urlbarView-type-icon"; noWrap.appendChild(typeIcon); + let tailPrefix = this._createElement("span"); + tailPrefix.className = "urlbarView-tail-prefix"; + noWrap.appendChild(tailPrefix); + item._elements.set("tailPrefix", tailPrefix); + // tailPrefix holds text only for alignment purposes so it should never be + // read to screen readers. + tailPrefix.toggleAttribute("aria-hidden", true); + + let tailPrefixStr = this._createElement("span"); + tailPrefixStr.className = "urlbarView-tail-prefix-string"; + tailPrefix.appendChild(tailPrefixStr); + item._elements.set("tailPrefixStr", tailPrefixStr); + + let tailPrefixChar = this._createElement("span"); + tailPrefixChar.className = "urlbarView-tail-prefix-char"; + tailPrefix.appendChild(tailPrefixChar); + item._elements.set("tailPrefixChar", tailPrefixChar); + let title = this._createElement("span"); title.className = "urlbarView-title"; noWrap.appendChild(title); item._elements.set("title", title); let tagsContainer = this._createElement("span"); tagsContainer.className = "urlbarView-tags"; noWrap.appendChild(tagsContainer); @@ -891,16 +909,26 @@ class UrlbarView { } let title = item._elements.get("title"); this._addTextContentWithHighlights( title, result.title, result.titleHighlights ); + + if (result.payload.tail && result.payload.tailOffsetIndex >= 0) { + this._fillTailSuggestionPrefix(item, result); + title.setAttribute("aria-label", result.payload.suggestion); + item.toggleAttribute("tail-suggestion", true); + } else { + item.removeAttribute("tail-suggestion"); + title.removeAttribute("aria-label"); + } + title._tooltip = result.title; if (title.hasAttribute("overflow")) { title.setAttribute("title", title._tooltip); } let tagsContainer = item._elements.get("tagsContainer"); tagsContainer.textContent = ""; if (result.payload.tags && result.payload.tags.length) { @@ -1320,16 +1348,35 @@ class UrlbarView { highlightIndex + highlightLength ); parentNode.appendChild(strong); } index = highlightIndex + highlightLength; } } + /** + * Adds markup for a tail suggestion prefix to a row. + * @param {Node} item + * The node for the result row. + * @param {UrlbarResult} result + * A UrlbarResult representing a tail suggestion. + */ + _fillTailSuggestionPrefix(item, result) { + let tailPrefixStrNode = item._elements.get("tailPrefixStr"); + let tailPrefixStr = result.payload.suggestion.substring( + 0, + result.payload.tailOffsetIndex + ); + tailPrefixStrNode.textContent = tailPrefixStr; + + let tailPrefixCharNode = item._elements.get("tailPrefixChar"); + tailPrefixCharNode.textContent = result.payload.tailPrefix; + } + _enableOrDisableOneOffSearches(enable = true) { if (enable) { this.oneOffSearchButtons.telemetryOrigin = "urlbar"; this.oneOffSearchButtons.style.display = ""; this.oneOffSearchButtons.textbox = this.input.inputField; this.oneOffSearchButtons.view = this; } else { this.oneOffSearchButtons.telemetryOrigin = null;
--- a/browser/components/urlbar/tests/unit/head.js +++ b/browser/components/urlbar/tests/unit/head.js @@ -298,17 +298,19 @@ async function addTestTailSuggestionsEng * @param {number} [options.keywordOffer] * A value from UrlbarUtils.KEYWORD_OFFER. * @returns {UrlbarResult} */ function makeSearchResult( queryContext, { suggestion, + tailPrefix, tail, + tailOffsetIndex, engineName, alias, query, engineIconUri, heuristic = false, keywordOffer, } ) { @@ -318,23 +320,34 @@ function makeSearchResult( keywordOffer = UrlbarUtils.KEYWORD_OFFER.NONE; if (alias && !query.trim() && alias.startsWith("@")) { keywordOffer = heuristic ? UrlbarUtils.KEYWORD_OFFER.HIDE : UrlbarUtils.KEYWORD_OFFER.SHOW; } } + // Tail suggestion common cases, handled here to reduce verbosity in tests. + if (tail && !tailPrefix) { + tailPrefix = "… "; + } + + if (!tailOffsetIndex) { + tailOffsetIndex = tail ? suggestion.indexOf(tail) : -1; + } + let result = new UrlbarResult( UrlbarUtils.RESULT_TYPE.SEARCH, UrlbarUtils.RESULT_SOURCE.SEARCH, ...UrlbarResult.payloadAndSimpleHighlights(queryContext.tokens, { engine: [engineName, UrlbarUtils.HIGHLIGHT.TYPED], suggestion: [suggestion, UrlbarUtils.HIGHLIGHT.SUGGESTED], + tailPrefix, tail: [tail, UrlbarUtils.HIGHLIGHT.SUGGESTED], + tailOffsetIndex, keyword: [alias, UrlbarUtils.HIGHLIGHT.TYPED], // Check against undefined so consumers can pass in the empty string. query: [ typeof query != "undefined" ? query : queryContext.searchString.trim(), UrlbarUtils.HIGHLIGHT.TYPED, ], isSearchHistory: false, icon: [engineIconUri ? engineIconUri : ""],
--- a/browser/components/urlbar/tests/unit/test_search_suggestions_tail.js +++ b/browser/components/urlbar/tests/unit/test_search_suggestions_tail.js @@ -131,22 +131,22 @@ add_task(async function basic_tail() { let context = createContext(query, { isPrivate: false }); await check_results({ context, matches: [ makeSearchResult(context, { engineName: ENGINE_NAME, heuristic: true }), makeSearchResult(context, { engineName: ENGINE_NAME, suggestion: query + "oronto", - tail: "… toronto", + tail: "toronto", }), makeSearchResult(context, { engineName: ENGINE_NAME, suggestion: query + "unisia", - tail: "… tunisia", + tail: "tunisia", }), ], }); await cleanUpSuggestions(); }); /** * Tests a suggestions provider that returns both normal and tail suggestions. @@ -183,22 +183,22 @@ add_task(async function mixed_results() makeSearchResult(context, { engineName: ENGINE_NAME, suggestion: "what is the time today texas", tail: undefined, }), makeSearchResult(context, { engineName: ENGINE_NAME, suggestion: query + "oronto", - tail: "… toronto", + tail: "toronto", }), makeSearchResult(context, { engineName: ENGINE_NAME, suggestion: query + "unisia", - tail: "… tunisia", + tail: "tunisia", }), ], }); await cleanUpSuggestions(); }); /** * Tests that tail suggestions are deduped if their full-text form is a dupe of @@ -217,17 +217,17 @@ add_task(async function dedupe_local() { makeSearchResult(context, { engineName: ENGINE_NAME, suggestion: query + "oronto", tail: undefined, }), makeSearchResult(context, { engineName: ENGINE_NAME, suggestion: query + "unisia", - tail: "… tunisia", + tail: "tunisia", }), ], }); Services.prefs.clearUserPref("browser.urlbar.maxHistoricalSearchSuggestions"); await cleanUpSuggestions(); }); @@ -241,17 +241,17 @@ add_task(async function limit_results() context.maxResults = 2; await check_results({ context, matches: [ makeSearchResult(context, { engineName: ENGINE_NAME, heuristic: true }), makeSearchResult(context, { engineName: ENGINE_NAME, suggestion: query + "oronto", - tail: "… toronto", + tail: "toronto", }), ], }); await cleanUpSuggestions(); }); function updateSearchHistory(op, value) { return new Promise((resolve, reject) => {
--- a/browser/themes/shared/urlbarView.inc.css +++ b/browser/themes/shared/urlbarView.inc.css @@ -360,16 +360,35 @@ tip buttons wrap around under it. We want the icon and title to remain on one line. So make the title's flex-basis the width of the parent minus the width of the icon. */ flex-basis: calc(100% - /* .urlbarView-row-inner padding-inline-start */ 6px - /* .urlbarView-favicon width */ 24px - /* .urlbarView-favicon margin-inline-end */ 12px); flex-grow: 1; flex-shrink: 1; } +/* Tail suggestions */ +.urlbarView-tail-prefix { + display: none; + justify-content: flex-end; + white-space: pre; +} + +.urlbarView-row[tail-suggestion] > .urlbarView-row-inner > .urlbarView-no-wrap > .urlbarView-tail-prefix { + display: inline-flex; +} + +.urlbarView-tail-prefix > .urlbarView-tail-prefix-string { + visibility: hidden; +} + +.urlbarView-tail-prefix > .urlbarView-tail-prefix-char { + position: absolute; +} + /* Title separator */ .urlbarView-title-separator::before { content: "\2014"; margin: 0 .4em; opacity: 0.6; }
--- a/toolkit/components/search/SearchSuggestionController.jsm +++ b/toolkit/components/search/SearchSuggestionController.jsm @@ -88,16 +88,37 @@ class SearchSuggestionEntry { get matchPrefix() { return this._matchPrefix; } get tail() { return this._tail; } + + get tailOffsetIndex() { + if (!this._tail) { + return -1; + } + + let offsetIndex = this._value.lastIndexOf(this._tail); + if (offsetIndex + this._tail.length < this._value.length) { + // We might have a tail suggestion that starts with a word contained in + // the full-text suggestion. e.g. "london sights in l" ... "london". + let lastWordIndex = this._value.lastIndexOf(" "); + if (this._tail.startsWith(this._value.substring(lastWordIndex))) { + offsetIndex = lastWordIndex; + } else { + // Something's gone wrong. Consumers should not show this result. + offsetIndex = -1; + } + } + + return offsetIndex; + } } // Maps each engine name to a unique firstPartyDomain, so that requests to // different engines are isolated from each other and from normal browsing. // This is the same for all the controllers. var gFirstPartyDomains = new Map(); /**
--- a/toolkit/components/search/tests/xpcshell/test_searchSuggest.js +++ b/toolkit/components/search/tests/xpcshell/test_searchSuggest.js @@ -342,16 +342,28 @@ add_task(async function empty_rich_resul Assert.equal(result.remote[1].value, "richempty query tail 1"); Assert.ok(!result.remote[1].matchPrefix); Assert.ok(!result.remote[1].tail); Assert.equal(result.remote[2].value, "richempty query tail 2"); Assert.ok(!result.remote[2].matchPrefix); Assert.ok(!result.remote[2].tail); }); +add_task(async function tail_offset_index() { + let controller = new SearchSuggestionController(); + let result = await controller.fetch("tail tail 1 t", false, getEngine); + Assert.equal(result.term, "tail tail 1 t"); + Assert.equal(result.local.length, 0); + Assert.equal(result.remote.length, 3); + Assert.equal(result.remote[1].value, "tail tail 1 t tail 1"); + Assert.equal(result.remote[1].matchPrefix, "… "); + Assert.equal(result.remote[1].tail, "tail 1"); + Assert.equal(result.remote[1].tailOffsetIndex, 14); +}); + add_task(async function fetch_twice_in_a_row() { // Two entries since the first will match the first fetch but not the second. await updateSearchHistory("bump", "delay local"); await updateSearchHistory("bump", "delayed local"); let controller = new SearchSuggestionController(); let resultPromise1 = controller.fetch("delay", false, getEngine);