Bug 1356593 - reimplement the search service's ParamSubstitution function to avoid doing unnecessary work, r=adw.
authorFlorian Queze <florian@queze.net>
Sat, 22 Apr 2017 02:08:54 +0200
changeset 566727 68e88b3e0e573392aba1e342b4dfd7bad9ecfeef
parent 566706 44ffc0867fbb501413696457535f3165cb26ef1f
child 566728 c1db9bb48b283e79f95ad3691b05737128e8088c
push id55308
push userbmo:rajesh.kathiriya507@gmail.com
push dateSat, 22 Apr 2017 10:15:05 +0000
reviewersadw
bugs1356593
milestone55.0a1
Bug 1356593 - reimplement the search service's ParamSubstitution function to avoid doing unnecessary work, r=adw.
toolkit/components/search/nsSearchService.js
toolkit/components/search/tests/xpcshell/test_paramSubstitution.js
toolkit/components/search/tests/xpcshell/xpcshell.ini
--- a/toolkit/components/search/nsSearchService.js
+++ b/toolkit/components/search/nsSearchService.js
@@ -135,52 +135,47 @@ const MOZSEARCH_LOCALNAME = "SearchPlugi
 
 const URLTYPE_SUGGEST_JSON = "application/x-suggestions+json";
 const URLTYPE_SEARCH_HTML  = "text/html";
 const URLTYPE_OPENSEARCH   = "application/opensearchdescription+xml";
 
 const BROWSER_SEARCH_PREF = "browser.search.";
 const LOCALE_PREF = "general.useragent.locale";
 
-const USER_DEFINED = "{searchTerms}";
+const USER_DEFINED = "searchTerms";
 
 // Custom search parameters
-const MOZ_OFFICIAL = AppConstants.MOZ_OFFICIAL_BRANDING ? "official" : "unofficial";
-
-const MOZ_PARAM_LOCALE         = /\{moz:locale\}/g;
-const MOZ_PARAM_DIST_ID        = /\{moz:distributionID\}/g;
-const MOZ_PARAM_OFFICIAL       = /\{moz:official\}/g;
+const MOZ_PARAM_LOCALE         = "moz:locale";
+const MOZ_PARAM_DIST_ID        = "moz:distributionID"
+const MOZ_PARAM_OFFICIAL       = "moz:official";
 
 // Supported OpenSearch parameters
 // See http://opensearch.a9.com/spec/1.1/querysyntax/#core
-const OS_PARAM_USER_DEFINED    = /\{searchTerms\??\}/g;
-const OS_PARAM_INPUT_ENCODING  = /\{inputEncoding\??\}/g;
-const OS_PARAM_LANGUAGE        = /\{language\??\}/g;
-const OS_PARAM_OUTPUT_ENCODING = /\{outputEncoding\??\}/g;
+const OS_PARAM_USER_DEFINED    = "searchTerms";
+const OS_PARAM_INPUT_ENCODING  = "inputEncoding";
+const OS_PARAM_LANGUAGE        = "language";
+const OS_PARAM_OUTPUT_ENCODING = "outputEncoding";
 
 // Default values
 const OS_PARAM_LANGUAGE_DEF         = "*";
 const OS_PARAM_OUTPUT_ENCODING_DEF  = "UTF-8";
 const OS_PARAM_INPUT_ENCODING_DEF   = "UTF-8";
 
 // "Unsupported" OpenSearch parameters. For example, we don't support
 // page-based results, so if the engine requires that we send the "page index"
 // parameter, we'll always send "1".
-const OS_PARAM_COUNT        = /\{count\??\}/g;
-const OS_PARAM_START_INDEX  = /\{startIndex\??\}/g;
-const OS_PARAM_START_PAGE   = /\{startPage\??\}/g;
+const OS_PARAM_COUNT        = "count";
+const OS_PARAM_START_INDEX  = "startIndex";
+const OS_PARAM_START_PAGE   = "startPage";
 
 // Default values
 const OS_PARAM_COUNT_DEF        = "20"; // 20 results
 const OS_PARAM_START_INDEX_DEF  = "1";  // start at 1st result
 const OS_PARAM_START_PAGE_DEF   = "1";  // 1st page
 
-// Optional parameter
-const OS_PARAM_OPTIONAL     = /\{(?:\w+:)?\w+\?\}/g;
-
 // A array of arrays containing parameters that we don't fully support, and
 // their default values. We will only send values for these parameters if
 // required, since our values are just really arbitrary "guesses" that should
 // give us the output we want.
 var OS_UNSUPPORTED_PARAMS = [
   [OS_PARAM_COUNT, OS_PARAM_COUNT_DEF],
   [OS_PARAM_START_INDEX, OS_PARAM_START_INDEX_DEF],
   [OS_PARAM_START_PAGE, OS_PARAM_START_PAGE_DEF],
@@ -984,54 +979,63 @@ function QueryParameter(aName, aValue, a
  *        This value must already be escaped appropriately - it is inserted
  *        as-is.
  * @param aEngine
  *        The engine which owns the string being acted on.
  *
  * @see http://opensearch.a9.com/spec/1.1/querysyntax/#core
  */
 function ParamSubstitution(aParamValue, aSearchTerms, aEngine) {
-  var value = aParamValue;
-
-  var distributionID =
-    Services.prefs.getCharPref(BROWSER_SEARCH_PREF + "distributionID",
-                               Services.appinfo.distributionID || "");
-
-  var official;
-  if (Services.prefs.getBoolPref(BROWSER_SEARCH_PREF + "official", MOZ_OFFICIAL))
-    official = "official";
-  else
-    official = "unofficial";
-
-  // Custom search parameters. These are only available to default search
-  // engines.
-  if (aEngine._isDefault) {
-    value = value.replace(MOZ_PARAM_LOCALE, getLocale());
-    value = value.replace(MOZ_PARAM_DIST_ID, distributionID);
-    value = value.replace(MOZ_PARAM_OFFICIAL, official);
-  }
-
-  // Insert the OpenSearch parameters we're confident about
-  value = value.replace(OS_PARAM_USER_DEFINED, aSearchTerms);
-  value = value.replace(OS_PARAM_INPUT_ENCODING, aEngine.queryCharset);
-  value = value.replace(OS_PARAM_LANGUAGE,
-                        getLocale() || OS_PARAM_LANGUAGE_DEF);
-  value = value.replace(OS_PARAM_OUTPUT_ENCODING,
-                        OS_PARAM_OUTPUT_ENCODING_DEF);
-
-  // Replace any optional parameters
-  value = value.replace(OS_PARAM_OPTIONAL, "");
-
-  // Insert any remaining required params with our default values
-  for (var i = 0; i < OS_UNSUPPORTED_PARAMS.length; ++i) {
-    value = value.replace(OS_UNSUPPORTED_PARAMS[i][0],
-                          OS_UNSUPPORTED_PARAMS[i][1]);
-  }
-
-  return value;
+  const PARAM_REGEXP = /\{((?:\w+:)?\w+)(\??)\}/g;
+  return aParamValue.replace(PARAM_REGEXP, function(match, name, optional) {
+    // {searchTerms} is by far the most common param so handle it first.
+    if (name == USER_DEFINED)
+      return aSearchTerms;
+
+    // {inputEncoding} is the second most common param.
+    if (name == OS_PARAM_INPUT_ENCODING)
+      return aEngine.queryCharset;
+
+    // moz: parameters are only available for default search engines.
+    if (name.startsWith("moz:") && aEngine._isDefault) {
+      // {moz:locale} and {moz:distributionID} are common
+      if (name == MOZ_PARAM_LOCALE)
+        return getLocale();
+      if (name == MOZ_PARAM_DIST_ID) {
+        return Services.prefs.getCharPref(BROWSER_SEARCH_PREF + "distributionID",
+                                          Services.appinfo.distributionID || "");
+      }
+      // {moz:official} seems to have little use.
+      if (name == MOZ_PARAM_OFFICIAL) {
+        if (Services.prefs.getBoolPref(BROWSER_SEARCH_PREF + "official",
+                                       AppConstants.MOZ_OFFICIAL_BRANDING))
+          return "official";
+        return "unofficial";
+      }
+    }
+
+    // Handle the less common OpenSearch parameters we're confident about.
+    if (name == OS_PARAM_LANGUAGE)
+      return getLocale() || OS_PARAM_LANGUAGE_DEF;
+    if (name == OS_PARAM_OUTPUT_ENCODING)
+      return OS_PARAM_OUTPUT_ENCODING_DEF;
+
+    // At this point, if a parameter is optional, just omit it.
+    if (optional)
+      return "";
+
+    // Replace unsupported parameters that only have hardcoded default values.
+    for (let param of OS_UNSUPPORTED_PARAMS) {
+      if (name == param[0])
+        return param[1];
+    }
+
+    // Don't replace unknown non-optional parameters.
+    return match;
+  });
 }
 
 /**
  * Creates an engineURL object, which holds the query URL and all parameters.
  *
  * @param aType
  *        A string containing the name of the MIME type of the search results
  *        returned by this URL.
@@ -1143,21 +1147,21 @@ EngineURL.prototype = {
 
       postData = Cc["@mozilla.org/network/mime-input-stream;1"].
                  createInstance(Ci.nsIMIMEInputStream);
       postData.addHeader("Content-Type", "application/x-www-form-urlencoded");
       postData.addContentLength = true;
       postData.setData(stringStream);
     }
 
-    return new Submission(makeURI(url), postData);
+    return new Submission(Services.io.newURI(url), postData);
   },
 
   _getTermsParameterName: function SRCH_EURL__getTermsParameterName() {
-    let queryParam = this.params.find(p => p.value == USER_DEFINED);
+    let queryParam = this.params.find(p => p.value == "{" + USER_DEFINED + "}");
     return queryParam ? queryParam.name : "";
   },
 
   _hasRelation: function SRC_EURL__hasRelation(aRel) {
     return this.rels.some(e => e == aRel.toLowerCase());
   },
 
   _initWithJSON: function SRC_EURL__initWithJSON(aJson, aEngine) {
@@ -1490,19 +1494,19 @@ Engine.prototype = {
    * "type" attribute in the "Url" node in the OpenSearch spec.)
    * This method will return the first matching URL object found, or null
    * if no matching URL is found.
    *
    * @param aType string to match the EngineURL's type attribute
    * @param aRel [optional] only return URLs that with this rel value
    */
   _getURLOfType: function SRCH_ENG__getURLOfType(aType, aRel) {
-    for (var i = 0; i < this._urls.length; ++i) {
-      if (this._urls[i].type == aType && (!aRel || this._urls[i]._hasRelation(aRel)))
-        return this._urls[i];
+    for (let url of this._urls) {
+      if (url.type == aType && (!aRel || url._hasRelation(aRel)))
+        return url;
     }
 
     return null;
   },
 
   _confirmAddEngine: function SRCH_SVC_confirmAddEngine() {
     var stringBundle = Services.strings.createBundle(SEARCH_BUNDLE);
     var titleMessage = stringBundle.GetStringFromName("addEngineConfirmTitle");
new file mode 100644
--- /dev/null
+++ b/toolkit/components/search/tests/xpcshell/test_paramSubstitution.js
@@ -0,0 +1,67 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+"use strict";
+
+function run_test() {
+  useHttpServer();
+
+  run_next_test();
+}
+
+add_task(function* test_paramSubstitution() {
+  yield asyncInit();
+
+  let prefix = "http://test.moz/search?q=";
+  let [engine] = yield addTestEngines([
+    { name: "test", details: ["", "test", "Search Test", "GET",
+                              prefix + "{searchTerms}"] },
+  ]);
+  let url = engine.wrappedJSObject._getURLOfType("text/html");
+  equal(url.template, prefix + "{searchTerms}");
+
+  let searchTerms = "fxsearch";
+  function check(template, expected) {
+    url.template = prefix + template;
+    equal(engine.getSubmission(searchTerms).uri.spec, prefix + expected);
+  }
+
+  // The same parameter can be used more than once.
+  check("{searchTerms}/{searchTerms}", searchTerms + "/" + searchTerms);
+
+  // Optional parameters are replaced if we known them.
+  check("{searchTerms?}", searchTerms);
+  check("{unknownOptional?}", "");
+  check("{unknownRequired}", "{unknownRequired}");
+
+  check("{language}", Services.locale.getRequestedLocale());
+  check("{language?}", Services.locale.getRequestedLocale());
+
+  engine.wrappedJSObject._queryCharset = "UTF-8";
+  check("{inputEncoding}", "UTF-8");
+  check("{inputEncoding?}", "UTF-8");
+  check("{outputEncoding}", "UTF-8");
+  check("{outputEncoding?}", "UTF-8");
+
+  // 'Unsupported' parameters with hard coded values used only when the parameter is required.
+  check("{count}", "20");
+  check("{count?}", "");
+  check("{startIndex}", "1");
+  check("{startIndex?}", "");
+  check("{startPage}", "1");
+  check("{startPage?}", "");
+
+  // Test moz: parameters (only supported for built-in engines, ie _isDefault == true).
+  check("{moz:distributionID}", "{moz:distributionID}");
+  check("{moz:official}", "{moz:official}");
+  check("{moz:locale}", "{moz:locale}");
+  engine.wrappedJSObject._loadPath = "[app]"; // This will make _isDefault return true;
+  check("{moz:distributionID}", "");
+  Services.prefs.setCharPref("browser.search.distributionID", "xpcshell");
+  check("{moz:distributionID}", "xpcshell");
+  Services.prefs.setBoolPref("browser.search.official", true);
+  check("{moz:official}", "official");
+  Services.prefs.setBoolPref("browser.search.official", false);
+  check("{moz:official}", "unofficial");
+  check("{moz:locale}", Services.locale.getRequestedLocale());
+});
--- a/toolkit/components/search/tests/xpcshell/xpcshell.ini
+++ b/toolkit/components/search/tests/xpcshell/xpcshell.ini
@@ -94,8 +94,9 @@ tags = addons
 [test_currentEngine_fallback.js]
 [test_require_engines_in_cache.js]
 [test_svg_icon.js]
 [test_searchReset.js]
 [test_addEngineWithDetails.js]
 [test_chromeresource_icon1.js]
 [test_chromeresource_icon2.js]
 [test_engineUpdate.js]
+[test_paramSubstitution.js]