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
--- 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");