From 7e76f843c7f045f3eff31741ae8958976630d5ae Mon Sep 17 00:00:00 2001 From: Marco Bonardo Date: Wed, 15 Jul 2020 09:53:04 +0000 Subject: [PATCH] Bug 1641467 - Remove temporary browser.fixup.defaultToSearch feature pref and its code. r=Gijs Differential Revision: https://phabricator.services.mozilla.com/D83554 --- .../browser/browser_UrlbarInput_trimURLs.js | 10 +- .../unit/test_providerHeuristicFallback.js | 17 --- .../tests/unit/test_search_suggestions.js | 15 -- docshell/base/URIFixup.jsm | 136 +----------------- docshell/test/unit/test_URIFixup_info.js | 34 ++--- 5 files changed, 19 insertions(+), 193 deletions(-) diff --git a/browser/components/urlbar/tests/browser/browser_UrlbarInput_trimURLs.js b/browser/components/urlbar/tests/browser/browser_UrlbarInput_trimURLs.js index 153c7e3aad30..3c1f3ea8e4d9 100644 --- a/browser/components/urlbar/tests/browser/browser_UrlbarInput_trimURLs.js +++ b/browser/components/urlbar/tests/browser/browser_UrlbarInput_trimURLs.js @@ -67,12 +67,10 @@ add_task(async function() { testVal("http:// invalid url"); testVal("http://someotherhostwithnodots"); - if (Services.prefs.getBoolPref("browser.fixup.defaultToSearch", true)) { - // This host is whitelisted, it can be trimmed. - testVal("http://localhost/ foo bar baz", "localhost/ foo bar baz"); - } else { - testVal("http://localhost/ foo bar baz"); - } + + // This host is whitelisted, it can be trimmed. + testVal("http://localhost/ foo bar baz", "localhost/ foo bar baz"); + // This is not trimmed because it's not in the domain whitelist. testVal( "http://localhost.localdomain/ foo bar baz", diff --git a/browser/components/urlbar/tests/unit/test_providerHeuristicFallback.js b/browser/components/urlbar/tests/unit/test_providerHeuristicFallback.js index 9dd9c02a501b..65bf8c3b1e96 100644 --- a/browser/components/urlbar/tests/unit/test_providerHeuristicFallback.js +++ b/browser/components/urlbar/tests/unit/test_providerHeuristicFallback.js @@ -199,7 +199,6 @@ add_task(async function() { }); info("string with known host"); - Services.prefs.setBoolPref("browser.fixup.defaultToSearch", true); query = "firefox/get"; context = createContext(query, { isPrivate: false }); await check_results({ @@ -212,22 +211,6 @@ add_task(async function() { ], }); - Services.prefs.setBoolPref("browser.fixup.defaultToSearch", false); - query = "firefox/get"; - context = createContext(query, { isPrivate: false }); - await check_results({ - context, - matches: [ - makeVisitResult(context, { - uri: `http://${query}`, - title: `http://${query}`, - iconUri: "page-icon:http://firefox/", - heuristic: true, - }), - ], - }); - Services.prefs.clearUserPref("browser.fixup.defaultToSearch"); - Services.prefs.setBoolPref("browser.fixup.domainwhitelist.firefox", true); registerCleanupFunction(() => { Services.prefs.clearUserPref("browser.fixup.domainwhitelist.firefox"); diff --git a/browser/components/urlbar/tests/unit/test_search_suggestions.js b/browser/components/urlbar/tests/unit/test_search_suggestions.js index a0446eeb79d2..c5e79faf7de0 100644 --- a/browser/components/urlbar/tests/unit/test_search_suggestions.js +++ b/browser/components/urlbar/tests/unit/test_search_suggestions.js @@ -867,21 +867,6 @@ add_task(async function prohibit_suggestions() { ], }); - if (!Services.prefs.getBoolPref("browser.fixup.defaultToSearch", true)) { - context = createContext("test/test", { isPrivate: false }); - await check_results({ - context, - matches: [ - makeVisitResult(context, { - uri: "http://test/test", - title: "http://test/test", - iconUri: "page-icon:http://test/", - heuristic: true, - }), - ], - }); - } - context = createContext("data:text/plain,Content", { isPrivate: false }); await check_results({ context, diff --git a/docshell/base/URIFixup.jsm b/docshell/base/URIFixup.jsm index aee46bfd73b9..9ef6c5ec6659 100644 --- a/docshell/base/URIFixup.jsm +++ b/docshell/base/URIFixup.jsm @@ -67,16 +67,6 @@ XPCOMUtils.defineLazyPreferenceGetter( "browser.fixup.alternate.enabled", true ); -// This is a feature preference that inverts the keyword fixup behavior to -// search by default, unless the string has URI characteristics. -// When set to false, we'll consider most strings URIs, unless they have search -// characteristics. -XPCOMUtils.defineLazyPreferenceGetter( - this, - "defaultToSearch", - "browser.fixup.defaultToSearch", - true -); const { FIXUP_FLAG_NONE, @@ -97,9 +87,6 @@ XPCOMUtils.defineLazyGetter( () => /^([a-z+.-]+:\/{0,3})*[^\/@]+@.+/i ); -// Regex used to look for ascii alphabetical characters. -XPCOMUtils.defineLazyGetter(this, "asciiAlphaRegex", () => /[a-z]/i); - // Regex used to identify specific URI characteristics to disallow searching. XPCOMUtils.defineLazyGetter( this, @@ -431,9 +418,7 @@ URIFixup.prototype = { keywordEnabled && fixupFlags & FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP && !inputHadDuffProtocol && - // When defaultToSearch is false, we are conservative about always going - // through keywordURIFixup. - (!defaultToSearch || !checkSuffix(info).suffix) && + !checkSuffix(info).suffix && keywordURIFixup(uriString, info, isPrivateContext, postData) ) { return info; @@ -907,14 +892,6 @@ function fixupURIProtocol(uriString) { * @returns {boolean} Whether the keyword fixup was succesful. */ function keywordURIFixup(uriString, fixupInfo, isPrivateContext, postData) { - if (!defaultToSearch) { - return keywordURIFixupLegacy( - uriString, - fixupInfo, - isPrivateContext, - postData - ); - } // Here is a few examples of strings that should be searched: // "what is mozilla" // "what is mozilla?" @@ -982,117 +959,6 @@ function keywordURIFixup(uriString, fixupInfo, isPrivateContext, postData) { return false; } -/** - * This is the old version of keywordURIFixup, used when - * browser.fixup.defaultToSearch is false - */ -function keywordURIFixupLegacy( - uriString, - fixupInfo, - isPrivateContext, - postData -) { - // These are keyword formatted strings - // "what is mozilla" - // "what is mozilla?" - // "docshell site:mozilla.org" - has no dot/colon in the first space-separated - // substring - // "?mozilla" - anything that begins with a question mark - // "?site:mozilla.org docshell" - // Things that have a quote before the first dot/colon - // "mozilla" - checked against the knownDomains to see if it's a host or not - // ".mozilla", "mozilla." - ditto - - // These are not keyword formatted strings - // "www.blah.com" - first space-separated substring contains a dot, doesn't - // start with "?" "www.blah.com stuff" "nonQualifiedHost:80" - first - // space-separated substring contains a colon, doesn't start with "?" - // "nonQualifiedHost:80 args" - // "nonQualifiedHost?" - // "nonQualifiedHost?args" - // "nonQualifiedHost?some args" - // "blah.com." - - // Check for IPs. - if (IPv4LikeRegex.test(uriString) || IPv6LikeRegex.test(uriString)) { - return false; - } - - // We do keyword lookups if the input starts with a question mark, or if it - // contains a space or quote, provided they don't come after a dot, colon or - // question mark. - if (uriString.startsWith("?") || /^[^.:?]*[\s"']/.test(uriString)) { - return tryKeywordFixupForURIInfo( - fixupInfo.originalInput, - fixupInfo, - isPrivateContext, - postData - ); - } - - // Avoid lookup if we can identify a host and it's known. - // Note that if dnsFirstForSingleWords is true isDomainKnown will always - // return true, so we can avoid checking dnsFirstForSingleWords after this. - let asciiHost = fixupInfo.fixedURI?.asciiHost; - if (asciiHost && isDomainKnown(asciiHost)) { - return false; - } - - // Or when the asciiHost is the same as displayHost and there are no - // alphabetical characters - let displayHost = fixupInfo.fixedURI && fixupInfo.fixedURI.displayHost; - let hasAsciiAlpha = asciiAlphaRegex.test(uriString); - if ( - asciiHost && - displayHost && - !hasAsciiAlpha && - asciiHost.toLowerCase() == displayHost.toLowerCase() - ) { - return tryKeywordFixupForURIInfo( - fixupInfo.originalInput, - fixupInfo, - isPrivateContext, - postData - ); - } - - // Avoid lookup if we reached this point and there is a question mark or colon. - if (uriString.includes(":") || uriString.includes("?")) { - return false; - } - - // Keyword lookup if there is exactly one dot and it is the first or last - // character of the input. - let firstDotIndex = uriString.indexOf("."); - if ( - firstDotIndex == uriString.length - 1 || - (firstDotIndex == 0 && firstDotIndex == uriString.lastIndexOf(".")) - ) { - return tryKeywordFixupForURIInfo( - fixupInfo.originalInput, - fixupInfo, - isPrivateContext, - postData - ); - } - - // Keyword lookup if there is no dot and the string doesn't include a slash, - // or any alphabetical character or a host. - if ( - firstDotIndex == -1 && - (!uriString.includes("/") || !hasAsciiAlpha || !asciiHost) - ) { - return tryKeywordFixupForURIInfo( - fixupInfo.originalInput, - fixupInfo, - isPrivateContext, - postData - ); - } - - return false; -} - /** * Mimics the logic in Services.io.extractScheme, but avoids crossing XPConnect. * @param {string} uriString the string to examine diff --git a/docshell/test/unit/test_URIFixup_info.js b/docshell/test/unit/test_URIFixup_info.js index 4bf1909d66d9..c5129f498141 100644 --- a/docshell/test/unit/test_URIFixup_info.js +++ b/docshell/test/unit/test_URIFixup_info.js @@ -33,10 +33,6 @@ var flagInputs = [ Services.uriFixup.FIXUP_FLAG_PRIVATE_CONTEXT, ]; -const kDefaultToSearch = Services.prefs.getBoolPref( - "browser.fixup.defaultToSearch", - true -); /* The following properties are supported for these test cases: { @@ -516,7 +512,7 @@ var testcases = [ alternateURI: "http://www.'.com/?", keywordLookup: true, protocolChange: true, - affectedByDNSForSingleWordHosts: kDefaultToSearch, + affectedByDNSForSingleWordHosts: true, }, { input: "whitelisted?.com", @@ -559,16 +555,16 @@ var testcases = [ fixedURI: "http://mozilla5/2", alternateURI: "http://www.mozilla5.com/2", protocolChange: true, - keywordLookup: kDefaultToSearch, - affectedByDNSForSingleWordHosts: kDefaultToSearch, + keywordLookup: true, + affectedByDNSForSingleWordHosts: true, }, { input: "mozilla/foo", fixedURI: "http://mozilla/foo", alternateURI: "http://www.mozilla.com/foo", protocolChange: true, - keywordLookup: kDefaultToSearch, - affectedByDNSForSingleWordHosts: kDefaultToSearch, + keywordLookup: true, + affectedByDNSForSingleWordHosts: true, }, { input: "mozilla\\", @@ -610,7 +606,7 @@ var testcases = [ fixedURI: "http://plonk/%20#", alternateURI: "http://www.plonk.com/%20#", protocolChange: true, - keywordLookup: !kDefaultToSearch, + keywordLookup: false, }, { input: "blah.com.", @@ -669,16 +665,14 @@ if (AppConstants.platform == "win") { alternateURI: "http://www.mozilla.com/", protocolChange: true, }); - if (kDefaultToSearch) { - testcases.push({ - input: "/a", - fixedURI: "http://a/", - alternateURI: "http://www.a.com/", - keywordLookup: true, - protocolChange: true, - affectedByDNSForSingleWordHosts: true, - }); - } + testcases.push({ + input: "/a", + fixedURI: "http://a/", + alternateURI: "http://www.a.com/", + keywordLookup: true, + protocolChange: true, + affectedByDNSForSingleWordHosts: true, + }); } else { testcases.push({ input: "/some/file.txt",