Bug 1340108 - autocomplete list entries match regardless of www. in them, r?gijs draft
authorSvetlana Orlik <sveta.orlik.code@gmail.com>
Wed, 01 Mar 2017 03:20:34 +0300
changeset 490800 f585885162e4ee872aa28964fbfe5ef50f5159d1
parent 490799 005e450d6c4e2e182ec8a05abd7ef722384c0554
child 490906 9ff32e8c799527cfa7c122c03f3517d7223223b6
push id47227
push userbmo:sveta.orlik.code@gmail.com
push dateWed, 01 Mar 2017 09:20:51 +0000
reviewersgijs
bugs1340108
milestone54.0a1
Bug 1340108 - autocomplete list entries match regardless of www. in them, r?gijs Still WIP TODO: add more tests on list entries TODO: edge case bug in autofill when user types "ww" MozReview-Commit-ID: GVsbnyfHivr
toolkit/components/places/UnifiedComplete.js
toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
toolkit/components/places/tests/unifiedcomplete/test_prefill_sites.js
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -554,25 +554,16 @@ XPCOMUtils.defineLazyGetter(this, "Prefs
 
 function PrefillSite(url, title) {
   this.uri = NetUtil.newURI(url);
   this.title = title;
   this._matchTitle = title.toLowerCase();
   this._hasWWW = this.uri.host.startsWith("www.");
   this._hostWithoutWWW = this._hasWWW ? this.uri.host.slice(4)
                                       : this.uri.host;
-/**
-    dump(`++++++++++++++++++++++++++++++++++
-          url:${url}
-          this.uri.spec:${this.uri.spec}
-          this.uri.host:${this.uri.host}
-          this.uri.scheme:${this.uri.scheme}
-          `
-        );
-**/
 }
 
 /**
  * Storage object for Prefill Sites.
  *   add(url, title): adds a site to storage
  *   populate(sites) : populates the  storage with array of [url,title]
  *   sites[]: resulting array of sites (PrefillSite objects)
  */
@@ -803,16 +794,23 @@ function Search(searchString, searchPara
   this._localMatchesStartIndex = 0;
 
   // Counts the number of inserted local matches.
   this._localMatchesCount = 0;
   // Counts the number of inserted remote matches.
   this._remoteMatchesCount = 0;
   // Counts the number of inserted extension matches.
   this._extensionMatchesCount = 0;
+
+  this._searchStringHasWWW = this._strippedPrefix.endsWith("www.");
+  this._searchStringWWW = this._searchStringHasWWW ? "www." : "";
+
+  this._searchStringSchemeFound = this._strippedPrefix.match(/^(\w+):/i);
+  this._searchStringScheme = this._searchStringSchemeFound ?
+                             this._searchStringSchemeFound[1].toLowerCase() : "";
 }
 
 Search.prototype = {
   /**
    * Enables the desired AutoComplete behavior.
    *
    * @param type
    *        The behavior type to set.
@@ -1066,107 +1064,98 @@ Search.prototype = {
     if (!Prefs.prefillSitesEnabled)
       return;
 
     // In case user typed just "https://" or "www." or "https://www."
     // - we do not put out the whole lot of sites
     if (!this._searchString)
       return;
 
+    if (!(this._searchStringScheme === "" ||
+          this._searchStringScheme === "https" ||
+          this._searchStringScheme === "http" ))
+      return;
+
+    let strictMatches = [];
+    let looseMatches = [];
+
     for (let site of PrefillSiteStorage.sites) {
-      if (site.uri.host.includes(this._searchString) ||
-          site._matchTitle.includes(this._searchString)) {
-        let match = {
-          value: site.uri.spec,
-          comment: site.title,
-          style: "prefill-site",
-          finalCompleteValue: site.uri.spec,
-          frecency: FRECENCY_DEFAULT - 1,
-        };
-        this._addMatch(match);
-      }
+      let match = {
+        value: site.uri.spec,
+        comment: site.title,
+        style: "prefill-site",
+        finalCompleteValue: site.uri.spec,
+        frecency: FRECENCY_DEFAULT - 1,
+      };
+      if (site.uri.spec.includes(this._strippedPrefix + this._searchString) ||
+          site._matchTitle.includes(this._strippedPrefix + this._searchString))
+        strictMatches.push(match);
+      else
+        if (
+             (
+               site.uri.host.includes(this._searchString) ||
+               site._matchTitle.includes(this._searchString)
+             ) &&
+             (
+               !this._searchStringScheme ||
+               this._searchStringScheme === site.uri.scheme
+             )
+            )
+          looseMatches.push(match);
     }
+    [...strictMatches, ...looseMatches].forEach(this._addMatch, this);
   },
 
   _matchPrefillSiteForAutofill() {
     if (!Prefs.prefillSitesEnabled)
       return false;
 
+/*
     let searchStringHasWWW = this._strippedPrefix.endsWith("www.");
     let searchStringWWW = searchStringHasWWW ? "www." : "";
 
     // Extract any stripped scheme (as user can type even "ftp://")
-    let searchStringSchemeFound = this._strippedPrefix.match(/^(\w+):\/\//i);
+    let searchStringSchemeFound = this._strippedPrefix.match(/^(\w+):/i);
     let searchStringScheme = searchStringSchemeFound ?
                              searchStringSchemeFound[1].toLowerCase() : "";
-    if (!(searchStringScheme === "" ||
-          searchStringScheme === "https" ||
-          searchStringScheme === "http" ))
+*/
+    if (!(this._searchStringScheme === "" ||
+          this._searchStringScheme === "https" ||
+          this._searchStringScheme === "http" ))
       return false;
 
     function matchStrict(site) {
-      return site.uri.host.startsWith(searchStringWWW + this._searchString) &&
-             (!searchStringScheme || searchStringScheme === site.uri.scheme)
+      return site.uri.host.startsWith(this._searchStringWWW + this._searchString) &&
+             (!this._searchStringScheme || this._searchStringScheme === site.uri.scheme)
     }
 
     function matchLoose(site) {
       return site._hostWithoutWWW.startsWith(this._searchString) &&
-             (!searchStringScheme || searchStringScheme === site.uri.scheme)
+             (!this._searchStringScheme || this._searchStringScheme === site.uri.scheme)
     }
 
     // Strict match has priority
     let site = PrefillSiteStorage.sites.find(matchStrict, this) ||
                PrefillSiteStorage.sites.find(matchLoose, this);
 
     if (!site)
       return false;
 
-    let finalWWW = (site._hasWWW || searchStringHasWWW) ? "www." : "";
+    let finalWWW = (site._hasWWW || this._searchStringHasWWW) ? "www." : "";
     let match = {
       // we show prefix just as user typed it
       value: this._strippedPrefix + stripPrefix(site.uri.spec),
       style: "autofill",
       finalCompleteValue: site.uri.scheme + "://" + finalWWW
                           + stripPrefix(site.uri.spec),
       frecency: FRECENCY_DEFAULT,
     };
     this._result.setDefaultIndex(0);
     this._addMatch(match);
     return true;
-
-/*
-    for (let site of PrefillSiteStorage.sites) {
-      let schemeMatches = !searchStringScheme ||
-                            searchStringScheme == site.uri.scheme;
-
-      //let protocolMatches = (searchStringHasHTTP && (site.uri.scheme === "http")) ||
-      //                      (searchStringHasHTTPS && (site.uri.scheme === "https"));
-
-      if (site._hostWithoutWWW.startsWith(this._searchString) &&
-          schemeMatches
- //          (!searchStringHasProtocol || (searchStringHasProtocol && protocolMatches))
-         ) {
-        // finalCompleteValue is where we go after Enter is pressed
-        // gets "www." if site has it OR user typed it
-        let finalWWW = (site._hasWWW || searchStringHasWWW) ? "www." : "";
-        let match = {
-          // we show prefix just as user typed it
-          value: this._strippedPrefix + stripPrefix(site.uri.spec),
-          style: "autofill",
-          finalCompleteValue: site.uri.scheme + "://" + finalWWW
-                              + stripPrefix(site.uri.spec),
-          frecency: FRECENCY_DEFAULT,
-        };
-        this._result.setDefaultIndex(0);
-        this._addMatch(match);
-        return true;
-      }
-    }
-    return false;
-*/
   },
 
   *_matchFirstHeuristicResult(conn) {
     // We always try to make the first result a special "heuristic" result.  The
     // heuristics below determine what type of result it will be, if any.
 
     let hasSearchTerms = this._searchTokens.length > 0;
 
@@ -1727,24 +1716,16 @@ Search.prototype = {
 
     if (this._addingHeuristicFirstMatch) {
       match.style += " heuristic";
     }
 
     match.icon = match.icon || "";
     match.finalCompleteValue = match.finalCompleteValue || "";
 
-/**
-    dump(`----------------------------------
-          match.value:${match.value}
-          match.comment:${match.comment}
-          match.icon:${match.icon}
-          match.style:${match.style}
-          match.finalCompleteValue:${match.finalCompleteValue}\n`);
-**/
     this._result.insertMatchAt(this._getInsertIndexForMatch(match),
                                match.value,
                                match.comment,
                                match.icon,
                                match.style,
                                match.finalCompleteValue);
 
     if (this._result.matchCount == 6)
--- a/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
+++ b/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js
@@ -213,17 +213,17 @@ function* check_autocomplete(test) {
 
       for (let i = firstIndexToCheck; i < controller.matchCount; i++) {
         let result = {
           value: controller.getValueAt(i),
           comment: controller.getCommentAt(i),
           style: controller.getStyleAt(i),
           image: controller.getImageAt(i),
         }
-        do_print(`Found value: "${result.value}", comment: "${result.comment}", style: "${result.style}" in results...`);
+        do_print(`Looking for "${result.value}", "${result.comment}" in expected results...`);
         let lowerBound = test.checkSorting ? i : firstIndexToCheck;
         let upperBound = test.checkSorting ? i + 1 : matches.length;
         let found = false;
         for (let j = lowerBound; j < upperBound; ++j) {
           // Skip processed expected results
           if (matches[j] == undefined)
             continue;
           if (_check_autocomplete_matches(matches[j], result)) {
@@ -369,17 +369,17 @@ function makeSearchMatch(input, extra = 
     searchQuery: "searchQuery" in extra ? extra.searchQuery : input,
   };
   if ("alias" in extra) {
     // May be undefined, which is expected, but in that case make sure it's not
     // included in the params of the moz-action URL.
     params.alias = extra.alias;
   }
   let style = [ "action", "searchengine" ];
-  if ("style" in extra && Array.isArray(extra.style)) {
+  if (Array.isArray(extra.style)) {
     style.push(...extra.style);
   }
   if (extra.heuristic) {
     style.push("heuristic");
   }
   return {
     uri: makeActionURI("searchengine", params),
     title: params.engineName,
--- a/toolkit/components/places/tests/unifiedcomplete/test_prefill_sites.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_prefill_sites.js
@@ -112,46 +112,88 @@ add_task(function* test_sorting_against_
       { uri: gooogleURI, title: "Gooogle", style: ["prefill-site"] },
     ],
   });
 
   yield cleanup();
 });
 
 add_task(function* test_scheme_and_www() {
-  let https_www_site_URI = NetUtil.newURI("https://www.ooops-https-www.com/");
-  let https_____site_URI = NetUtil.newURI("https://ooops-https.com/");
-  // let http__www_site_URI = NetUtil.newURI("HTTP://www.ooops-HTTP-www.com/");
-  let http______site_URI = NetUtil.newURI("HTTP://ooops-HTTP.com/");
+  let ooops_https_www_URI = NetUtil.newURI("https://www.ooops-https-www.com/");
+  let ooops_https_____URI = NetUtil.newURI("https://ooops-https.com/");
+  let ooops_http______URI = NetUtil.newURI("HTTP://ooops-HTTP.com/");
+  let ooops_http__www_URI = NetUtil.newURI("HTTP://www.ooops-HTTP-www.com/");
 
-  autocompleteObject.addPrefillSite(https_www_site_URI.spec, "Ooops");
-  autocompleteObject.addPrefillSite(https_____site_URI.spec, "Ooops");
-  // autocompleteObject.addPrefillSite(http__www_site_URI.spec, "Ooops");
-  autocompleteObject.addPrefillSite(http______site_URI.spec, "Ooops");
+  // Order is important to check sortig
+  autocompleteObject.addPrefillSite(ooops_https_www_URI.spec, "Ooops");
+  autocompleteObject.addPrefillSite(ooops_https_____URI.spec, "Ooops");
+  autocompleteObject.addPrefillSite(ooops_http______URI.spec, "Ooops");
+  autocompleteObject.addPrefillSite(ooops_http__www_URI.spec, "Ooops");
 
   let tests =
-  // User typed
-  // autofilled
-  // completed
   [
+    // User typed,
+    // Inline autofill,
+    // Substitute after enter is pressed,
+    //   [List matches, with sorting]
+    //   not tested if omitted
+    //   first one is always an autofill entry
+
+    [// Protocol by itself doesn't match anything
+    "https://",
+    "https://",
+    "https://",
+      []
+    ],
+
+    [// "www." by itself doesn't match anything
+    "www.",
+    "www.",
+    "www.",
+      []
+    ],
+
+    [// Protocol with "www." by itself doesn't match anything
+    "http://www.",
+    "http://www.",
+    "http://www.",
+      []
+    ],
+
     [// ftp: - ignore
     "ftp://ooops",
     "ftp://ooops",
     "ftp://ooops",
+      []
     ],
+
     [// Strict match, no "www."
     "ooops",
     "ooops-https.com/",
     "https://ooops-https.com/", // 2nd in list, but has priority as strict
+      [
+      "https://ooops-https.com/",
+      "https://www.ooops-https-www.com/",
+      "HTTP://ooops-HTTP.com/",
+      "HTTP://www.ooops-HTTP-www.com/",
+      ]
     ],
+
     [// Strict match with "www."
     "www.ooops",
     "www.ooops-https-www.com/",
     "https://www.ooops-https-www.com/",
+      [// Matches with "www." sorted on top
+      "https://www.ooops-https-www.com/",
+      "HTTP://www.ooops-HTTP-www.com/",
+      "https://ooops-https.com/",
+      "HTTP://ooops-HTTP.com/",
+      ]
     ],
+
     [// Loose match: search no "www.", result with "www."
     "ooops-https-www",
     "ooops-https-www.com/",
     "https://www.ooops-https-www.com/",
     ],
     [// Loose match: search "www.", no-www site gets "www."
     "www.ooops-https.",
     "www.ooops-https.com/",
@@ -174,22 +216,40 @@ add_task(function* test_scheme_and_www()
     ],
     [// Wrong protocol
     "http://ooops-https",
     "http://ooops-https",
     "http://ooops-https",
     ],
   ];
 
+  function toMatch(url, index) {
+    let uri = NetUtil.newURI(url);
+    if (index === 0) // first entry is always an autofill entry
+      return {
+        uri,
+        title: url.toLowerCase().slice(0, -1), // slice trailing slash
+        style: ["autofill", "heuristic"],
+      };
+    return {
+      uri,
+      title: "Ooops",
+      style: ["prefill-site"],
+    };
+  }
+
   for (let test of tests) {
+    let matches = test[3] ? test[3].map(toMatch) : null;
     do_print("User types: " + test[0]);
     yield check_autocomplete({
+      checkSorting: true,
       search: test[0],
       autofilled: test[1].toLowerCase(),
       completed: test[2].toLowerCase(),
+      matches,
     });
   }
 
   yield cleanup();
 });
 
 add_task(function* test_data_file() {
   let response = yield fetch("chrome://global/content/unifiedcomplete-top-urls.json");