Bug 1322747 - Show https in autofill heuristic results. r?mak
MozReview-Commit-ID: GlOoeQBOMIi
--- a/browser/base/content/test/urlbar/browser_urlbarAutoFillTrimURLs.js
+++ b/browser/base/content/test/urlbar/browser_urlbarAutoFillTrimURLs.js
@@ -10,40 +10,107 @@ add_task(function* setup() {
Services.prefs.clearUserPref(PREF_AUTOFILL);
yield PlacesTestUtils.clearHistory();
gURLBar.handleRevert();
});
Services.prefs.setBoolPref(PREF_TRIMURL, true);
Services.prefs.setBoolPref(PREF_AUTOFILL, true);
// Adding a tab would hit switch-to-tab, so it's safer to just add a visit.
- yield PlacesTestUtils.addVisits({
+ yield PlacesTestUtils.addVisits([{
uri: "http://www.autofilltrimurl.com/whatever",
transition: Ci.nsINavHistoryService.TRANSITION_TYPED,
- });
+ }, {
+ uri: "https://www.secureautofillurl.com/whatever",
+ transition: Ci.nsINavHistoryService.TRANSITION_TYPED,
+ }]);
});
function* promiseSearch(searchtext) {
gURLBar.focus();
gURLBar.inputField.value = searchtext.substr(0, searchtext.length - 1);
EventUtils.synthesizeKey(searchtext.substr(-1, 1), {});
yield promiseSearchComplete();
}
-add_task(function* () {
- yield promiseSearch("http://");
- is(gURLBar.inputField.value, "http://", "Autofilled value is as expected");
+function* promiseTestResult(test) {
+ info("Searching for '${test.search}'");
+
+ yield promiseSearch(test.search);
+
+ is(gURLBar.inputField.value, test.autofilledValue,
+ `Autofilled value is as expected for search '${test.search}'`);
+
+ let result = gURLBar.popup.richlistbox.getItemAtIndex(0);
+
+ is(result._titleText.textContent, test.resultListDisplayTitle,
+ `Autocomplete result should have displayed title as expected for search '${test.search}'`);
+
+ is(result._actionText.textContent, test.resultListActionText,
+ `Autocomplete action text should be as expected for search '${test.search}'`);
+
+ is(result.getAttribute("type"), test.resultListType,
+ `Autocomplete result should have searchengine for the type for search '${test.search}'`);
+
+ is(gURLBar.mController.getFinalCompleteValueAt(0), test.finalCompleteValue,
+ `Autocomplete item should go to the expected final value for search '${test.search}'`);
+}
+
+const tests = [{
+ search: "http://",
+ autofilledValue: "http://",
+ resultListDisplayTitle: "http://",
+ resultListActionText: "Search with Google",
+ resultListType: "searchengine",
+ finalCompleteValue: 'moz-action:searchengine,{"engineName":"Google","input":"http%3A%2F%2F","searchQuery":"http%3A%2F%2F"}'
+ }, {
+ search: "https://",
+ autofilledValue: "https://",
+ resultListDisplayTitle: "https://",
+ resultListActionText: "Search with Google",
+ resultListType: "searchengine",
+ finalCompleteValue: 'moz-action:searchengine,{"engineName":"Google","input":"https%3A%2F%2F","searchQuery":"https%3A%2F%2F"}'
+ }, {
+ search: "au",
+ autofilledValue: "autofilltrimurl.com/",
+ resultListDisplayTitle: "www.autofilltrimurl.com",
+ resultListActionText: "Visit",
+ resultListType: "",
+ finalCompleteValue: "www.autofilltrimurl.com/"
+ }, {
+ search: "http://au",
+ autofilledValue: "http://autofilltrimurl.com/",
+ resultListDisplayTitle: "autofilltrimurl.com",
+ resultListActionText: "Visit",
+ resultListType: "",
+ finalCompleteValue: "http://autofilltrimurl.com/"
+ }, {
+ search: "sec",
+ autofilledValue: "secureautofillurl.com/",
+ resultListDisplayTitle: "https://www.secureautofillurl.com",
+ resultListActionText: "Visit",
+ resultListType: "",
+ finalCompleteValue: "https://www.secureautofillurl.com/"
+ }, {
+ search: "https://sec",
+ autofilledValue: "https://secureautofillurl.com/",
+ resultListDisplayTitle: "https://secureautofillurl.com",
+ resultListActionText: "Visit",
+ resultListType: "",
+ finalCompleteValue: "https://secureautofillurl.com/"
+ },
+];
+
+add_task(function* autofill_tests() {
+ for (let test of tests) {
+ yield promiseTestResult(test);
+ }
});
-add_task(function* () {
- yield promiseSearch("http://au");
- is(gURLBar.inputField.value, "http://autofilltrimurl.com/", "Autofilled value is as expected");
-});
-
-add_task(function* () {
+add_task(function* autofill_complete_domain() {
yield promiseSearch("http://www.autofilltrimurl.com");
is(gURLBar.inputField.value, "http://www.autofilltrimurl.com/", "Autofilled value is as expected");
// Now ensure selecting from the popup correctly trims.
is(gURLBar.controller.matchCount, 2, "Found the expected number of matches");
EventUtils.synthesizeKey("VK_DOWN", {});
is(gURLBar.inputField.value, "www.autofilltrimurl.com/whatever", "trim was applied correctly");
});
--- a/toolkit/components/places/UnifiedComplete.js
+++ b/toolkit/components/places/UnifiedComplete.js
@@ -652,24 +652,31 @@ function stripHttpAndTrim(spec) {
}
if (spec.endsWith("/")) {
spec = spec.slice(0, -1);
}
return spec;
}
/**
- * Returns the key to be used for a URL in a map for the purposes of removing
+ * Returns the key to be used for a match in a map for the purposes of removing
* duplicate entries - any 2 URLs that should be considered the same should
* return the same key. For some moz-action URLs this will unwrap the params
* and return a key based on the wrapped URL.
*/
-function makeKeyForURL(actionUrl) {
+function makeKeyForURL(match) {
+ let actionUrl = match.value;
+
// At this stage we only consider moz-action URLs.
if (!actionUrl.startsWith("moz-action:")) {
+ // For autofill entries, we need to have a key based on the comment rather
+ // than the value field, because the latter may have been trimmed.
+ if (match.hasOwnProperty("style") && match.style.includes("autofill")) {
+ return stripHttpAndTrim(match.comment);
+ }
return stripHttpAndTrim(actionUrl);
}
let [, type, params] = actionUrl.match(/^moz-action:([^,]+),(.*)$/);
try {
params = JSON.parse(params);
} catch (ex) {
// This is unexpected in this context, so just return the input.
return stripHttpAndTrim(actionUrl);
@@ -1615,17 +1622,17 @@ Search.prototype = {
_addMatch(match) {
// A search could be canceled between a query start and its completion,
// in such a case ensure we won't notify any result for it.
if (!this.pending)
return;
// Must check both id and url, cause keywords dynamically modify the url.
- let urlMapKey = makeKeyForURL(match.value);
+ let urlMapKey = makeKeyForURL(match);
if ((match.placeId && this._usedPlaceIds.has(match.placeId)) ||
this._usedURLs.has(urlMapKey)) {
return;
}
// Add this to our internal tracker to ensure duplicates do not end up in
// the result.
// Not all entries have a place id, thus we fallback to the url for them.
@@ -1700,19 +1707,26 @@ Search.prototype = {
// If the untrimmed value doesn't preserve the user's input just
// ignore it and complete to the found host.
if (untrimmedHost &&
!untrimmedHost.toLowerCase().includes(this._trimmedOriginalSearchString.toLowerCase())) {
untrimmedHost = null;
}
match.value = this._strippedPrefix + trimmedHost;
- // Remove the trailing slash.
- match.comment = stripHttpAndTrim(trimmedHost);
match.finalCompleteValue = untrimmedHost;
+
+ // The comment should be the user's final destination so that the user
+ // will definitely know where he is going to end up. For example, if the
+ // user is visiting a secure page, we'll leave the https on it, so that
+ // they know it'll be secure.
+ // We fallback to match.value, as that's what autocomplete does if
+ // finalCompleteValue is null.
+ match.comment = stripHttpAndTrim(match.finalCompleteValue || match.value);
+
if (faviconUrl) {
match.icon = PlacesUtils.favicons
.getFaviconLinkForIcon(NetUtil.newURI(faviconUrl)).spec;
}
// Although this has a frecency, this query is executed before any other
// queries that would result in frecency matches.
match.frecency = frecency;
match.style = "autofill";
--- 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(`Looking for "${result.value}", "${result.comment}" in expected results...`);
+ do_print(`Found value: "${result.value}", comment: "${result.comment}", style: "${result.style}" in 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)) {
--- a/toolkit/components/places/tests/unifiedcomplete/test_autofill_default_behavior.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_autofill_default_behavior.js
@@ -7,28 +7,31 @@
*/
add_task(function* test_default_behavior_host() {
let uri1 = NetUtil.newURI("http://typed/");
let uri2 = NetUtil.newURI("http://visited/");
let uri3 = NetUtil.newURI("http://bookmarked/");
let uri4 = NetUtil.newURI("http://tpbk/");
let uri5 = NetUtil.newURI("http://tagged/");
+ let uri6 = NetUtil.newURI("https://secure/");
yield PlacesTestUtils.addVisits([
{ uri: uri1, title: "typed", transition: TRANSITION_TYPED },
{ uri: uri2, title: "visited" },
{ uri: uri4, title: "tpbk", transition: TRANSITION_TYPED },
+ { uri: uri6, title: "secure", transition: TRANSITION_TYPED },
]);
yield addBookmark( { uri: uri3, title: "bookmarked" } );
yield addBookmark( { uri: uri4, title: "tpbk" } );
yield addBookmark( { uri: uri5, title: "title", tags: ["foo"] } );
yield setFaviconForHref(uri1.spec, "chrome://global/skin/icons/information-16.png");
yield setFaviconForHref(uri3.spec, "chrome://global/skin/icons/error-16.png");
+ yield setFaviconForHref(uri6.spec, "chrome://global/skin/icons/question-16.png");
// RESTRICT TO HISTORY.
Services.prefs.setBoolPref("browser.urlbar.suggest.history", true);
Services.prefs.setBoolPref("browser.urlbar.suggest.history.onlyTyped", false);
Services.prefs.setBoolPref("browser.urlbar.suggest.bookmark", false);
do_print("Restrict history, common visit, should not autoFill");
yield check_autocomplete({
@@ -42,16 +45,25 @@ add_task(function* test_default_behavior
yield check_autocomplete({
search: "ty",
matches: [ { uri: uri1, title: "typed", style: [ "autofill", "heuristic" ],
icon: "chrome://global/skin/icons/information-16.png" } ],
autofilled: "typed/",
completed: "typed/"
});
+ do_print("Restrict history, secure typed visit, should autoFill with https");
+ yield check_autocomplete({
+ search: "secure",
+ matches: [ { uri: uri6, title: "https://secure", style: [ "autofill", "heuristic" ],
+ icon: "chrome://global/skin/icons/question-16.png" } ],
+ autofilled: "secure/",
+ completed: "https://secure/"
+ });
+
// Don't autoFill this one cause it's not typed.
do_print("Restrict history, bookmark, should not autoFill");
yield check_autocomplete({
search: "bo",
matches: [ ],
autofilled: "bo",
completed: "bo"
});
--- a/toolkit/components/places/tests/unifiedcomplete/test_dupe_urls.js
+++ b/toolkit/components/places/tests/unifiedcomplete/test_dupe_urls.js
@@ -14,10 +14,26 @@ add_task(function* test_dupe_urls() {
yield check_autocomplete({
search: "moz",
autofilled: "mozilla.org/",
completed: "mozilla.org/",
matches: [ { uri: NetUtil.newURI("http://mozilla.org/"),
title: "mozilla.org",
style: [ "autofill", "heuristic" ] } ]
});
- yield cleanup();
});
+
+add_task(function* test_dupe_secure_urls() {
+ yield PlacesTestUtils.addVisits({
+ uri: NetUtil.newURI("https://example.org/"),
+ transition: TRANSITION_TYPED
+ }, {
+ uri: NetUtil.newURI("https://example.org/?")
+ });
+ yield check_autocomplete({
+ search: "exam",
+ autofilled: "example.org/",
+ completed: "https://example.org/",
+ matches: [ { uri: NetUtil.newURI("https://example.org/"),
+ title: "https://example.org",
+ style: [ "autofill", "heuristic" ] } ]
+ });
+});
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -2061,17 +2061,17 @@ extends="chrome://global/content/binding
let action;
if (initialTypes.has("autofill")) {
// Treat autofills as visiturl actions.
action = {
type: "visiturl",
params: {
- url: originalUrl,
+ url: this.getAttribute("title"),
},
};
}
this.removeAttribute("actiontype");
this.classList.remove("overridable-action");
// If the type includes an action, set up the item appropriately.