diff --git a/toolkit/components/places/UnifiedComplete.js b/toolkit/components/places/UnifiedComplete.js index d93e16be8408..d8eb2cde9970 100644 --- a/toolkit/components/places/UnifiedComplete.js +++ b/toolkit/components/places/UnifiedComplete.js @@ -585,18 +585,6 @@ XPCOMUtils.defineLazyGetter(this, "ProfileAgeCreatedPromise", () => { // Helper functions -/** - * Used to unescape encoded URI strings and drop information that we do not - * care about. - * - * @param spec - * The text to unescape and modify. - * @return the modified spec. - */ -function fixupSearchText(spec) { - return textURIService.unEscapeURIForUI("UTF-8", stripPrefix(spec)); -} - /** * Generates the tokens used in searching from a given string. * @@ -640,16 +628,18 @@ function stripPrefix(spec) { * * @param spec * The text to modify. + * @param trimSlash + * Whether to trim the trailing slash. * @return the modified spec. */ -function stripHttpAndTrim(spec) { +function stripHttpAndTrim(spec, trimSlash = true) { if (spec.startsWith("http://")) { spec = spec.slice(7); } if (spec.endsWith("?")) { spec = spec.slice(0, -1); } - if (spec.endsWith("/")) { + if (trimSlash && spec.endsWith("/")) { spec = spec.slice(0, -1); } return spec; @@ -742,7 +732,16 @@ function Search(searchString, searchParam, autocompleteListener, // We want to store the original string for case sensitive searches. this._originalSearchString = searchString; this._trimmedOriginalSearchString = searchString.trim(); - this._searchString = fixupSearchText(this._trimmedOriginalSearchString.toLowerCase()); + let strippedOriginalSearchString = + stripPrefix(this._trimmedOriginalSearchString.toLowerCase()); + this._searchString = + textURIService.unEscapeURIForUI("UTF-8", strippedOriginalSearchString); + + // The protocol and the host are lowercased by nsIURI, so it's fine to + // lowercase the typed prefix, to add it back to the results later. + this._strippedPrefix = this._trimmedOriginalSearchString.slice( + 0, this._trimmedOriginalSearchString.length - strippedOriginalSearchString.length + ).toLowerCase(); this._matchBehavior = Prefs.matchBehavior; // Set the default behavior for this search. @@ -762,19 +761,6 @@ function Search(searchString, searchParam, autocompleteListener, this._searchTokens = this.filterTokens(getUnfilteredSearchTokens(this._searchString)); - // The protocol and the host are lowercased by nsIURI, so it's fine to - // lowercase the typed prefix, to add it back to the results later. - this._strippedPrefix = this._trimmedOriginalSearchString.slice( - 0, this._trimmedOriginalSearchString.length - this._searchString.length - ).toLowerCase(); - // The URIs in the database are fixed-up, so we can match on a lowercased - // host, but the path must be matched in a case sensitive way. - let pathIndex = - this._trimmedOriginalSearchString.indexOf("/", this._strippedPrefix.length); - this._autofillUrlSearchString = fixupSearchText( - this._trimmedOriginalSearchString.slice(0, pathIndex).toLowerCase() + - this._trimmedOriginalSearchString.slice(pathIndex) - ); this._prohibitSearchSuggestions = prohibitSearchSuggestions; @@ -1089,7 +1075,6 @@ Search.prototype = { if (site.uri.host.startsWith(this._searchString)) { let match = { value: stripPrefix(site.uri.spec), - comment: site.title, style: "autofill", finalCompleteValue: site.uri.spec, frecency: FRECENCY_DEFAULT, @@ -1625,6 +1610,20 @@ Search.prototype = { if (!this.pending) return; + // For autofill entries, the comment field must be a stripped version + // of the final destination url, 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, to let him know that. + // This must happen before generating the dedupe key. + if (match.hasOwnProperty("style") && match.style.includes("autofill")) { + // We fallback to match.value, as that's what autocomplete does if + // finalCompleteValue is null. + // Trim only if the value looks like a domain, we want to retain the + // trailing slash if we're completing a url to the next slash. + match.comment = stripHttpAndTrim(match.finalCompleteValue || match.value, + !this._searchString.includes("/")); + } + // Must check both id and url, cause keywords dynamically modify the url. let urlMapKey = makeKeyForURL(match); if ((match.placeId && this._usedPlaceIds.has(match.placeId)) || @@ -1698,28 +1697,19 @@ Search.prototype = { _processHostRow(row) { let match = {}; - let trimmedHost = row.getResultByIndex(QUERYINDEX_URL); - let untrimmedHost = row.getResultByIndex(QUERYINDEX_TITLE); + let strippedHost = row.getResultByIndex(QUERYINDEX_URL); + let unstrippedHost = row.getResultByIndex(QUERYINDEX_TITLE); let frecency = row.getResultByIndex(QUERYINDEX_FRECENCY); let faviconUrl = row.getResultByIndex(QUERYINDEX_ICONURL); - // If the untrimmed value doesn't preserve the user's input just + // If the unfixup 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; + if (!unstrippedHost.toLowerCase().includes(this._trimmedOriginalSearchString.toLowerCase())) { + unstrippedHost = null; } - match.value = this._strippedPrefix + 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); + match.value = this._strippedPrefix + strippedHost; + match.finalCompleteValue = unstrippedHost; if (faviconUrl) { match.icon = PlacesUtils.favicons @@ -1733,44 +1723,43 @@ Search.prototype = { }, _processUrlRow(row) { - let match = {}; - let value = row.getResultByIndex(QUERYINDEX_URL); - let url = fixupSearchText(value); + let url = row.getResultByIndex(QUERYINDEX_URL); + let strippedUrl = stripPrefix(url); + let prefix = url.substr(0, url.length - strippedUrl.length); let frecency = row.getResultByIndex(QUERYINDEX_FRECENCY); let faviconUrl = row.getResultByIndex(QUERYINDEX_ICONURL); - let prefix = value.slice(0, value.length - stripPrefix(value).length); - // We must complete the URL up to the next separator (which is /, ? or #). - let separatorIndex = url.slice(this._searchString.length) - .search(/[\/\?\#]/); + let searchString = stripPrefix(this._trimmedOriginalSearchString); + let separatorIndex = strippedUrl.slice(searchString.length) + .search(/[\/\?\#]/); if (separatorIndex != -1) { - separatorIndex += this._searchString.length; - if (url[separatorIndex] == "/") { + separatorIndex += searchString.length; + if (strippedUrl[separatorIndex] == "/") { separatorIndex++; // Include the "/" separator } - url = url.slice(0, separatorIndex); + strippedUrl = strippedUrl.slice(0, separatorIndex); } - // If the untrimmed value doesn't preserve the user's input just - // ignore it and complete to the found url. - let untrimmedURL = prefix + url; - if (untrimmedURL && - !untrimmedURL.toLowerCase().includes(this._trimmedOriginalSearchString.toLowerCase())) { - untrimmedURL = null; - } + let match = { + value: this._strippedPrefix + strippedUrl, + // Although this has a frecency, this query is executed before any other + // queries that would result in frecency matches. + frecency, + style: "autofill" + }; - match.value = this._strippedPrefix + url; - match.comment = url; - match.finalCompleteValue = untrimmedURL; 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"; + + // Complete to the found url only if its untrimmed value preserves the + // user's input. + if (url.toLowerCase().includes(this._trimmedOriginalSearchString.toLowerCase())) { + match.finalCompleteValue = prefix + strippedUrl; + } + return match; }, @@ -2019,9 +2008,16 @@ Search.prototype = { // We expect this to be a full URL, not just a host. We want to extract the // host and use that as a guess for whether we'll get a result from a URL // query. - let slashIndex = this._autofillUrlSearchString.indexOf("/"); - let revHost = this._autofillUrlSearchString.substring(0, slashIndex).toLowerCase() - .split("").reverse().join("") + "."; + // The URIs in the database are fixed-up, so we can match on a lowercased + // host, but the path must be matched in a case sensitive way. + let pathIndex = this._trimmedOriginalSearchString.indexOf("/", this._strippedPrefix.length); + let revHost = this._trimmedOriginalSearchString + .substring(this._strippedPrefix.length, pathIndex) + .toLowerCase().split("").reverse().join("") + "."; + let searchString = stripPrefix( + this._trimmedOriginalSearchString.slice(0, pathIndex).toLowerCase() + + this._trimmedOriginalSearchString.slice(pathIndex) + ); let typed = Prefs.autofillTyped || this.hasBehavior("typed"); let bookmarked = this.hasBehavior("bookmark") && !this.hasBehavior("history"); @@ -2037,7 +2033,7 @@ Search.prototype = { query.push({ query_type: QUERYTYPE_AUTOFILL_URL, - searchString: this._autofillUrlSearchString, + searchString, revHost }); diff --git a/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js b/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js index 488c931cf6be..6af3d067ba6c 100644 --- a/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js +++ b/toolkit/components/places/tests/unifiedcomplete/head_autocomplete.js @@ -374,7 +374,7 @@ function makeSearchMatch(input, extra = {}) { params.alias = extra.alias; } let style = [ "action", "searchengine" ]; - if (Array.isArray(extra.style)) { + if ("style" in extra && Array.isArray(extra.style)) { style.push(...extra.style); } if (extra.heuristic) { diff --git a/toolkit/components/places/tests/unifiedcomplete/test_encoded_urls.js b/toolkit/components/places/tests/unifiedcomplete/test_encoded_urls.js new file mode 100644 index 000000000000..45e79c18e66c --- /dev/null +++ b/toolkit/components/places/tests/unifiedcomplete/test_encoded_urls.js @@ -0,0 +1,71 @@ +add_task(function* test_encoded() { + do_print("Searching for over encoded url should not break it"); + yield PlacesTestUtils.addVisits({ + uri: NetUtil.newURI("https://www.mozilla.com/search/top/?q=%25%32%35"), + title: "https://www.mozilla.com/search/top/?q=%25%32%35", + transition: TRANSITION_TYPED + }); + yield check_autocomplete({ + search: "https://www.mozilla.com/search/top/?q=%25%32%35", + matches: [ { uri: NetUtil.newURI("https://www.mozilla.com/search/top/?q=%25%32%35"), + title: "https://www.mozilla.com/search/top/?q=%25%32%35", + style: [ "autofill", "heuristic" ] }], + autofilled: "https://www.mozilla.com/search/top/?q=%25%32%35", + completed: "https://www.mozilla.com/search/top/?q=%25%32%35" + }); + yield cleanup(); +}); + +add_task(function* test_encoded_trimmed() { + do_print("Searching for over encoded url should not break it"); + yield PlacesTestUtils.addVisits({ + uri: NetUtil.newURI("https://www.mozilla.com/search/top/?q=%25%32%35"), + title: "https://www.mozilla.com/search/top/?q=%25%32%35", + transition: TRANSITION_TYPED + }); + yield check_autocomplete({ + search: "mozilla.com/search/top/?q=%25%32%35", + matches: [ { uri: NetUtil.newURI("https://www.mozilla.com/search/top/?q=%25%32%35"), + title: "https://www.mozilla.com/search/top/?q=%25%32%35", + style: [ "autofill", "heuristic" ] }], + autofilled: "mozilla.com/search/top/?q=%25%32%35", + completed: "https://www.mozilla.com/search/top/?q=%25%32%35" + }); + yield cleanup(); +}); + +add_task(function* test_encoded_partial() { + do_print("Searching for over encoded url should not break it"); + yield PlacesTestUtils.addVisits({ + uri: NetUtil.newURI("https://www.mozilla.com/search/top/?q=%25%32%35"), + title: "https://www.mozilla.com/search/top/?q=%25%32%35", + transition: TRANSITION_TYPED + }); + yield check_autocomplete({ + search: "https://www.mozilla.com/search/top/?q=%25", + matches: [ { uri: NetUtil.newURI("https://www.mozilla.com/search/top/?q=%25%32%35"), + title: "https://www.mozilla.com/search/top/?q=%25%32%35", + style: [ "autofill", "heuristic" ] }], + autofilled: "https://www.mozilla.com/search/top/?q=%25%32%35", + completed: "https://www.mozilla.com/search/top/?q=%25%32%35" + }); + yield cleanup(); +}); + +add_task(function* test_encoded_path() { + do_print("Searching for over encoded url should not break it"); + yield PlacesTestUtils.addVisits({ + uri: NetUtil.newURI("https://www.mozilla.com/%25%32%35/top/"), + title: "https://www.mozilla.com/%25%32%35/top/", + transition: TRANSITION_TYPED + }); + yield check_autocomplete({ + search: "https://www.mozilla.com/%25%32%35/t", + matches: [ { uri: NetUtil.newURI("https://www.mozilla.com/%25%32%35/top/"), + title: "https://www.mozilla.com/%25%32%35/top/", + style: [ "autofill", "heuristic" ] }], + autofilled: "https://www.mozilla.com/%25%32%35/top/", + completed: "https://www.mozilla.com/%25%32%35/top/" + }); + yield cleanup(); +}); diff --git a/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini b/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini index 6e96e44f78f0..6f4a225df0d2 100644 --- a/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini +++ b/toolkit/components/places/tests/unifiedcomplete/xpcshell.ini @@ -22,6 +22,7 @@ support-files = [test_dupe_urls.js] [test_empty_search.js] [test_enabled.js] +[test_encoded_urls.js] [test_escape_self.js] [test_extension_matches.js] [test_ignore_protocol.js]