Bug 1228258 - Search aliases should be trimmed. r=mak
authorgasolin <gasolin@gmail.com>
Mon, 25 Apr 2016 15:19:36 +0800
changeset 296379 ce2501a5f268283105549cc5316a35ca938c099f
parent 296378 2451d15ff23433445b43eee9ceb565b7d8750029
child 296380 26c8641039836d44fde70b1887116b22febdc58a
push id76311
push usercbook@mozilla.com
push dateFri, 06 May 2016 12:26:12 +0000
treeherdermozilla-inbound@84a3e5716801 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1228258
milestone49.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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]