commit 5da87acff0149c9bbede73455b9eece0366e2f3e
parent 9d9ed7f9ee5e1eb3ddb31b563ca06042e576e3e2
Author: Daisuke Akatsuka <daisuke@birchill.co.jp>
Date: Thu, 11 Dec 2025 01:12:05 +0000
Bug 1995227: Remove qsSuggestion special handling r=adw
Differential Revision: https://phabricator.services.mozilla.com/D275008
Diffstat:
9 files changed, 97 insertions(+), 47 deletions(-)
diff --git a/browser/components/urlbar/QuickSuggest.sys.mjs b/browser/components/urlbar/QuickSuggest.sys.mjs
@@ -703,6 +703,42 @@ class _QuickSuggest {
}
/**
+ * Returns the title and highlights for suggestions that should display their
+ * full keywords.
+ *
+ * When `fullKeyword` is defined, highlighting will be applied only to it, not
+ * to the title as a whole; otherwise highlighting will not be applied at all.
+ * It's unclear if that's the intended UI spec, but historically it's how
+ * highlighting has been implemented for suggestions that should display their
+ * full keywords.
+ *
+ * @param {object} options
+ * @param {Array} options.tokens
+ * It is compatible to UrlbarQueryContext.tokens.
+ * @param {Values<typeof lazy.UrlbarUtils.HIGHLIGHT>} [options.highlightType]
+ * @param {string} [options.fullKeyword]
+ * Full keyword if there is.
+ * @param {string} options.title
+ * Suggestion title.
+ * @returns {object} { value, highlights }
+ * The value will be used for title.
+ * The highlights will be created by UrlbarUtils.getTokenMatches().
+ */
+ getFullKeywordTitleAndHighlights({
+ tokens,
+ highlightType,
+ fullKeyword,
+ title,
+ }) {
+ return {
+ value: fullKeyword ? `${fullKeyword} — ${title}` : title,
+ highlights: fullKeyword
+ ? lazy.UrlbarUtils.getTokenMatches(tokens, fullKeyword, highlightType)
+ : [],
+ };
+ }
+
+ /**
* @returns {object}
* An object that maps from Nimbus variable names to their corresponding
* prefs, for prefs in `SUGGEST_PREFS` with `nimbusVariableIfExposedInUi`
diff --git a/browser/components/urlbar/UrlbarProviderQuickSuggest.sys.mjs b/browser/components/urlbar/UrlbarProviderQuickSuggest.sys.mjs
@@ -470,11 +470,20 @@ export class UrlbarProviderQuickSuggest extends UrlbarProvider {
isManageable: true,
};
+ let titleHighlights;
if (suggestion.full_keyword) {
- payload.title = suggestion.title;
- payload.qsSuggestion = suggestion.full_keyword;
+ let { value, highlights } =
+ lazy.QuickSuggest.getFullKeywordTitleAndHighlights({
+ tokens: queryContext.tokens,
+ highlightType: UrlbarUtils.HIGHLIGHT.SUGGESTED,
+ fullKeyword: suggestion.full_keyword,
+ title: suggestion.title,
+ });
+ payload.title = value;
+ titleHighlights = highlights;
} else {
payload.title = suggestion.title;
+ titleHighlights = UrlbarUtils.HIGHLIGHT.TYPED;
payload.shouldShowUrl = true;
}
@@ -484,8 +493,7 @@ export class UrlbarProviderQuickSuggest extends UrlbarProvider {
isBestMatch: !!suggestion.is_top_pick,
payload,
highlights: {
- qsSuggestion: UrlbarUtils.HIGHLIGHT.SUGGESTED,
- title: UrlbarUtils.HIGHLIGHT.TYPED,
+ title: titleHighlights,
},
});
}
diff --git a/browser/components/urlbar/UrlbarResult.sys.mjs b/browser/components/urlbar/UrlbarResult.sys.mjs
@@ -39,7 +39,10 @@ ChromeUtils.defineESModuleGetters(lazy, {
export class UrlbarResult {
/**
* @typedef {{ [name: string]: any }} Payload
- * @typedef {Record<string, typeof lazy.UrlbarUtils.HIGHLIGHT>} Highlights
+ *
+ * @typedef {typeof lazy.UrlbarUtils.HIGHLIGHT} HighlightType
+ * @typedef {Array<[number, number]>} HighlightIndexes e.g. [[index, length],,]
+ * @typedef {Record<string, HighlightType | HighlightIndexes>} Highlights
*/
/**
@@ -331,23 +334,23 @@ export class UrlbarResult {
let highlightable = value;
let highlightType;
- if (isTitle) {
- if (payloadName == "qsSuggestion") {
- value = this.payload.qsSuggestion + " — " + this.payload.title;
- } else if (payloadName == "url") {
- // If there's no title, show the domain as the title. Not all valid URLs
- // have a domain.
- try {
- value = highlightable = new URL(this.payload.url).URI.displayHostPort;
- highlightType = lazy.UrlbarUtils.HIGHLIGHT.TYPED;
- } catch (e) {}
- }
+ if (isTitle && payloadName == "url") {
+ // If there's no title, show the domain as the title. Not all valid URLs
+ // have a domain.
+ try {
+ value = highlightable = new URL(this.payload.url).URI.displayHostPort;
+ highlightType = lazy.UrlbarUtils.HIGHLIGHT.TYPED;
+ } catch (e) {}
}
if (typeof value == "string") {
value = value.substring(0, lazy.UrlbarUtils.MAX_TEXT_LENGTH);
}
+ if (Array.isArray(this.#highlights?.[payloadName])) {
+ return { value, highlights: this.#highlights[payloadName] };
+ }
+
highlightType ??= this.#highlights?.[payloadName];
if (!options.tokens?.length || !highlightType) {
@@ -384,9 +387,6 @@ export class UrlbarResult {
case lazy.UrlbarUtils.RESULT_TYPE.URL:
case lazy.UrlbarUtils.RESULT_TYPE.OMNIBOX:
case lazy.UrlbarUtils.RESULT_TYPE.REMOTE_TAB:
- if (this.payload.qsSuggestion) {
- return "qsSuggestion";
- }
if (this.payload.fallbackTitle) {
return "fallbackTitle";
}
diff --git a/browser/components/urlbar/UrlbarUtils.sys.mjs b/browser/components/urlbar/UrlbarUtils.sys.mjs
@@ -2039,9 +2039,6 @@ UrlbarUtils.RESULT_PAYLOAD_SCHEMA = {
provider: {
type: "string",
},
- qsSuggestion: {
- type: "string",
- },
requestId: {
type: "string",
},
diff --git a/browser/components/urlbar/private/AmpSuggestions.sys.mjs b/browser/components/urlbar/private/AmpSuggestions.sys.mjs
@@ -110,11 +110,25 @@ export class AmpSuggestions extends SuggestProvider {
this.#replaceSuggestionTemplates(normalized);
}
+ let isTopPick =
+ lazy.UrlbarPrefs.get("quickSuggestAmpTopPickCharThreshold") &&
+ lazy.UrlbarPrefs.get("quickSuggestAmpTopPickCharThreshold") <=
+ queryContext.trimmedLowerCaseSearchString.length;
+
+ let { value: title, highlights: titleHighlights } =
+ lazy.QuickSuggest.getFullKeywordTitleAndHighlights({
+ tokens: queryContext.tokens,
+ highlightType: isTopPick
+ ? lazy.UrlbarUtils.HIGHLIGHT.TYPED
+ : lazy.UrlbarUtils.HIGHLIGHT.SUGGESTED,
+ fullKeyword: normalized.fullKeyword,
+ title: normalized.title,
+ });
+
let payload = {
url: normalized.url,
originalUrl: normalized.rawUrl,
- title: normalized.title,
- qsSuggestion: normalized.fullKeyword,
+ title,
requestId: normalized.requestId,
urlTimestampIndex: normalized.urlTimestampIndex,
sponsoredImpressionUrl: normalized.impressionUrl,
@@ -126,11 +140,6 @@ export class AmpSuggestions extends SuggestProvider {
isManageable: true,
};
- let isTopPick =
- lazy.UrlbarPrefs.get("quickSuggestAmpTopPickCharThreshold") &&
- lazy.UrlbarPrefs.get("quickSuggestAmpTopPickCharThreshold") <=
- queryContext.trimmedLowerCaseSearchString.length;
-
let resultParams = {};
if (isTopPick) {
resultParams.isBestMatch = true;
@@ -154,9 +163,7 @@ export class AmpSuggestions extends SuggestProvider {
...resultParams,
payload,
highlights: {
- qsSuggestion: isTopPick
- ? lazy.UrlbarUtils.HIGHLIGHT.TYPED
- : lazy.UrlbarUtils.HIGHLIGHT.SUGGESTED,
+ title: titleHighlights,
},
});
}
diff --git a/browser/components/urlbar/private/WikipediaSuggestions.sys.mjs b/browser/components/urlbar/private/WikipediaSuggestions.sys.mjs
@@ -48,20 +48,26 @@ export class WikipediaSuggestions extends SuggestProvider {
}
makeResult(queryContext, suggestion) {
+ let { value: title, highlights: titleHighlights } =
+ lazy.QuickSuggest.getFullKeywordTitleAndHighlights({
+ tokens: queryContext.tokens,
+ highlightType: lazy.UrlbarUtils.HIGHLIGHT.SUGGESTED,
+ // Merino uses snake_case, so this will be `full_keyword` for it.
+ fullKeyword: suggestion.fullKeyword ?? suggestion.full_keyword,
+ title: suggestion.title,
+ });
+
return new lazy.UrlbarResult({
type: lazy.UrlbarUtils.RESULT_TYPE.URL,
source: lazy.UrlbarUtils.RESULT_SOURCE.SEARCH,
payload: {
url: suggestion.url,
- title: suggestion.title,
- qsSuggestion:
- // Merino uses snake_case, so this will be `full_keyword` for it.
- suggestion.fullKeyword ?? suggestion.full_keyword,
+ title,
isBlockable: true,
isManageable: true,
},
highlights: {
- qsSuggestion: lazy.UrlbarUtils.HIGHLIGHT.SUGGESTED,
+ title: titleHighlights,
},
});
}
diff --git a/browser/components/urlbar/tests/quicksuggest/QuickSuggestTestUtils.sys.mjs b/browser/components/urlbar/tests/quicksuggest/QuickSuggestTestUtils.sys.mjs
@@ -481,14 +481,13 @@ class _QuickSuggestTestUtils {
source: lazy.UrlbarUtils.RESULT_SOURCE.SEARCH,
heuristic: false,
payload: {
- title,
+ title: fullKeyword ? `${fullKeyword} — ${title}` : title,
url,
originalUrl,
requestId,
source,
provider,
isSponsored: true,
- qsSuggestion: fullKeyword ?? keyword,
sponsoredImpressionUrl: impressionUrl,
sponsoredClickUrl: clickUrl,
sponsoredBlockId: blockId,
@@ -587,7 +586,7 @@ class _QuickSuggestTestUtils {
source: lazy.UrlbarUtils.RESULT_SOURCE.SEARCH,
heuristic: false,
payload: {
- title,
+ title: fullKeyword ? `${fullKeyword} — ${title}` : title,
url,
icon,
iconBlob,
@@ -595,7 +594,6 @@ class _QuickSuggestTestUtils {
provider,
telemetryType,
isSponsored: false,
- qsSuggestion: fullKeyword ?? keyword,
isBlockable: true,
isManageable: true,
},
diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_impressionCaps.js b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_impressionCaps.js
@@ -43,7 +43,6 @@ const EXPECTED_SPONSORED_URLBAR_RESULT = {
url: "http://example.com/sponsored",
originalUrl: "http://example.com/sponsored",
title: "Sponsored suggestion",
- qsSuggestion: "sponsored",
icon: null,
isSponsored: true,
sponsoredImpressionUrl: "http://example.com/impression",
@@ -71,7 +70,6 @@ const EXPECTED_NONSPONSORED_URLBAR_RESULT = {
url: "http://example.com/nonsponsored",
originalUrl: "http://example.com/nonsponsored",
title: "Non-sponsored suggestion",
- qsSuggestion: "nonsponsored",
icon: null,
isSponsored: false,
sponsoredImpressionUrl: "http://example.com/impression",
diff --git a/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_merino.js b/browser/components/urlbar/tests/quicksuggest/unit/test_quicksuggest_merino.js
@@ -525,7 +525,9 @@ add_task(async function dismissals_amp() {
heuristic: false,
payload: {
provider: suggestion.provider,
- title: suggestion.title,
+ title: suggestion.full_keyword
+ ? `${suggestion.full_keyword} — ${suggestion.title}`
+ : suggestion.title,
url: suggestion.url,
originalUrl: suggestion.original_url || suggestion.url,
dismissalKey: suggestion.dismissal_key,
@@ -535,7 +537,6 @@ add_task(async function dismissals_amp() {
sponsoredBlockId: suggestion.block_id,
sponsoredAdvertiser: suggestion.advertiser,
sponsoredIabCategory: suggestion.iab_category,
- qsSuggestion: suggestion.full_keyword,
isBlockable: true,
isManageable: true,
isSponsored: true,
@@ -985,10 +986,9 @@ add_task(async function bestMatch() {
heuristic: false,
payload: {
telemetryType: provider,
- title: "title",
+ title: "full_keyword — title",
url: "url",
icon: null,
- qsSuggestion: "full_keyword",
isSponsored: false,
isBlockable: true,
isManageable: true,
