Bug 1228258 - search aliases should be trimmed; r?mak draft
authorgasolin <gasolin@gmail.com>
Mon, 25 Apr 2016 15:19:36 +0800
changeset 363649 793d5103d7baf78b922ffb26e16a50e9ec49ddeb
parent 362784 fd84d2a8fe7c488456b52d65087f993de5d262bc
child 520101 8c93674ef9365b7c656e042f81c2dbd1a2b4bc32
push id17271
push userbmo:gasolin@mozilla.com
push dateThu, 05 May 2016 07:46:42 +0000
reviewersmak
bugs1228258
milestone49.0a1
Bug 1228258 - search aliases should be trimmed; r?mak MozReview-Commit-ID: CzTU7aBnIRd MozReview-Commit-ID: 2OzxRTtr39b
browser/components/preferences/in-content/search.js
toolkit/components/search/nsSearchService.js
toolkit/components/search/tests/xpcshell/test_engine_set_alias.js
toolkit/components/search/tests/xpcshell/xpcshell.ini
--- a/browser/components/preferences/in-content/search.js
+++ b/browser/components/preferences/in-content/search.js
@@ -237,27 +237,28 @@ var gSearchPane = {
     gEngineView.rowCountChanged(index, -1);
     gEngineView.invalidate();
     gEngineView.selection.select(Math.min(index, gEngineView.lastIndex));
     gEngineView.ensureRowIsVisible(gEngineView.currentIndex);
     document.getElementById("engineList").focus();
   },
 
   editKeyword: Task.async(function* (aEngine, aNewKeyword) {
-    if (aNewKeyword) {
+    let keyword = aNewKeyword.trim();
+    if (keyword) {
       let eduplicate = false;
       let dupName = "";
 
       // Check for duplicates in Places keywords.
-      let bduplicate = !!(yield PlacesUtils.keywords.fetch(aNewKeyword));
+      let bduplicate = !!(yield PlacesUtils.keywords.fetch(keyword));
 
       // Check for duplicates in changes we haven't committed yet
       let engines = gEngineView._engineStore.engines;
       for (let engine of engines) {
-        if (engine.alias == aNewKeyword &&
+        if (engine.alias == keyword &&
             engine.name != aEngine.name) {
           eduplicate = true;
           dupName = engine.name;
           break;
         }
       }
 
       // Notify the user if they have chosen an existing engine/bookmark keyword
@@ -267,17 +268,17 @@ var gSearchPane = {
         let bmsg = strings.getString("duplicateBookmarkMsg");
         let emsg = strings.getFormattedString("duplicateEngineMsg", [dupName]);
 
         Services.prompt.alert(window, dtitle, eduplicate ? emsg : bmsg);
         return false;
       }
     }
 
-    gEngineView._engineStore.changeEngine(aEngine, "alias", aNewKeyword);
+    gEngineView._engineStore.changeEngine(aEngine, "alias", keyword);
     gEngineView.invalidate();
     return true;
   }),
 
   saveOneClickEnginesList: function () {
     let hiddenList = [];
     for (let engine of gEngineView._engineStore.engines) {
       if (!engine.shown)
--- a/toolkit/components/search/nsSearchService.js
+++ b/toolkit/components/search/nsSearchService.js
@@ -979,17 +979,17 @@ function getMozParamPref(prefName) {
 var gInitialized = false;
 function notifyAction(aEngine, aVerb) {
   if (gInitialized) {
     LOG("NOTIFY: Engine: \"" + aEngine.name + "\"; Verb: \"" + aVerb + "\"");
     Services.obs.notifyObservers(aEngine, SEARCH_ENGINE_TOPIC, aVerb);
   }
 }
 
-function  parseJsonFromStream(aInputStream) {
+function parseJsonFromStream(aInputStream) {
   const json = Cc["@mozilla.org/dom/json;1"].createInstance(Ci.nsIJSON);
   const data = json.decodeFromStream(aInputStream, aInputStream.available());
   return data;
 }
 
 /**
  * Simple object representing a name/value pair.
  */
@@ -2130,17 +2130,18 @@ Engine.prototype = {
     return this._metaData[name] || undefined;
   },
 
   // nsISearchEngine
   get alias() {
     return this.getAttr("alias");
   },
   set alias(val) {
-    this.setAttr("alias", val);
+    var value = val ? val.trim() : null;
+    this.setAttr("alias", value);
     notifyAction(this, SEARCH_ENGINE_CHANGED);
   },
 
   /**
    * Return the built-in identifier of app-provided engines.
    *
    * Note that this identifier is substantially similar to _id, with the
    * following exceptions:
new file mode 100644
--- /dev/null
+++ b/toolkit/components/search/tests/xpcshell/test_engine_set_alias.js
@@ -0,0 +1,80 @@
+"use strict";
+
+function run_test() {
+  useHttpServer();
+
+  run_next_test();
+}
+
+add_task(function* test_engine_set_alias() {
+  yield asyncInit();
+  do_print("Set engine alias");
+  let [engine1] = yield addTestEngines([
+    {
+      name: "bacon",
+      details: ["", "b", "Search Bacon", "GET", "http://www.bacon.test/find"]
+    }
+  ]);
+  Assert.equal(engine1.alias, "b");
+  engine1.alias = "a";
+  Assert.equal(engine1.alias, "a");
+  Services.search.removeEngine(engine1);
+});
+
+add_task(function* test_engine_set_alias_with_left_space() {
+  do_print("Set engine alias with left space");
+  let [engine2] = yield addTestEngines([
+    {
+      name: "bacon",
+      details: ["", "   a", "Search Bacon", "GET", "http://www.bacon.test/find"]
+    }
+  ]);
+  Assert.equal(engine2.alias, "a");
+  engine2.alias = "    c";
+  Assert.equal(engine2.alias, "c");
+  Services.search.removeEngine(engine2);
+});
+
+add_task(function* test_engine_set_alias_with_right_space() {
+  do_print("Set engine alias with right space");
+  let [engine3] = yield addTestEngines([
+    {
+      name: "bacon",
+      details: ["", "c   ", "Search Bacon", "GET", "http://www.bacon.test/find"]
+    }
+  ]);
+  Assert.equal(engine3.alias, "c");
+  engine3.alias = "o    ";
+  Assert.equal(engine3.alias, "o");
+  Services.search.removeEngine(engine3);
+});
+
+add_task(function* test_engine_set_alias_with_right_left_space() {
+  do_print("Set engine alias with left and right space");
+  let [engine4] = yield addTestEngines([
+    {
+      name: "bacon",
+      details: ["", " o  ", "Search Bacon", "GET", "http://www.bacon.test/find"]
+    }
+  ]);
+  Assert.equal(engine4.alias, "o");
+  engine4.alias = "  n ";
+  Assert.equal(engine4.alias, "n");
+  Services.search.removeEngine(engine4);
+});
+
+add_task(function* test_engine_set_alias_with_space() {
+  do_print("Set engine alias with space");
+  let [engine5] = yield addTestEngines([
+    {
+      name: "bacon",
+      details: ["", " ", "Search Bacon", "GET", "http://www.bacon.test/find"]
+    }
+  ]);
+  Assert.equal(engine5.alias, null);
+  engine5.alias = "b";
+  Assert.equal(engine5.alias, "b");
+  engine5.alias = "  ";
+  Assert.equal(engine5.alias, null);
+  Services.search.removeEngine(engine5);
+});
--- a/toolkit/components/search/tests/xpcshell/xpcshell.ini
+++ b/toolkit/components/search/tests/xpcshell/xpcshell.ini
@@ -30,16 +30,17 @@ support-files =
   data/search.sqlite
   data/searchSuggestions.sjs
   data/searchTest.jar
 
 [test_nocache.js]
 [test_645970.js]
 [test_bug930456.js]
 [test_bug930456_child.js]
+[test_engine_set_alias.js]
 [test_identifiers.js]
 [test_invalid_engine_from_dir.js]
 [test_init_async_multiple.js]
 [test_init_async_multiple_then_sync.js]
 [test_json_cache.js]
 [test_location.js]
 [test_location_error.js]
 [test_location_malformed_json.js]