Backed out changeset 7f58c77bb165 (bug 1057186) for xpshell bustage

docshell/base/nsDefaultURIFixup.cpp

@@ -386,35 +386,14 @@ nsDefaultURIFixup::GetFixupURIInfo(const nsACString& aStringURI, uint32_t aFixup
         info->mFixupCreatedAlternateURI = MakeAlternateURI(info->mFixedURI);
     }
 
-    // If there is no relevent dot in the host, do we require the domain to
-    // be whitelisted?
-    if (info->mFixedURI) {
-        if (aFixupFlags & FIXUP_FLAG_REQUIRE_WHITELISTED_HOST) {
-
-            nsAutoCString asciiHost;
-            if (NS_SUCCEEDED(info->mFixedURI->GetAsciiHost(asciiHost)) &&
-                !asciiHost.IsEmpty()) {
-
-                uint32_t dotLoc = uint32_t(asciiHost.FindChar('.'));
-
-                if ((dotLoc == uint32_t(kNotFound) || dotLoc == asciiHost.Length() - 1)) {
-                    if (IsDomainWhitelisted(asciiHost, dotLoc)) {
-                        info->mPreferredURI = info->mFixedURI;
-                    }
-                } else {
-                    info->mPreferredURI = info->mFixedURI;
-                }
-            }
-        } else {
-            info->mPreferredURI = info->mFixedURI;
-        }
-
-        return NS_OK;
-    }
-
     // If we still haven't been able to construct a valid URI, try to force a
     // keyword match.  This catches search strings with '.' or ':' in them.
-    if (sFixupKeywords && (aFixupFlags & FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP)) {
+    if (info->mFixedURI)
+    {
+        info->mPreferredURI = info->mFixedURI;
+    }
+    else if (sFixupKeywords && (aFixupFlags & FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP))
+    {
         rv = KeywordToURI(aStringURI, aPostData, getter_AddRefs(info->mPreferredURI));
         if (NS_SUCCEEDED(rv) && info->mPreferredURI)
         {
@@ -985,17 +964,26 @@ void nsDefaultURIFixup::KeywordURIFixup(const nsACString & aURIString,
               (dotLoc == lastDotLoc && (dotLoc == 0 || dotLoc == aURIString.Length() - 1))) &&
              colonLoc == uint32_t(kNotFound) && qMarkLoc == uint32_t(kNotFound))
     {
-
         nsAutoCString asciiHost;
         if (aFixupInfo->mFixedURI &&
             NS_SUCCEEDED(aFixupInfo->mFixedURI->GetAsciiHost(asciiHost)) &&
-            !asciiHost.IsEmpty()) {
+            !asciiHost.IsEmpty())
-
+        {
-            if (IsDomainWhitelisted(asciiHost, dotLoc)) {
+            // Check if this domain is whitelisted as an actual
+            // domain (which will prevent a keyword query)
+            // NB: any processing of the host here should stay in sync with
+            // code in the front-end(s) that set the pref.
+            nsAutoCString pref("browser.fixup.domainwhitelist.");
+            if (dotLoc == aURIString.Length() - 1) {
+              pref.Append(Substring(asciiHost, 0, asciiHost.Length() - 1));
+            } else {
+              pref.Append(asciiHost);
+            }
+            if (Preferences::GetBool(pref.get(), false))
+            {
                 return;
             }
         }
-
         // If we get here, we don't have a valid URI, or we did but the
         // host is not whitelisted, so we do a keyword search *anyway*:
         rv = KeywordToURI(aFixupInfo->mOriginalInput, aPostData,
@@ -1007,25 +995,6 @@ void nsDefaultURIFixup::KeywordURIFixup(const nsACString & aURIString,
     }
 }
 
-bool nsDefaultURIFixup::IsDomainWhitelisted(const nsAutoCString aAsciiHost,
-                                            const uint32_t aDotLoc)
-{
-    // Check if this domain is whitelisted as an actual
-    // domain (which will prevent a keyword query)
-    // NB: any processing of the host here should stay in sync with
-    // code in the front-end(s) that set the pref.
-
-    nsAutoCString pref("browser.fixup.domainwhitelist.");
-
-    if (aDotLoc == aAsciiHost.Length() - 1) {
-      pref.Append(Substring(aAsciiHost, 0, aAsciiHost.Length() - 1));
-    } else {
-      pref.Append(aAsciiHost);
-    }
-
-    return Preferences::GetBool(pref.get(), false);
-}
-
 
 nsresult NS_NewURIFixup(nsIURIFixup **aURIFixup)
 {

docshell/base/nsDefaultURIFixup.h

@@ -37,8 +37,6 @@ private:
     bool PossiblyHostPortUrl(const nsACString& aUrl);
     bool MakeAlternateURI(nsIURI *aURI);
     bool IsLikelyFTP(const nsCString& aHostSpec);
-    bool IsDomainWhitelisted(const nsAutoCString aAsciiHost,
-                             const uint32_t aDotLoc);
 };
 
 class nsDefaultURIFixupInfo : public nsIURIFixupInfo

docshell/base/nsIURIFixup.idl

@@ -63,7 +63,7 @@ interface nsIURIFixupInfo : nsISupports
 /**
  * Interface implemented by objects capable of fixing up strings into URIs
  */
-[scriptable, uuid(49298f2b-3630-4874-aecc-522300a7fead)]
+[scriptable, uuid(80d4932e-bb2e-4afb-98e0-de9cc9ea7d82)]
 interface nsIURIFixup : nsISupports
 {
     /** No fixup flags. */
@@ -83,18 +83,12 @@ interface nsIURIFixup : nsISupports
     const unsigned long FIXUP_FLAGS_MAKE_ALTERNATE_URI = 2;
 
     /**
-     * For an input that may be just a domain with only 1 level (eg, "mozilla"),
-     * require that the host be whitelisted.
-     *
-     * Overridden by FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP.
-     */
-    const unsigned long FIXUP_FLAG_REQUIRE_WHITELISTED_HOST = 4;
-
-    /*
      * Fix common scheme typos.
      */
     const unsigned long FIXUP_FLAG_FIX_SCHEME_TYPOS = 8;
 
+    /* Note that flag 4 is available. */
+
     /**
      * Converts an internal URI (e.g. a wyciwyg URI) into one which we can
      * expose to the user, for example on the URL bar.

docshell/test/unit/test_nsDefaultURIFixup_info.js

@@ -3,8 +3,7 @@ let urifixup = Cc["@mozilla.org/docshell/urifixup;1"].
 
 Components.utils.import("resource://gre/modules/Services.jsm");
 
-let prefList = ["browser.fixup.typo.scheme", "keyword.enabled",
+let prefList = ["browser.fixup.typo.scheme", "keyword.enabled"];
-                "browser.fixup.domainwhitelist.whitelisted"];
 for (let pref of prefList) {
   Services.prefs.setBoolPref(pref, true);
 }
@@ -35,8 +34,7 @@ do_register_cleanup(function() {
 let flagInputs = [
   urifixup.FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP,
   urifixup.FIXUP_FLAGS_MAKE_ALTERNATE_URI,
-  urifixup.FIXUP_FLAG_FIX_SCHEME_TYPOS,
+  urifixup.FIXUP_FLAG_FIX_SCHEME_TYPOS
-  urifixup.FIXUP_FLAG_REQUIRE_WHITELISTED_HOST,
 ];
 
 flagInputs.concat([
@@ -46,191 +44,42 @@ flagInputs.concat([
   flagInputs[0] | flagInputs[1] | flagInputs[2]
 ]);
 
-/*
+let testcases = [
-  The following properties are supported for these test cases:
+  ["http://www.mozilla.org", "http://www.mozilla.org/", null, false, false],
-  {
+  ["http://127.0.0.1/", "http://127.0.0.1/", null, false, false],
-    input: "", // Input string, required
+  ["file:///foo/bar", "file:///foo/bar", null, false, false],
-    fixedURI: "", // Expected fixedURI
+  ["://www.mozilla.org", "http://www.mozilla.org/", null, false, true],
-    alternateURI: "", // Expected alternateURI
+  ["www.mozilla.org", "http://www.mozilla.org/", null, false, true],
-    keywordLookup: false, // Whether a keyword lookup is expected
+  ["http://mozilla/", "http://mozilla/", "http://www.mozilla.com/", false, false],
-    protocolChange: false, // Whether a protocol change is expected
+  ["http://test./", "http://test./", "http://www.test./", false, false],
-    affectedByWhitelist: false, // Whether the input host is affected by the whitelist
+  ["127.0.0.1", "http://127.0.0.1/", null, false, true],
-    inWhitelist: false, // Whether the input host is in the whitelist
+  ["1234", "http://1234/", "http://www.1234.com/", true, true],
-  }
+  ["host/foo.txt", "http://host/foo.txt", "http://www.host.com/foo.txt", false, true],
-*/
+  ["mozilla", "http://mozilla/", "http://www.mozilla.com/", true, true],
-let testcases = [ {
+  ["test.", "http://test./", "http://www.test./", true, true],
-    input: "http://www.mozilla.org",
+  [".test", "http://.test/", "http://www..test/", true, true],
-    fixedURI: "http://www.mozilla.org/",
+  ["mozilla is amazing", null, null, true, true],
-  }, {
+  ["mozilla ", "http://mozilla/", "http://www.mozilla.com/", true, true],
-    input: "http://127.0.0.1/",
+  ["   mozilla  ", "http://mozilla/", "http://www.mozilla.com/", true, true],
-    fixedURI: "http://127.0.0.1/",
+  ["mozilla \\", null, null, true, true],
-  }, {
+  ["mozilla \\ foo.txt", null, null, true, true],
-    input: "file:///foo/bar",
+  ["mozilla \\\r foo.txt", null, null, true, true],
-    fixedURI: "file:///foo/bar",
+  ["mozilla\n", "http://mozilla/", "http://www.mozilla.com/", true, true],
-  }, {
+  ["mozilla \r\n", "http://mozilla/", "http://www.mozilla.com/", true, true],
-    input: "://www.mozilla.org",
+  ["moz\r\nfirefox\nos\r", "http://mozfirefoxos/", "http://www.mozfirefoxos.com/", true, true],
-    fixedURI: "http://www.mozilla.org/",
+  ["moz\r\n firefox\n", null, null, true, true],
-    protocolChange: true,
+  ["", null, null, true, true],
-  }, {
+  ["[]", null, null, true, true]
-    input: "www.mozilla.org",
+];
-    fixedURI: "http://www.mozilla.org/",
-    protocolChange: true,
-  }, {
-    input: "http://mozilla/",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-  }, {
-    input: "http://test./",
-    fixedURI: "http://test./",
-    alternateURI: "http://www.test./",
-  }, {
-    input: "127.0.0.1",
-    fixedURI: "http://127.0.0.1/",
-    protocolChange: true,
-  }, {
-    input: "1234",
-    fixedURI: "http://1234/",
-    alternateURI: "http://www.1234.com/",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: "host/foo.txt",
-    fixedURI: "http://host/foo.txt",
-    alternateURI: "http://www.host.com/foo.txt",
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: "mozilla",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: "test.",
-    fixedURI: "http://test./",
-    alternateURI: "http://www.test./",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: ".test",
-    fixedURI: "http://.test/",
-    alternateURI: "http://www..test/",
-    keywordLookup: true,
-    protocolChange: true,
-  }, {
-    input: "mozilla is amazing",
-    keywordLookup: true,
-    protocolChange: true,
-  }, {
-    input: "mozilla ",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: "   mozilla  ",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: "mozilla \\",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: "mozilla \\ foo.txt",
-    keywordLookup: true,
-    protocolChange: true,
-  }, {
-    input: "mozilla \\\r foo.txt",
-    keywordLookup: true,
-    protocolChange: true,
-  }, {
-    input: "mozilla\n",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: "mozilla \r\n",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: "moz\r\nfirefox\nos\r",
-    fixedURI: "http://mozfirefoxos/",
-    alternateURI: "http://www.mozfirefoxos.com/",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  }, {
-    input: "moz\r\n firefox\n",
-    keywordLookup: true,
-    protocolChange: true,
-  }, {
-    input: "",
-    keywordLookup: true,
-    protocolChange: true,
-  }, {
-    input: "[]",
-    keywordLookup: true,
-    protocolChange: true,
-  }, {
-    input: "http://whitelisted/",
-    fixedURI: "http://whitelisted/",
-    alternateURI: "http://www.whitelisted.com/",
-    affectedByWhitelist: true,
-    inWhitelist: true,
-  }];
 
 if (Services.appinfo.OS.toLowerCase().startsWith("win")) {
-  testcases.push({
+  testcases.push(["C:\\some\\file.txt", "file:///C:/some/file.txt", null, false, true]);
-    input: "C:\\some\\file.txt",
+  testcases.push(["//mozilla", "http://mozilla/", "http://www.mozilla.com/", false, true]);
-    fixedURI: "file:///C:/some/file.txt",
+  testcases.push(["mozilla\\", "http://mozilla/", "http://www.mozilla.com/", true, true]);
-    protocolChange: true,
-  });
-  testcases.push({
-    input: "//mozilla",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-    protocolChange: true,
-    affectedByWhitelist: true,
-  });
-  testcases.push({
-    input: "mozilla\\",
-    fixedURI: "http://mozilla/",
-    alternateURI: "http://www.mozilla.com/",
-    keywordLookup: true,
-    protocolChange: true,
-    affectedByWhitelist: true,
-  });
 } else {
-  testcases.push({
+  testcases.push(["/some/file.txt", "file:///some/file.txt", null, false, true]);
-    input: "/some/file.txt",
+  testcases.push(["//mozilla", "file:////mozilla", null, false, true]);
-    fixedURI: "file:///some/file.txt",
+  testcases.push(["mozilla\\", "http://mozilla\\/", "http://www.mozilla/", true, true]);
-    protocolChange: true,
-  });
-  testcases.push({
-    input: "//mozilla",
-    fixedURI: "file:////mozilla",
-    protocolChange: true,
-  });
-  testcases.push({
-    input: "mozilla\\",
-    fixedURI: "http://mozilla\\/",
-    alternateURI: "http://www.mozilla.com/",
-    keywordLookup: true,
-    protocolChange: true,
-  });
 }
 
 function sanitize(input) {
@@ -238,20 +87,8 @@ function sanitize(input) {
 }
 
 function run_test() {
-  for (let { input: testInput,
+  for (let [testInput, expectedFixedURI, alternativeURI,
-             fixedURI: expectedFixedURI,
+            expectKeywordLookup, expectProtocolChange] of testcases) {
-             alternateURI: alternativeURI,
-             keywordLookup: expectKeywordLookup,
-             protocolChange: expectProtocolChange,
-             affectedByWhitelist: affectedByWhitelist,
-             inWhitelist: inWhitelist } of testcases) {
-
-    // Explicitly force these into a boolean
-    expectKeywordLookup = !!expectKeywordLookup;
-    expectProtocolChange = !!expectProtocolChange;
-    affectedByWhitelist = !!affectedByWhitelist;
-    inWhitelist = !!inWhitelist;
-
     for (let flags of flagInputs) {
       let info;
       let fixupURIOnly = null;
@@ -272,12 +109,10 @@ function run_test() {
         continue;
       }
 
-      do_print("Checking \"" + testInput + "\" with flags " + flags);
+      do_print("Checking " + testInput + " with flags " + flags);
 
       // Both APIs should then also be using the same spec.
-      do_check_eq(!!fixupURIOnly, !!info.preferredURI);
+      do_check_eq(fixupURIOnly.spec, info.preferredURI.spec);
-      if (fixupURIOnly)
-        do_check_eq(fixupURIOnly.spec, info.preferredURI.spec);
 
       let isFileURL = expectedFixedURI && expectedFixedURI.startsWith("file");
 
@@ -296,25 +131,10 @@ function run_test() {
       do_check_eq(info.fixupCreatedAlternateURI, makeAlternativeURI && alternativeURI != null);
 
       // Check the preferred URI
-      let requiresWhitelistedDomain = flags & urifixup.FIXUP_FLAG_REQUIRE_WHITELISTED_HOST
+      if (couldDoKeywordLookup && expectKeywordLookup) {
-      if (couldDoKeywordLookup) {
-        if (expectKeywordLookup) {
-          if (!affectedByWhitelist || (affectedByWhitelist && !inWhitelist)) {
         let urlparamInput = encodeURIComponent(sanitize(testInput)).replace("%20", "+", "g");
         let searchURL = kSearchEngineURL.replace("{searchTerms}", urlparamInput);
         do_check_eq(info.preferredURI.spec, searchURL);
-      } else {
-            do_check_eq(info.preferredURI, null);
-          }
-        } else {
-          do_check_eq(info.preferredURI.spec, info.fixedURI.spec);
-        }
-      } else if (requiresWhitelistedDomain) {
-        // Not a keyword search, but we want to enforce the host whitelist
-        if (!affectedByWhitelist || (affectedByWhitelist && inWhitelist))
-          do_check_eq(info.preferredURI.spec, info.fixedURI.spec);
-        else
-          do_check_eq(info.preferredURI, null);
       } else {
         // In these cases, we should never be doing a keyword lookup and
         // the fixed URI should be preferred: