From 39f306a329b288e33b53327e1f939ad5a18105bf Mon Sep 17 00:00:00 2001 From: Coroiu Cristina Date: Thu, 28 Nov 2019 17:26:07 +0200 Subject: [PATCH] Backed out 3 changesets (bug 1598571) for browser-chrome failures at browser/components/search/test/browser/browser_amazon.js on a CLOSED TREE Backed out changeset 6426d4607db0 (bug 1598571) Backed out changeset ff2c15cb11b4 (bug 1598571) Backed out changeset 290a5cd61260 (bug 1598571) --- toolkit/components/search/SearchEngine.jsm | 225 ++++++------------ .../search/tests/xpcshell/test_pref.js | 56 +---- 2 files changed, 81 insertions(+), 200 deletions(-) diff --git a/toolkit/components/search/SearchEngine.jsm b/toolkit/components/search/SearchEngine.jsm index 610c3f889cd4..5b65aa7fdbca 100644 --- a/toolkit/components/search/SearchEngine.jsm +++ b/toolkit/components/search/SearchEngine.jsm @@ -299,120 +299,38 @@ function sanitizeName(name) { } /** - * A simple class to handle caching of preferences that may be read from - * parameters. - */ -const ParamPreferenceCache = { - QueryInterface: ChromeUtils.generateQI([ - Ci.nsIObserver, - Ci.nsISupportsWeakReference, - ]), - - initCache() { - this.branch = Services.prefs.getDefaultBranch( - SearchUtils.BROWSER_SEARCH_PREF + "param." - ); - this.cache = new Map(); - for (let prefName of this.branch.getChildList("")) { - this.cache.set(prefName, this.branch.getCharPref(prefName, null)); - } - this.branch.addObserver("", this, true); - }, - - observe(subject, topic, data) { - this.cache.set(data, this.branch.getCharPref(data, null)); - }, - - getPref(prefName) { - if (!this.cache) { - this.initCache(); - } - return this.cache.get(prefName); - }, -}; - -/** - * Represents a name/value pair for a parameter - * @see nsISearchEngine::addParam - */ -class QueryParameter { - /** - * @see nsISearchEngine::addParam - * @param {string} name - * @param {string} value - * @param {string} purpose - * The search purpose for which matches when this parameter should be - * applied, e.g. "searchbar", "contextmenu". - */ - constructor(name, value, purpose) { - if (!name || value == null) { - SearchUtils.fail("missing name or value for QueryParameter!"); - } - - this.name = name; - this._value = value; - this.purpose = purpose; - } - - get value() { - return this._value; - } - - toJSON() { - const result = { - name: this.name, - value: this.value, - }; - if (this.purpose) { - result.purpose = this.purpose; - } - return result; - } + * Retrieve a pref from the search param branch. Returns null if the + * preference is not found. + * + * @param {string} prefName + * The name of the pref. + * @returns {string|null} + * The value of the preference. + **/ +function getMozParamPref(prefName) { + let branch = Services.prefs.getDefaultBranch( + SearchUtils.BROWSER_SEARCH_PREF + "param." + ); + let prefValue = branch.getCharPref(prefName, null); + return prefValue ? encodeURIComponent(prefValue) : null; } /** - * Represents a special paramater that can be set by preferences. The - * value is read from the 'browser.search.param.*' default preference - * branch. + * Simple object representing a name/value pair. + * @see nsISearchEngine::addParam + * + * @param {string} name + * @param {string} value + * @param {string} purpose */ -class QueryPreferenceParameter extends QueryParameter { - /** - * @param {string} name - * The name of the parameter as injected into the query string. - * @param {string} prefName - * The name of the preference to read from the branch. - * @param {string} purpose - * The search purpose for which matches when this parameter should be - * applied, e.g. `searchbar`, `contextmenu`. - */ - constructor(name, prefName, purpose) { - super(name, prefName, purpose); +function QueryParameter(name, value, purpose) { + if (!name || value == null) { + SearchUtils.fail("missing name or value for QueryParameter!"); } - get value() { - const prefValue = ParamPreferenceCache.getPref(this._value); - return prefValue ? encodeURIComponent(prefValue) : null; - } - - /** - * Converts the object to json. This object is converted with a mozparam flag - * as it gets written to the cache and hence we then know what type it is - * when reading it back. - * - * @returns {object} - */ - toJSON() { - const result = { - condition: "pref", - mozparam: true, - name: this.name, - pref: this._value, - }; - if (this.purpose) { - result.purpose = this.purpose; - } - return result; - } + this.name = name; + this.value = value; + this.purpose = purpose; } /** @@ -558,6 +476,8 @@ function EngineURL(mimeType, requestMethod, template, resultDomain) { this.method = method; this.params = []; this.rels = []; + // Don't serialize expanded mozparams + this.mozparams = {}; var templateURI = SearchUtils.makeURI(template); if (!templateURI) { @@ -592,36 +512,11 @@ EngineURL.prototype = { this.params.push(new QueryParameter(name, value, purpose)); }, - /** - * Adds a MozParam to the parameters list for this URL. For purpose based params - * these are saved as standard parameters, for preference based we save them - * as a special type. - * - * @param {object} param - * @param {string} param.name - * The name of the parameter to add to the url. - * @param {string} [param.condition] - * The type of parameter this is, e.g. "pref" for a preference parameter, - * or "purpose" for a value-based parameter with a specific purpose. The - * default is "purpose". - * @param {string} [param.value] - * The value if it is a "purpose" parameter. - * @param {string} [param.purpose] - * The purpose of the parameter for when it is applied, e.g. for `searchbar` - * searches. - * @param {string} [param.pref] - * The preference name of the parameter, that gets appended to - * `browser.search.param.`. - */ - _addMozParam(param) { - const purpose = param.purpose || undefined; - if (param.condition && param.condition == "pref") { - this.params.push( - new QueryPreferenceParameter(param.name, param.pref, purpose) - ); - } else { - this.addParam(param.name, param.value || undefined, purpose); - } + // Note: This method requires that aObj has a unique name or the previous MozParams entry with + // that name will be overwritten. + _addMozParam(obj) { + obj.mozparam = true; + this.mozparams[obj.name] = obj; }, getSubmission(searchTerms, engine, purpose) { @@ -631,8 +526,9 @@ EngineURL.prototype = { // If a particular purpose isn't defined in the plugin, fallback to 'searchbar'. if ( - requestPurpose != "searchbar" && - !this.params.some(p => p.purpose && p.purpose == requestPurpose) + !this.params.some( + p => p.purpose !== undefined && p.purpose == requestPurpose + ) ) { requestPurpose = "searchbar"; } @@ -644,17 +540,13 @@ EngineURL.prototype = { var param = this.params[i]; // If this parameter has a purpose, only add it if the purpose matches - if (param.purpose && param.purpose != requestPurpose) { + if (param.purpose !== undefined && param.purpose != requestPurpose) { continue; } - const paramValue = param.value; - // Preference MozParams might not have a preferenced saved, or a value. - if (paramValue) { - var value = ParamSubstitution(paramValue, searchTerms, engine); + var value = ParamSubstitution(param.value, searchTerms, engine); - dataArray.push(param.name + "=" + value); - } + dataArray.push(param.name + "=" + value); } let dataString = dataArray.join("&"); @@ -706,6 +598,12 @@ EngineURL.prototype = { for (let i = 0; i < json.params.length; ++i) { let param = json.params[i]; if (param.mozparam) { + if (param.condition == "pref") { + let value = getMozParamPref(param.pref); + if (value) { + this.addParam(param.name, value); + } + } this._addMozParam(param); } else { this.addParam(param.name, param.value, param.purpose || undefined); @@ -721,10 +619,9 @@ EngineURL.prototype = { */ toJSON() { var json = { - params: this.params, + template: this.template, rels: this.rels, resultDomain: this.resultDomain, - template: this.template, }; if (this.type != SearchUtils.URL_TYPE.SEARCH) { @@ -734,6 +631,11 @@ EngineURL.prototype = { json.method = this.method; } + function collapseMozParams(param) { + return this.mozparams[param.name] || param; + } + json.params = this.params.map(collapseMozParams, this); + return json; }, }; @@ -1462,7 +1364,15 @@ SearchEngine.prototype = { if ((p.condition || p.purpose) && !this._isDefault) { continue; } - url._addMozParam(p); + if (p.condition == "pref") { + let value = getMozParamPref(p.pref); + if (value) { + url.addParam(p.name, value); + } + url._addMozParam(p); + } else { + url.addParam(p.name, p.value, p.purpose || undefined); + } } } @@ -1596,6 +1506,7 @@ SearchEngine.prototype = { // We only support MozParams for default search engines this._isDefault ) { + var value; let condition = param.getAttribute("condition"); // MozParams must have a condition to be valid @@ -1611,10 +1522,6 @@ SearchEngine.prototype = { continue; } - // We can't make these both use _addMozParam due to the fallback - // handling - WebExtension parameters get treated as MozParams even - // if they are not, and hence don't have the condition parameter, so - // we can't warn for them. switch (condition) { case "purpose": url.addParam( @@ -1622,8 +1529,14 @@ SearchEngine.prototype = { param.getAttribute("value"), param.getAttribute("purpose") ); + // _addMozParam is not needed here since it can be serialized fine without. _addMozParam + // also requires a unique "name" which is not normally the case when @purpose is used. break; case "pref": + value = getMozParamPref(param.getAttribute("pref"), value); + if (value) { + url.addParam(param.getAttribute("name"), value); + } url._addMozParam({ pref: param.getAttribute("pref"), name: param.getAttribute("name"), @@ -2071,14 +1984,14 @@ SearchEngine.prototype = { return this.__internalAliases; }, - _getSearchFormWithPurpose(purpose) { + _getSearchFormWithPurpose(aPurpose = "") { // First look for a var searchFormURL = this._getURLOfType( SearchUtils.URL_TYPE.SEARCH, "searchform" ); if (searchFormURL) { - let submission = searchFormURL.getSubmission("", this, purpose); + let submission = searchFormURL.getSubmission("", this, aPurpose); // If the rel=searchform URL is not type="get" (i.e. has postData), // ignore it, since we can only return a URL. diff --git a/toolkit/components/search/tests/xpcshell/test_pref.js b/toolkit/components/search/tests/xpcshell/test_pref.js index 2487fcf9e5f9..b5da40a190e1 100644 --- a/toolkit/components/search/tests/xpcshell/test_pref.js +++ b/toolkit/components/search/tests/xpcshell/test_pref.js @@ -6,12 +6,7 @@ "use strict"; -const defaultBranch = Services.prefs.getDefaultBranch( - SearchUtils.BROWSER_SEARCH_PREF -); -const baseURL = "https://www.google.com/search?q=foo"; - -add_task(async function setup() { +function run_test() { // The test engines used in this test need to be recognized as 'default' // engines, or their MozParams will be ignored. let url = "resource://test/data/"; @@ -19,9 +14,14 @@ add_task(async function setup() { .getProtocolHandler("resource") .QueryInterface(Ci.nsIResProtocolHandler); resProt.setSubstitution("search-extensions", Services.io.newURI(url)); -}); -add_task(async function test_pref_initial_value() { + run_next_test(); +} + +add_task(async function test_pref() { + let defaultBranch = Services.prefs.getDefaultBranch( + SearchUtils.BROWSER_SEARCH_PREF + ); defaultBranch.setCharPref("param.code", "good&id=unique"); Services.prefs.setCharPref( SearchUtils.BROWSER_SEARCH_PREF + "param.code", @@ -31,44 +31,12 @@ add_task(async function test_pref_initial_value() { await AddonTestUtils.promiseStartupManager(); await Services.search.init(); - const engine = Services.search.getEngineByName("engine-pref"); - const base = baseURL + "&code="; - Assert.equal( - engine.getSubmission("foo").uri.spec, - base + "good%26id%3Dunique", - "Should have got the submission URL with the correct code" - ); - - // Now clear the user-set preference. Having a user set preference means - // we don't get updates from the pref service of changes on the default - // branch. Normally, this won't be an issue, since we don't expect users - // to be playing with these prefs, and worst-case, they'll just get the - // actual change on restart. - Services.prefs.clearUserPref(SearchUtils.BROWSER_SEARCH_PREF + "param.code"); -}); - -add_task(async function test_pref_updated() { - // Update the pref without re-init nor restart. - defaultBranch.setCharPref("param.code", "supergood&id=unique123456"); - - const engine = Services.search.getEngineByName("engine-pref"); - const base = baseURL + "&code="; - Assert.equal( - engine.getSubmission("foo").uri.spec, - base + "supergood%26id%3Dunique123456", - "Should have got the submission URL with the updated code" - ); -}); - -add_task(async function test_pref_cleared() { - // Update the pref without re-init nor restart. - // Note you can't delete a preference from the default branch. - defaultBranch.setCharPref("param.code", ""); - let engine = Services.search.getEngineByName("engine-pref"); + let base = "https://www.google.com/search?q=foo&code="; Assert.equal( engine.getSubmission("foo").uri.spec, - baseURL, - "Should have just the base URL after the pref was cleared" + base + "good%26id%3Dunique" ); + + do_test_finished(); });