Bug 1801813 - Improve search engine load path reporting for add-ons, user and enterprise policy engines. r=mcheang,settings-reviewers,mconley

Differential Revision: https://phabricator.services.mozilla.com/D162700
This commit is contained in:
Mark Banner 2022-12-14 15:12:10 +00:00
parent 3372e411f9
commit 640699e64b
23 changed files with 335 additions and 56 deletions

View File

@ -24,14 +24,14 @@ SearchTestUtils.init(this);
const DEFAULT_ENGINE = {
id: "basic",
name: "basic",
loadPath: "[other]addEngineWithDetails:basic@search.mozilla.org",
loadPath: "[addon]basic@search.mozilla.org",
submissionUrl:
"https://mochi.test:8888/browser/browser/components/search/test/browser/?search=&foo=1",
};
const ALTERNATE_ENGINE = {
id: "simple",
name: "Simple Engine",
loadPath: "[other]addEngineWithDetails:simple@search.mozilla.org",
loadPath: "[addon]simple@search.mozilla.org",
submissionUrl: "https://example.com/?sourceId=Mozilla-search&search=",
};
const ALTERNATE2_ENGINE = {
@ -267,7 +267,7 @@ add_task(async function test_extension_setting_default_engine_external() {
await checkTelemetry("addon-install", DEFAULT_ENGINE, {
id: "other-Example Engine",
name: "Example Engine",
loadPath: "[other]addEngineWithDetails:extension1@mozilla.com",
loadPath: "[addon]extension1@mozilla.com",
submissionUrl: "https://example.com/?q=",
});
clearTelemetry();
@ -292,7 +292,7 @@ add_task(async function test_extension_setting_default_engine_external() {
{
id: "other-Example Engine",
name: "Example Engine",
loadPath: "[other]addEngineWithDetails:extension1@mozilla.com",
loadPath: "[addon]extension1@mozilla.com",
submissionUrl: "https://example.com/?q=",
},
DEFAULT_ENGINE
@ -317,7 +317,7 @@ add_task(async function test_extension_setting_default_engine_external() {
await checkTelemetry("addon-install", DEFAULT_ENGINE, {
id: "other-Example Engine",
name: "Example Engine",
loadPath: "[other]addEngineWithDetails:extension1@mozilla.com",
loadPath: "[addon]extension1@mozilla.com",
submissionUrl: "https://example.com/?q=",
});

View File

@ -277,8 +277,7 @@ add_task(async function test_setDefaultEngine() {
prev_id: engine1.telemetryId,
new_id: "other-engine2",
new_name: "engine2",
new_load_path:
"[other]addEngineWithDetails:engine2@tests.mozilla.org",
new_load_path: "[addon]engine2@tests.mozilla.org",
new_sub_url: "",
},
},
@ -298,7 +297,7 @@ add_task(async function test_setDefaultEngine() {
previous_engine_id: engine1.telemetryId,
new_engine_id: "other-engine2",
new_display_name: "engine2",
new_load_path: "[other]addEngineWithDetails:engine2@tests.mozilla.org",
new_load_path: "[addon]engine2@tests.mozilla.org",
new_submission_url: "",
},
},
@ -341,8 +340,7 @@ add_task(async function test_setPrivateDefaultEngine() {
prev_id: engine2.telemetryId,
new_id: "other-engine1",
new_name: "engine1",
new_load_path:
"[other]addEngineWithDetails:engine1@tests.mozilla.org",
new_load_path: "[addon]engine1@tests.mozilla.org",
new_sub_url: "",
},
},
@ -363,7 +361,7 @@ add_task(async function test_setPrivateDefaultEngine() {
previous_engine_id: engine2.telemetryId,
new_engine_id: "other-engine1",
new_display_name: "engine1",
new_load_path: "[other]addEngineWithDetails:engine1@tests.mozilla.org",
new_load_path: "[addon]engine1@tests.mozilla.org",
new_submission_url: "",
},
},

View File

@ -6,14 +6,12 @@ const TEST_PAGE_BASENAME = "contentSearchUI.html";
const TEST_ENGINE1 = {
name: "searchSuggestionEngine1",
id: "other-searchSuggestionEngine1",
loadPath:
"[other]addEngineWithDetails:searchsuggestionengine1@tests.mozilla.org",
loadPath: "[addon]searchsuggestionengine1@tests.mozilla.org",
};
const TEST_ENGINE2 = {
name: "searchSuggestionEngine2",
id: "other-searchSuggestionEngine2",
loadPath:
"[other]addEngineWithDetails:searchsuggestionengine2@tests.mozilla.org",
loadPath: "[addon]searchsuggestionengine2@tests.mozilla.org",
};
const TEST_MSG = "ContentSearchUIControllerTest";

View File

@ -45,7 +45,7 @@ export class AddonSearchEngine extends SearchEngine {
let id = extensionId + (details?.locale ?? json._locale);
super({
loadPath: "[other]addEngineWithDetails:" + extensionId,
loadPath: "[addon]" + extensionId,
isAppProvided,
id,
});

View File

@ -32,7 +32,7 @@ export class PolicySearchEngine extends SearchEngine {
let id = "policy-" + (options.details?.name ?? options.json._name);
super({
loadPath: "[other]addEngineWithDetails:set-via-policy",
loadPath: "[policy]",
id,
});

View File

@ -1620,7 +1620,7 @@ export class SearchService {
let engineSettings = settings.engines.find(
e => e.id == prevCurrentEngineId
);
if (engineSettings?._loadPath?.includes("set-via-policy")) {
if (engineSettings?._loadPath?.startsWith("[policy]")) {
return false;
}
}
@ -2179,10 +2179,10 @@ export class SearchService {
try {
let engine;
if (loadPath?.includes("set-via-policy")) {
if (loadPath?.startsWith("[policy]")) {
skippedEngines++;
continue;
} else if (loadPath?.includes("set-via-user")) {
} else if (loadPath?.startsWith("[user]")) {
engine = new lazy.UserSearchEngine({ json: engineJSON });
} else if (engineJSON.extensionID ?? engineJSON._extensionID) {
engine = new lazy.AddonSearchEngine({

View File

@ -163,6 +163,11 @@ export class SearchSettings {
Services.prefs.clearUserPref(prefName);
}
// Added in Firefox 110.
if (this.#settings.version < 8 && Array.isArray(this.#settings.engines)) {
this.#migrateTelemetryLoadPaths();
}
return structuredClone(json);
}
@ -550,6 +555,27 @@ export class SearchSettings {
}
}
/**
* Migrates telemetry load paths for versions of settings prior to v8.
*/
#migrateTelemetryLoadPaths() {
for (let engine of this.#settings.engines) {
if (!engine._loadPath) {
continue;
}
if (engine._loadPath.includes("set-via-policy")) {
engine._loadPath = "[policy]";
} else if (engine._loadPath.includes("set-via-user")) {
engine._loadPath = "[user]";
} else if (engine._loadPath.startsWith("[other]addEngineWithDetails:")) {
engine._loadPath = engine._loadPath.replace(
"[other]addEngineWithDetails:",
"[addon]"
);
}
}
}
/**
* Returns the engine associated with the name without SearchService
* initialization checks.

View File

@ -283,7 +283,7 @@ export var SearchUtils = {
* The current settings version.
*/
get SETTINGS_VERSION() {
return 7;
return 8;
},
/**

View File

@ -28,7 +28,7 @@ export class UserSearchEngine extends SearchEngine {
*/
constructor(options = {}) {
super({
loadPath: "[other]addEngineWithDetails:set-via-user",
loadPath: "[user]",
});
if (options.details) {

View File

@ -56,7 +56,7 @@ search.engine.default:
description: |
A path relating to where the search engine was installed/loaded from.
For example:
`[other]addEngineWithDetails:<extension id>` for a WebExtension based
`[addon]<extension id>` for a WebExtension based
engine.
`[https]developer.mozilla.org/mdn-web-docs.xml` for an OpenSearch based
engine.
@ -225,7 +225,7 @@ search.engine.private:
description: |
A path relating to where the search engine was installed/loaded from.
For example:
`[other]addEngineWithDetails:<extension id>` for a WebExtension based
`[addon]<extension id>` for a WebExtension based
engine.
`[https]developer.mozilla.org/mdn-web-docs.xml` for an OpenSearch based
engine.

View File

@ -0,0 +1,134 @@
{
"version": 7,
"engines": [
{
"id": "google@search.mozilla.orgdefault",
"_name": "Google",
"_isAppProvided": true,
"_metaData": { "order": 1 }
},
{
"id": "amazon@search.mozilla.orgen-GB",
"_name": "Amazon.co.uk",
"_isAppProvided": true,
"_metaData": { "order": 2 }
},
{
"id": "bing@search.mozilla.orgdefault",
"_name": "Bing",
"_isAppProvided": true,
"_metaData": { "order": 3 }
},
{
"id": "ddg@search.mozilla.orgdefault",
"_name": "DuckDuckGo",
"_isAppProvided": true,
"_metaData": { "order": 4 }
},
{
"id": "ebay@search.mozilla.orguk",
"_name": "eBay",
"_isAppProvided": true,
"_metaData": { "order": 5 }
},
{
"id": "wikipedia@search.mozilla.orgdefault",
"_name": "Wikipedia (en)",
"_isAppProvided": true,
"_metaData": { "order": 6 }
},
{
"id": "policy-Policy",
"_name": "Policy",
"_loadPath": "[other]addEngineWithDetails:set-via-policy",
"_metaData": { "alias": "PolicyAlias", "order": 6 }
},
{
"id": "bbc163e7-7b1a-47aa-a32c-c59062de2753",
"_name": "Bugzilla@Mozilla",
"_loadPath": "[https]bugzilla.mozilla.org/bugzillamozilla.xml",
"description": "Bugzilla@Mozilla Quick Search",
"_metaData": {
"loadPathHash": "Bxz6jVe3IIBxLLaafUus536LMyLKoGZm7xsBv/yiTw8=",
"order": 8,
"alias": "bugzillaAlias"
},
"_urls": [
{
"params": [],
"rels": [],
"template": "https://bugzilla.mozilla.org/buglist.cgi?quicksearch={searchTerms}"
}
],
"_orderHint": null,
"_telemetryId": null,
"_updateInterval": null,
"_updateURL": null,
"_iconUpdateURL": null,
"_extensionID": null,
"_locale": null,
"_definedAliases": []
},
{
"id": "bbc163e7-7b1a-47aa-a32c-c59062de2754",
"_name": "User",
"_loadPath": "[other]addEngineWithDetails:set-via-user",
"_metaData": {
"order": 9,
"alias": "UserAlias"
},
"_urls": [
{
"params": [],
"rels": [],
"template": "https://example.com/test?q={searchTerms}"
}
],
"_telemetryId": null,
"_updateInterval": null,
"_updateURL": null,
"_iconUpdateURL": null,
"_extensionID": null,
"_locale": null,
"_hasPreferredIcon": null
},
{
"id": "example@tests.mozilla.orgdefault",
"_name":"Example",
"_loadPath":"[other]addEngineWithDetails:example@tests.mozilla.org",
"description":null,
"_iconURL":"",
"_metaData":{},
"_urls":[
{
"params": [
{
"name": "q",
"value": "{searchTerms}"
}
],
"rels": [],
"template": "https://example.com/"}
],
"_telemetryId": null,
"_updateInterval": null,
"_updateURL": null,
"_iconUpdateURL": null,
"_extensionID": "example@tests.mozilla.org",
"_locale": "default",
"_definedAliases": [],
"_hasPreferredIcon": null
}
],
"metaData": {
"useSavedOrder": true,
"locale": "en-US",
"region": "GB",
"channel": "default",
"experiment": "",
"distroID": "",
"appDefaultEngine": "Google",
"current": "Bing",
"hash": "5Of6s1D+BDjPRti1wqtFyBTH1PnOf9n6cRwWlEXZhd0="
}
}

View File

@ -296,7 +296,7 @@ async function setupRemoteSettings() {
sinon.stub(settings, "get").returns([
{
id: "load-paths",
matches: ["[other]addEngineWithDetails:searchignore@mozilla.com"],
matches: ["[addon]searchignore@mozilla.com"],
_status: "synced",
},
{

View File

@ -56,14 +56,14 @@ add_task(async function test_defaultPrivateEngine() {
normal: {
engineId: "engine",
displayName: "Test search engine",
loadPath: "[other]addEngineWithDetails:engine@search.mozilla.org",
loadPath: "[addon]engine@search.mozilla.org",
submissionUrl: "https://www.google.com/search?q=",
verified: "default",
},
private: {
engineId: "engine-pref",
displayName: "engine-pref",
loadPath: "[other]addEngineWithDetails:engine-pref@search.mozilla.org",
loadPath: "[addon]engine-pref@search.mozilla.org",
submissionUrl: "https://www.google.com/search?q=",
verified: "default",
},
@ -92,15 +92,14 @@ add_task(async function test_defaultPrivateEngine() {
normal: {
engineId: "engine",
displayName: "Test search engine",
loadPath: "[other]addEngineWithDetails:engine@search.mozilla.org",
loadPath: "[addon]engine@search.mozilla.org",
submissionUrl: "https://www.google.com/search?q=",
verified: "default",
},
private: {
engineId: "engine-rel-searchform-purpose",
displayName: "engine-rel-searchform-purpose",
loadPath:
"[other]addEngineWithDetails:engine-rel-searchform-purpose@search.mozilla.org",
loadPath: "[addon]engine-rel-searchform-purpose@search.mozilla.org",
submissionUrl: "https://www.google.com/search?q=&channel=sb",
verified: "default",
},
@ -140,15 +139,14 @@ add_task(async function test_defaultPrivateEngine() {
normal: {
engineId: "engine",
displayName: "Test search engine",
loadPath: "[other]addEngineWithDetails:engine@search.mozilla.org",
loadPath: "[addon]engine@search.mozilla.org",
submissionUrl: "https://www.google.com/search?q=",
verified: "default",
},
private: {
engineId: "engine-chromeicon",
displayName: "engine-chromeicon",
loadPath:
"[other]addEngineWithDetails:engine-chromeicon@search.mozilla.org",
loadPath: "[addon]engine-chromeicon@search.mozilla.org",
submissionUrl: "https://www.google.com/search?q=",
verified: "default",
},

View File

@ -63,7 +63,7 @@ add_task(async function test_ignoreList() {
id: "load-paths",
schema: 1553857697843,
last_modified: 1553859483588,
matches: ["[other]addEngineWithDetails:searchignore@mozilla.com"],
matches: ["[addon]searchignore@mozilla.com"],
},
{
id: "submission-urls",

View File

@ -51,7 +51,7 @@ add_task(async function test_initialization_delayed_addon_manager() {
normal: {
engineId: "engine",
displayName: "Test search engine",
loadPath: "[other]addEngineWithDetails:engine@search.mozilla.org",
loadPath: "[addon]engine@search.mozilla.org",
submissionUrl: "https://www.google.com/search?q=",
verified: "default",
},

View File

@ -36,10 +36,7 @@ add_task(async function test_migrateLegacyEngine() {
);
engine = Services.search.getEngineByName("simple");
Assert.equal(
engine.wrappedJSObject._loadPath,
"[other]addEngineWithDetails:" + kExtensionID
);
Assert.equal(engine.wrappedJSObject._loadPath, "[addon]" + kExtensionID);
Assert.equal(engine.wrappedJSObject._extensionID, kExtensionID);
Assert.equal(
@ -79,10 +76,7 @@ add_task(async function test_migrateLegacyEngineDifferentName() {
// The engine should have changed its name.
engine = Services.search.getEngineByName("simple search");
Assert.equal(
engine.wrappedJSObject._loadPath,
"[other]addEngineWithDetails:" + kExtensionID
);
Assert.equal(engine.wrappedJSObject._loadPath, "[addon]" + kExtensionID);
Assert.equal(engine.wrappedJSObject._extensionID, kExtensionID);
Assert.equal(

View File

@ -375,7 +375,7 @@ for (const test of tests) {
{
defaultSearchEngine: "simple-addon",
defaultSearchEngineData: {
loadPath: "[other]addEngineWithDetails:simple@search.mozilla.org",
loadPath: "[addon]simple@search.mozilla.org",
name: "Simple Engine",
origin: "default",
submissionURL: test.expected.searchUrl.replace("{searchTerms}", ""),

View File

@ -93,7 +93,7 @@ add_task(async function test_enterprise_policy_engine() {
normal: {
engineId: "other-policy",
displayName: "policy",
loadPath: "[other]addEngineWithDetails:set-via-policy",
loadPath: "[policy]",
submissionUrl: "blank:",
verified: "verified",
},

View File

@ -0,0 +1,130 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
/*
* Test migration load path for user, enterprise policy and add-on
* engines.
*/
"use strict";
const { EnterprisePolicyTesting } = ChromeUtils.importESModule(
"resource://testing-common/EnterprisePolicyTesting.sys.mjs"
);
const enterprisePolicy = {
policies: {
SearchEngines: {
Add: [
{
Name: "Policy",
Encoding: "windows-1252",
URLTemplate: "http://example.com/?q={searchTerms}",
},
],
},
},
};
add_task(async function setup() {
// This initializes the policy engine for xpcshell tests
let policies = Cc["@mozilla.org/enterprisepolicies;1"].getService(
Ci.nsIObserver
);
policies.observe(null, "policies-startup", null);
Services.prefs
.getDefaultBranch(SearchUtils.BROWSER_SEARCH_PREF + "param.")
.setCharPref("test", "expected");
await SearchTestUtils.useTestEngines("data1");
await AddonTestUtils.promiseStartupManager();
await EnterprisePolicyTesting.setupPolicyEngineWithJson(enterprisePolicy);
// Setting the enterprise policy starts the search service initialising,
// so we wait for that to complete before starting the test, we can
// then also add an extra add-on engine.
await Services.search.init();
let settingsFileWritten = promiseAfterSettings();
await SearchTestUtils.installSearchExtension();
await settingsFileWritten;
});
/**
* Loads the settings file and ensures it has not already been migrated.
*/
add_task(async function test_load_and_check_settings() {
let settingsTemplate = await readJSONFile(
do_get_file("data/search-legacy-old-loadPaths.json")
);
Assert.less(
settingsTemplate.version,
8,
"Should be a version older than when indexing engines by id was introduced"
);
let engine = settingsTemplate.engines.find(e => e.id == "policy-Policy");
Assert.equal(
engine._loadPath,
"[other]addEngineWithDetails:set-via-policy",
"Should have a old style load path for the policy engine"
);
engine = settingsTemplate.engines.find(
e => e.id == "bbc163e7-7b1a-47aa-a32c-c59062de2754"
);
Assert.equal(
engine._loadPath,
"[other]addEngineWithDetails:set-via-user",
"Should have a old style load path for the user engine"
);
engine = settingsTemplate.engines.find(
e => e.id == "example@tests.mozilla.orgdefault"
);
Assert.equal(
engine._loadPath,
"[other]addEngineWithDetails:example@tests.mozilla.org",
"Should have a old style load path for the add-on engine"
);
await promiseSaveSettingsData(settingsTemplate);
});
/**
* Tests that an installed engine matches the expected data.
*
* @param {object} expectedData The expected data for the engine
*/
async function assertInstalledEngineMatches(expectedData) {
let engine = await Services.search.getEngineByName(expectedData.name);
Assert.ok(engine, `Should have found the ${expectedData.type} engine`);
Assert.equal(
engine.wrappedJSObject._loadPath,
expectedData.loadPath,
"Should have migrated the loadPath"
);
}
add_task(async function test_migration_from_pre_ids() {
const settingsFileWritten = promiseAfterSettings();
await Services.search.wrappedJSObject.reset();
await Services.search.init();
await settingsFileWritten;
await assertInstalledEngineMatches({
type: "Policy",
name: "Policy",
loadPath: "[policy]",
});
await assertInstalledEngineMatches({
type: "User",
name: "User",
loadPath: "[user]",
});
await assertInstalledEngineMatches({
type: "Add-on",
name: "Example",
loadPath: "[addon]example@tests.mozilla.org",
});
});

View File

@ -65,31 +65,31 @@ const MAIN_CONFIG = [
const testSearchEngine = {
id: "engine",
name: "Test search engine",
loadPath: "[other]addEngineWithDetails:engine@search.mozilla.org",
loadPath: "[addon]engine@search.mozilla.org",
submissionUrl: "https://www.google.com/search?q=",
};
const testChromeIconEngine = {
id: "engine-chromeicon",
name: "engine-chromeicon",
loadPath: "[other]addEngineWithDetails:engine-chromeicon@search.mozilla.org",
loadPath: "[addon]engine-chromeicon@search.mozilla.org",
submissionUrl: "https://www.google.com/search?q=",
};
const testFrEngine = {
id: "engine-fr",
name: "Test search engine (fr)",
loadPath: "[other]addEngineWithDetails:engine-fr@search.mozilla.org",
loadPath: "[addon]engine-fr@search.mozilla.org",
submissionUrl: "https://www.google.fr/search?q=&ie=iso-8859-1&oe=iso-8859-1",
};
const testPrefEngine = {
id: "engine-pref",
name: "engine-pref",
loadPath: "[other]addEngineWithDetails:engine-pref@search.mozilla.org",
loadPath: "[addon]engine-pref@search.mozilla.org",
submissionUrl: "https://www.google.com/search?q=",
};
const testEngine2 = {
id: "engine2",
name: "A second test engine",
loadPath: "[other]addEngineWithDetails:engine2@search.mozilla.org",
loadPath: "[addon]engine2@search.mozilla.org",
submissionUrl: "https://duckduckgo.com/?q=",
};

View File

@ -48,7 +48,7 @@ add_task(async function test_user_engine() {
normal: {
engineId: "other-user",
displayName: "user",
loadPath: "[other]addEngineWithDetails:set-via-user",
loadPath: "[user]",
submissionUrl: "blank:",
verified: "verified",
},

View File

@ -39,6 +39,7 @@ support-files =
data/search-legacy.json
data/search-legacy-correct-default-engine-hashes.json
data/search-legacy-no-ids.json
data/search-legacy-old-loadPaths.json
data/search-legacy-wrong-default-engine-hashes.json
data/search-obsolete-app.json
data/search-obsolete-distribution.json
@ -232,6 +233,7 @@ skip-if =
[test_settings_ignorelist.js]
support-files = data/search_ignorelist.json
[test_settings_migration_ids.js]
[test_settings_migration_loadPath.js]
[test_settings_none.js]
skip-if =
debug && socketprocess_networking # Bug 1759035

View File

@ -134,8 +134,7 @@ async function checkDefaultSearch(privateOn, reInitSearchService) {
Assert.equal(data.settings.defaultSearchEngine, "telemetrySearchIdentifier");
let expectedSearchEngineData = {
name: "telemetrySearchIdentifier",
loadPath:
"[other]addEngineWithDetails:telemetrySearchIdentifier@search.mozilla.org",
loadPath: "[addon]telemetrySearchIdentifier@search.mozilla.org",
origin: "default",
submissionURL:
"https://ar.wikipedia.org/wiki/%D8%AE%D8%A7%D8%B5:%D8%A8%D8%AD%D8%AB?search=&sourceId=Mozilla-search",
@ -214,7 +213,7 @@ async function checkDefaultSearch(privateOn, reInitSearchService) {
const EXPECTED_SEARCH_ENGINE = "other-" + SEARCH_ENGINE_ID;
const EXPECTED_SEARCH_ENGINE_DATA = {
name: SEARCH_ENGINE_ID,
loadPath: `[other]addEngineWithDetails:${SEARCH_ENGINE_ID}@test.engine`,
loadPath: `[addon]${SEARCH_ENGINE_ID}@test.engine`,
origin: "verified",
};
if (privateOn) {
@ -376,7 +375,7 @@ add_task(async function test_defaultSearchEngine_paramsChanged() {
TelemetryEnvironmentTesting.checkEnvironmentData(data);
Assert.deepEqual(data.settings.defaultSearchEngineData, {
name: "TestEngine",
loadPath: "[other]addEngineWithDetails:testengine@tests.mozilla.org",
loadPath: "[addon]testengine@tests.mozilla.org",
origin: "verified",
submissionURL: "https://www.google.com/fake1?q=",
});
@ -404,7 +403,7 @@ add_task(async function test_defaultSearchEngine_paramsChanged() {
TelemetryEnvironmentTesting.checkEnvironmentData(data);
Assert.deepEqual(data.settings.defaultSearchEngineData, {
name: "TestEngine",
loadPath: "[other]addEngineWithDetails:testengine@tests.mozilla.org",
loadPath: "[addon]testengine@tests.mozilla.org",
origin: "verified",
submissionURL: "https://www.google.com/fake2?q=",
});