commit b652076c527679b102ff06d4926b2277c263ec43
parent 397c3187280dfa84ce9da56a9944d945bf73977d
Author: Mark Banner <standard8@mozilla.com>
Date: Wed, 29 Oct 2025 13:58:47 +0000
Bug 1997053 - Add a warning, and set a preference if a search engine matches the ignore lists. r=search-reviewers,daleharvey
Differential Revision: https://phabricator.services.mozilla.com/D270470
Diffstat:
4 files changed, 123 insertions(+), 6 deletions(-)
diff --git a/toolkit/components/search/SearchService.sys.mjs b/toolkit/components/search/SearchService.sys.mjs
@@ -1690,7 +1690,18 @@ export class SearchService {
* Returns true if the engine matches a ignorelists entry.
*/
#engineMatchesIgnoreLists(engine) {
+ /** @type {(name: string, url: string, type: string) => void} */
+ let logIgnored = (name, url, type) => {
+ lazy.logConsole.warn("Search engine", name, `matches ${type}`, url);
+ Services.prefs.setCharPref(
+ lazy.SearchUtils.BROWSER_SEARCH_PREF + "lastEngineIgnored",
+ // Limit length of url to avoid storing too much in prefs.
+ `${Math.trunc(Date.now() / 1000)} Search engine '${name}' matches ${type} ignore list ${url.substring(0, 200)}`
+ );
+ };
+
if (this.#loadPathIgnoreList.includes(engine._loadPath)) {
+ logIgnored(engine.name, engine._loadPath, "load path");
return true;
}
let url = engine.searchURLWithNoTerms.spec.toLowerCase();
@@ -1699,6 +1710,7 @@ export class SearchService {
url.includes(code.toLowerCase())
)
) {
+ logIgnored(engine.name, url, "submission url");
return true;
}
return false;
@@ -2486,7 +2498,6 @@ export class SearchService {
*/
#addEngineToStore(engine, skipDuplicateCheck = false) {
if (this.#engineMatchesIgnoreLists(engine)) {
- lazy.logConsole.debug("#addEngineToStore: Ignoring engine");
return;
}
diff --git a/toolkit/components/search/tests/xpcshell/settings/ignorelist.json b/toolkit/components/search/tests/xpcshell/settings/ignorelist.json
@@ -63,7 +63,8 @@
"_definedAliases": [],
"_updateInterval": null,
"_updateURL": null,
- "_iconUpdateURL": null
+ "_iconUpdateURL": null,
+ "_loadPath": "[https]www.example.com/search.xml"
}
]
}
diff --git a/toolkit/components/search/tests/xpcshell/test_ignorelist.js b/toolkit/components/search/tests/xpcshell/test_ignorelist.js
@@ -41,7 +41,7 @@ add_setup(async () => {
});
add_task(async function test_ignoreListOnLoadSettings() {
- consoleAllowList.push("Failed to load");
+ let finishListening = TestUtils.listenForConsoleMessages();
Assert.ok(
!Services.search.isInitialized,
@@ -63,12 +63,12 @@ add_task(async function test_ignoreListOnLoadSettings() {
await ignoreListUpdateCompleted;
Assert.ok(
- !(await Services.search.getEngineByName("Test search engine")),
+ !Services.search.getEngineByName("Test search engine"),
"Should not have installed the add-on engine from settings"
);
Assert.ok(
- !(await Services.search.getEngineByName("OpenSearchTest")),
+ !Services.search.getEngineByName("OpenSearchTest"),
"Should not have installed the OpenSearch engine from settings"
);
@@ -77,6 +77,45 @@ add_task(async function test_ignoreListOnLoadSettings() {
["defaultEngine"],
"Should have correctly started and installed only the default engine"
);
+
+ assertPreference(
+ "OpenSearchTest",
+ "submission url",
+ "https://www.example.com/test?search_query=&opensearch=sng"
+ );
+
+ let consoleMessages = await finishListening();
+ Assert.deepEqual(
+ consoleMessages
+ .filter(msg => msg.level == "warn")
+ .map(msg => {
+ return {
+ level: msg.level,
+ arguments: msg.arguments,
+ };
+ }),
+ [
+ {
+ level: "warn",
+ arguments: [
+ "Search engine",
+ "Test search engine",
+ "matches submission url",
+ "https://www.example.com/search?q=&ignore=true&channel=sng",
+ ],
+ },
+ {
+ level: "warn",
+ arguments: [
+ "Search engine",
+ "OpenSearchTest",
+ "matches submission url",
+ "https://www.example.com/test?search_query=&opensearch=sng",
+ ],
+ },
+ ],
+ "Should log console warnings for both search engines"
+ );
});
add_task(async function test_ignoreListOnInstall() {
@@ -88,18 +127,26 @@ add_task(async function test_ignoreListOnInstall() {
await SearchTestUtils.installSearchExtension({
name: kSearchEngineID1,
search_url: kSearchEngineURL1,
+ search_url_get_params: "",
});
let engine = Services.search.getEngineByName(kSearchEngineID1);
Assert.equal(
engine,
null,
- "Engine with ignored search params should not exist"
+ "Engine with ignored search params should not be added"
+ );
+
+ assertPreference(
+ kSearchEngineID1,
+ "submission url",
+ kSearchEngineURL1.replace("{searchTerms}", "")
);
await SearchTestUtils.installSearchExtension({
name: kSearchEngineID2,
search_url: kSearchEngineURL2,
+ search_url_get_params: "",
});
// An ignored engine shouldn't be available at all
@@ -110,10 +157,19 @@ add_task(async function test_ignoreListOnInstall() {
"Engine with ignored search params of a different case should not exist"
);
+ assertPreference(
+ kSearchEngineID2,
+ "submission url",
+ kSearchEngineURL2.replace("{searchTerms}", "").toLowerCase()
+ );
+
+ let finishListening = TestUtils.listenForConsoleMessages();
+
await SearchTestUtils.installSearchExtension({
id: kExtensionID,
name: kSearchEngineID3,
search_url: kSearchEngineURL3,
+ search_url_get_params: "",
});
// An ignored engine shouldn't be available at all
@@ -123,4 +179,52 @@ add_task(async function test_ignoreListOnInstall() {
null,
"Engine with ignored extension id should not exist"
);
+
+ assertPreference(kSearchEngineID3, "load path", `[addon]${kExtensionID}`);
+
+ let consoleMessages = await finishListening();
+ Assert.deepEqual(
+ consoleMessages
+ .filter(msg => msg.level == "warn")
+ .map(msg => {
+ return {
+ level: msg.level,
+ arguments: msg.arguments,
+ };
+ }),
+ [
+ {
+ level: "warn",
+ arguments: [
+ "Search engine",
+ kSearchEngineID3,
+ "matches load path",
+ `[addon]${kExtensionID}`,
+ ],
+ },
+ ],
+ "Should have logged a console message"
+ );
});
+
+/**
+ * Asserts that the lastEngineIgnored preference is set correctly.
+ *
+ * @param {string} engineName
+ * @param {string} ignoreListtype
+ * @param {string} url
+ */
+function assertPreference(engineName, ignoreListtype, url) {
+ let prefValue = Services.prefs.getCharPref(
+ SearchUtils.BROWSER_SEARCH_PREF + "lastEngineIgnored"
+ );
+ let matchResult = prefValue.match(/\d+ (.*?) (?:(https.*|\[.*))/);
+ Assert.deepEqual(
+ matchResult.slice(1),
+ [
+ `Search engine '${engineName}' matches ${ignoreListtype} ignore list`,
+ url,
+ ],
+ "Should have set the preference to the last engine ignored"
+ );
+}
diff --git a/toolkit/modules/Troubleshoot.sys.mjs b/toolkit/modules/Troubleshoot.sys.mjs
@@ -41,6 +41,7 @@ const PREFS_FOR_DISPLAY = [
"browser.places.",
"browser.privatebrowsing.",
"browser.search.context.loadInBackground",
+ "browser.search.lastEngineIgnored",
"browser.search.lastSettingsCorruptTime",
"browser.search.log",
"browser.search.openintab",
