Bug 1641467 - Remove temporary browser.fixup.defaultToSearch feature pref and its code. r=Gijs

Differential Revision: https://phabricator.services.mozilla.com/D83554
2020-07-15 09:53:04 +00:00 · 2020-07-15 09:53:04 +00:00 · 7e76f843c7
--- 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",
--- 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");
--- 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,
--- 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
--- 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",