commit d852361059f6ab860189145cedef6d5d41783a78
parent 46108568a4b70855b955234af2f999f39f8190b9
Author: Drew Willcoxon <adw@mozilla.com>
Date: Thu, 13 Nov 2025 21:18:50 +0000
Bug 1999858 - Fix an AMO/addon suggestions dismissal error. r=daisuke,urlbar-reviewers
The addon test does test individual dismissals, but it didn't catch this bug
because it uses a fake, synthetic result. I modified `head.js` so all these
xpcshell dismissal tests now use a real result. I verified that this does catch
the problem (which I've fixed of course).
Differential Revision: https://phabricator.services.mozilla.com/D272378
Diffstat:
3 files changed, 59 insertions(+), 11 deletions(-)
diff --git a/browser/components/urlbar/private/AddonSuggestions.sys.mjs b/browser/components/urlbar/private/AddonSuggestions.sys.mjs
@@ -78,11 +78,6 @@ export class AddonSuggestions extends SuggestProvider {
return null;
}
- if (suggestion.source == "rust") {
- suggestion.icon = suggestion.iconUrl;
- delete suggestion.iconUrl;
- }
-
// Set UTM params unless they're already defined. This allows remote
// settings or Merino to override them if need be.
let url = new URL(suggestion.url);
@@ -96,6 +91,8 @@ export class AddonSuggestions extends SuggestProvider {
url: url.href,
originalUrl: suggestion.url,
shouldShowUrl: true,
+ // Rust uses `iconUrl` but Merino uses `icon`.
+ icon: suggestion.iconUrl ?? suggestion.icon,
title: suggestion.title,
description: suggestion.description,
bottomTextL10n: {
diff --git a/browser/components/urlbar/tests/quicksuggest/QuickSuggestTestUtils.sys.mjs b/browser/components/urlbar/tests/quicksuggest/QuickSuggestTestUtils.sys.mjs
@@ -1028,9 +1028,10 @@ class _QuickSuggestTestUtils {
source = "rust",
provider = "Yelp",
isTopPick = false,
- // The default Yelp suggestedIndex is 0, unlike most other Suggest
- // suggestion types, which use -1. Note that many callers still use
- // -1 here because they test without the search suggestion provider.
+ // The logic for the default Yelp `suggestedIndex` is complex and depends on
+ // whether `UrlbarProviderSearchSuggestions` is active and whether search
+ // suggestions are shown first. By default -- when the answer to both of
+ // those questions is Yes -- Yelp's `suggestedIndex` is 0.
suggestedIndex = 0,
isSuggestedIndexRelativeToGroup = true,
originalUrl = undefined,
diff --git a/browser/components/urlbar/tests/quicksuggest/unit/head.js b/browser/components/urlbar/tests/quicksuggest/unit/head.js
@@ -210,10 +210,16 @@ async function doDismissOneTest({
"quicksuggest-dismissals-changed"
);
+ let actualResult = await getActualResult({
+ providers,
+ query: queriesForDismissals[0].query,
+ expectedResult: result,
+ });
+
triggerCommand({
- result,
command,
feature,
+ result: actualResult,
expectedCountsByCall: {
removeResult: 1,
},
@@ -227,7 +233,7 @@ async function doDismissOneTest({
"canClearDismissedSuggestions should return true after triggering command"
);
Assert.ok(
- await QuickSuggest.isResultDismissed(result),
+ await QuickSuggest.isResultDismissed(actualResult),
"The result should be dismissed"
);
@@ -330,10 +336,16 @@ async function doDismissAllTest({
"quicksuggest-dismissals-changed"
);
+ let actualResult = await getActualResult({
+ providers,
+ query: queries[0].query,
+ expectedResult: result,
+ });
+
triggerCommand({
- result,
command,
feature,
+ result: actualResult,
expectedCountsByCall: {
removeResult: 1,
},
@@ -399,6 +411,44 @@ async function doDismissAllTest({
}
/**
+ * Does a search, asserts an actual result exists that matches the given result,
+ * and returns it.
+ *
+ * @param {object} options
+ * Options object.
+ * @param {SuggestFeature} options.query
+ * The search string.
+ * @param {UrlbarResult} options.expectedResult
+ * The expected result.
+ * @param {string[]} [options.providers]
+ * The providers to query.
+ */
+async function getActualResult({
+ query,
+ expectedResult,
+ providers = [UrlbarProviderQuickSuggest.name],
+}) {
+ info("Doing search to get an actual result: " + JSON.stringify(query));
+ let context = createContext(query, {
+ providers,
+ isPrivate: false,
+ });
+ await check_results({
+ context,
+ matches: [expectedResult],
+ });
+
+ let actualResult = context.results.find(
+ r =>
+ r.providerName == UrlbarProviderQuickSuggest.name &&
+ r.payload.provider == expectedResult.payload.provider
+ );
+ Assert.ok(actualResult, "Search should have returned a matching result");
+
+ return actualResult;
+}
+
+/**
* Does some "show less frequently" tests where the cap is set in remote
* settings and Nimbus. See `doOneShowLessFrequentlyTest()`. This function
* assumes the matching behavior implemented by the given `SuggestFeature` is
