Bug 1059846 - Search engine may autofill in-the-middle. r=ttaubert

toolkit/components/autocomplete/nsAutoCompleteController.cpp

@@ -1583,8 +1583,6 @@ nsAutoCompleteController::GetFinalDefaultCompleteValue(nsAString &_retval)
     _retval = finalCompleteValue;
   }
 
-  MOZ_ASSERT(FindInReadable(inputValue, _retval, nsCaseInsensitiveStringComparator()),
-             "Return value must include input value.");
   return NS_OK;
 }
 

toolkit/components/places/UnifiedComplete.js

@@ -730,9 +730,40 @@ Search.prototype = {
     let match = yield PlacesSearchAutocompleteProvider.findMatchByToken(
                                                            this._searchString);
     if (match) {
+      // The match doesn't contain a 'scheme://www.' prefix, but since we have
+      // stripped it from the search string, here we could still be matching
+      // 'https://www.g' to 'google.com'.
+      // There are a couple cases where we don't want to match though:
+      //
+      //  * If the protocol differs we should not match. For example if the user
+      //    searched https we should not return http.
+      try {
+        let prefixURI = NetUtil.newURI(this._strippedPrefix);
+        let finalURI = NetUtil.newURI(match.url);
+        if (prefixURI.scheme != finalURI.scheme)
+          return;
+      } catch (e) {}
+
+      //  * If the user typed "www." but the final url doesn't have it, we
+      //    should not match as well, the two urls may point to different pages.
+      if (this._strippedPrefix.endsWith("www.") &&
+          !stripHttpAndTrim(match.url).startsWith("www."))
+        return;
+
+      let value = this._strippedPrefix + match.token;
+
+      // In any case, we should never arrive here with a value that doesn't
+      // match the search string.  If this happens there is some case we
+      // are not handling properly yet.
+      if (!value.startsWith(this._originalSearchString)) {
+        Components.utils.reportError(`Trying to inline complete in-the-middle
+                                      ${this._originalSearchString} to ${value}`);
+        return;
+      }
+
       this._result.setDefaultIndex(0);
       this._addFrecencyMatch({
-        value: match.token,
+        value: value,
         comment: match.engineName,
         icon: match.iconUrl,
         style: "priority-search",

toolkit/components/places/tests/unifiedcomplete/test_avoid_middle_complete.js

@@ -81,6 +81,72 @@ add_task(function* test_searchEngine_trailing_space_noautofill() {
   yield cleanup();
 });
 
+add_task(function* test_searchEngine_www_noautofill() {
+  Services.search.addEngineWithDetails("HamSearch", "", "", "",
+                                       "GET", "http://ham.search/");
+  let engine = Services.search.getEngineByName("HamSearch");
+  engine.addParam("q", "{searchTerms}", null);
+  do_register_cleanup(() => Services.search.removeEngine(engine));
+
+  do_log_info("Should not autoFill search engine if search string contains www. but engine doesn't");
+  yield check_autocomplete({
+    search: "www.ham",
+    autofilled: "www.ham",
+    completed: "www.ham"
+  });
+
+  yield cleanup();
+});
+
+add_task(function* test_searchEngine_different_scheme_noautofill() {
+  Services.search.addEngineWithDetails("PieSearch", "", "", "",
+                                       "GET", "https://pie.search/");
+  let engine = Services.search.getEngineByName("PieSearch");
+  engine.addParam("q", "{searchTerms}", null);
+  do_register_cleanup(() => Services.search.removeEngine(engine));
+
+  do_log_info("Should not autoFill search engine if search string has a different scheme.");
+  yield check_autocomplete({
+    search: "http://pie",
+    autofilled: "http://pie",
+    completed: "http://pie"
+  });
+
+  yield cleanup();
+});
+
+add_task(function* test_searchEngine_matching_prefix_autofill() {
+  Services.search.addEngineWithDetails("BeanSearch", "", "", "",
+                                       "GET", "http://www.bean.search/");
+  let engine = Services.search.getEngineByName("BeanSearch");
+  engine.addParam("q", "{searchTerms}", null);
+  do_register_cleanup(() => Services.search.removeEngine(engine));
+
+
+  do_log_info("Should autoFill search engine if search string has matching prefix.");
+  yield check_autocomplete({
+    search: "http://www.be",
+    autofilled: "http://www.bean.search",
+    completed: "http://www.bean.search"
+  })
+
+  do_log_info("Should autoFill search engine if search string has www prefix.");
+  yield check_autocomplete({
+    search: "www.be",
+    autofilled: "www.bean.search",
+    completed: "http://www.bean.search"
+  });
+
+  do_log_info("Should autoFill search engine if search string has matching scheme.");
+  yield check_autocomplete({
+    search: "http://be",
+    autofilled: "http://bean.search",
+    completed: "http://www.bean.search"
+  });
+
+  yield cleanup();
+});
+
 add_task(function* test_prefix_autofill() {
   yield promiseAddVisits({ uri: NetUtil.newURI("http://mozilla.org/test/"),
                            transition: TRANSITION_TYPED });