commit 0e580a7096800da611767de08808b805a0486ff1
parent 45400a1060cccb320da95bd77fc5000fb3169039
Author: Mark Banner <standard8@mozilla.com>
Date: Thu, 30 Oct 2025 08:44:45 +0000
Bug 1997161 - Stop saving the order hint in search engine settings files. r=mcheang
The order hint is only used for configuration based search engines, but they have a toJSON which skips saving it.
Differential Revision: https://phabricator.services.mozilla.com/D270571
Diffstat:
4 files changed, 37 insertions(+), 29 deletions(-)
diff --git a/toolkit/components/search/ConfigSearchEngine.sys.mjs b/toolkit/components/search/ConfigSearchEngine.sys.mjs
@@ -442,6 +442,13 @@ export class ConfigSearchEngine extends SearchEngine {
#telemetryId;
/**
+ * The order hint for this engine, as determined by the search configuration.
+ *
+ * @type {?number}
+ */
+ #orderHint;
+
+ /**
* @param {object} options
* The options for this search engine.
* @param {SearchEngineDefinition} options.config
@@ -580,6 +587,16 @@ export class ConfigSearchEngine extends SearchEngine {
}
/**
+ * Gets the order hint for this engine. This is determined from the search
+ * configuration when the engine is initialized.
+ *
+ * @type {?number}
+ */
+ get orderHint() {
+ return this.#orderHint;
+ }
+
+ /**
* Returns the icon URL for the search engine closest to the preferred width.
*
* @param {number} preferredWidth
@@ -680,7 +697,7 @@ export class ConfigSearchEngine extends SearchEngine {
* The engine definition from the search config for this engine.
*/
#init(engineConfig) {
- this._orderHint = engineConfig.orderHint;
+ this.#orderHint = engineConfig.orderHint;
this.#telemetryId = engineConfig.identifier;
this.#isGeneralPurposeSearchEngine =
engineConfig.classification == lazy.SearchEngineClassification.GENERAL;
diff --git a/toolkit/components/search/SearchEngine.sys.mjs b/toolkit/components/search/SearchEngine.sys.mjs
@@ -567,8 +567,6 @@ export class SearchEngine {
_name = null;
// The name of the charset used to submit the search terms.
_queryCharset = null;
- // The order hint from the configuration (if any).
- _orderHint = null;
// Set to true once the engine has been added to the store, and the initial
// notification sent. This allows to skip sending notifications during
// initialization.
@@ -1044,7 +1042,6 @@ export class SearchEngine {
json.queryCharset || lazy.SearchUtils.DEFAULT_QUERY_CHARSET;
this._iconMapObj = json._iconMapObj || null;
this._metaData = json._metaData || {};
- this._orderHint = json._orderHint || null;
this._definedAliases = json._definedAliases || [];
// These changed keys in Firefox 80, maintain the old keys
// for backwards compatibility.
@@ -1078,7 +1075,6 @@ export class SearchEngine {
"_iconMapObj",
"_metaData",
"_urls",
- "_orderHint",
"_filePath",
"_definedAliases",
];
@@ -1152,10 +1148,11 @@ export class SearchEngine {
* Gets the order hint for this engine. This is determined from the search
* configuration when the engine is initialized.
*
- * @type {number}
+ * @type {?number}
*/
get orderHint() {
- return this._orderHint;
+ // Overridden in derived classes.
+ return null;
}
/**
diff --git a/toolkit/components/search/SearchUtils.sys.mjs b/toolkit/components/search/SearchUtils.sys.mjs
@@ -7,6 +7,10 @@
import { AppConstants } from "resource://gre/modules/AppConstants.sys.mjs";
import { XPCOMUtils } from "resource://gre/modules/XPCOMUtils.sys.mjs";
+/**
+ * @import {SearchEngine} from "moz-src:///toolkit/components/search/SearchEngine.sys.mjs"
+ */
+
const lazy = XPCOMUtils.declareLazy({
logConsole: () =>
console.createInstance({
@@ -405,16 +409,16 @@ export var SearchUtils = {
*
* @param {object} options
* The options for this function.
- * @param {object[]} options.engines
+ * @param {SearchEngine[]} options.engines
* An array of engine objects to sort. These should have the `name` and
* `orderHint` fields as top-level properties.
- * @param {object} options.appDefaultEngine
+ * @param {SearchEngine} options.appDefaultEngine
* The application default engine.
- * @param {object} [options.appPrivateDefaultEngine]
+ * @param {SearchEngine} [options.appPrivateDefaultEngine]
* The application private default engine, if any.
* @param {string} [options.locale]
* The current application locale, or the locale to use for the sorting.
- * @returns {object[]}
+ * @returns {SearchEngine[]}
* The sorted array of engine objects.
*/
sortEnginesByDefaults({
@@ -423,7 +427,9 @@ export var SearchUtils = {
appPrivateDefaultEngine,
locale = Services.locale.appLocaleAsBCP47,
}) {
+ /** @type {SearchEngine[]} */
const sortedEngines = [];
+ /** @type {Set<string>} */
const addedEngines = new Set();
function maybeAddEngineToSort(engine) {
@@ -447,15 +453,14 @@ export var SearchUtils = {
maybeAddEngineToSort(appPrivateDefault);
}
- let remainingEngines;
const collator = new Intl.Collator(locale);
- remainingEngines = engines.filter(e => !addedEngines.has(e.name));
+ let remainingEngines = engines.filter(e => !addedEngines.has(e.name));
// We sort by highest orderHint first, then alphabetically by name.
remainingEngines.sort((a, b) => {
- if (a._orderHint && b.orderHint) {
- if (a._orderHint == b.orderHint) {
+ if (a.orderHint && b.orderHint) {
+ if (a.orderHint == b.orderHint) {
return collator.compare(a.name, b.name);
}
return b.orderHint - a.orderHint;
diff --git a/toolkit/components/search/tests/xpcshell/test_webextensions_upgrade.js b/toolkit/components/search/tests/xpcshell/test_webextensions_upgrade.js
@@ -95,20 +95,10 @@ add_task(async function test_upgrade_changes_name() {
Ci.nsISearchService.CHANGE_REASON_UNKNOWN
);
- // When we add engines currently, we normally force using the saved order.
- // Reset that here, so we can check the order is reset in the case this
- // is a application provided engine change.
- Services.search.wrappedJSObject._settings.setMetaDataAttribute(
- "useSavedOrder",
- false
- );
- Services.search.getEngineByName("engine1").wrappedJSObject._orderHint = null;
- Services.search.getEngineByName("engine2").wrappedJSObject._orderHint = null;
-
Assert.deepEqual(
(await Services.search.getVisibleEngines()).map(e => e.name),
["engine1", "engine2", "engine"],
- "Should have the expected order initially"
+ "Should have the expected engines initially"
);
let promiseChanged = TestUtils.topicObserved(
@@ -141,9 +131,8 @@ add_task(async function test_upgrade_changes_name() {
Assert.deepEqual(
(await Services.search.getVisibleEngines()).map(e => e.name),
- // Expected order: Default, then others in alphabetical.
- ["engine1", "Bar", "engine2"],
- "Should have updated the engine order"
+ ["engine1", "engine2", "Bar"],
+ "Should have the initial engines plus the upgraded one"
);
await extension.unload();
