Bug 982428 - allow fixing up URIs with schemes that we can't handle, r=bz

docshell/base/nsDefaultURIFixup.cpp

@@ -23,12 +23,18 @@
 #include "nsIObserverService.h"
 #include "nsXULAppAPI.h"
 
+// Used to check if external protocol schemes are usable
+#include "nsCExternalHandlerService.h"
+#include "nsIExternalProtocolService.h"
+
 using namespace mozilla;
 
 /* Implementation file */
 NS_IMPL_ISUPPORTS(nsDefaultURIFixup, nsIURIFixup)
 
+static bool sInitializedPrefCaches = false;
 static bool sFixTypos = true;
+static bool sFixupKeywords = true;
 
 nsDefaultURIFixup::nsDefaultURIFixup()
 {
@@ -203,12 +209,19 @@ nsDefaultURIFixup::CreateFixupURI(const nsACString& aStringURI, uint32_t aFixupF
 #endif
     }
 
-    // Check if we want to fix up common scheme typos.
-    rv = Preferences::AddBoolVarCache(&sFixTypos,
-                                      "browser.fixup.typo.scheme",
-                                      sFixTypos);
-    MOZ_ASSERT(NS_SUCCEEDED(rv),
-              "Failed to observe \"browser.fixup.typo.scheme\"");
+    if (!sInitializedPrefCaches) {
+      // Check if we want to fix up common scheme typos.
+      rv = Preferences::AddBoolVarCache(&sFixTypos,
+                                        "browser.fixup.typo.scheme",
+                                        sFixTypos);
+      MOZ_ASSERT(NS_SUCCEEDED(rv),
+                "Failed to observe \"browser.fixup.typo.scheme\"");
+
+      rv = Preferences::AddBoolVarCache(&sFixupKeywords, "keyword.enabled",
+                                        sFixupKeywords);
+      MOZ_ASSERT(NS_SUCCEEDED(rv), "Failed to observe \"keyword.enabled\"");
+      sInitializedPrefCaches = true;
+    }
 
     // Fix up common scheme typos.
     if (sFixTypos && (aFixupFlags & FIXUP_FLAG_FIX_SCHEME_TYPOS)) {
@@ -262,6 +275,27 @@ nsDefaultURIFixup::CreateFixupURI(const nsACString& aStringURI, uint32_t aFixupF
             return rv;
         }
     }
+
+    if (*aURI && ourHandler == extHandler && sFixupKeywords &&
+        (aFixupFlags & FIXUP_FLAG_FIX_SCHEME_TYPOS)) {
+        nsCOMPtr<nsIExternalProtocolService> extProtService =
+            do_GetService(NS_EXTERNALPROTOCOLSERVICE_CONTRACTID);
+        if (extProtService) {
+            bool handlerExists = false;
+            rv = extProtService->ExternalProtocolHandlerExists(scheme.get(), &handlerExists);
+            if (NS_FAILED(rv)) {
+                return rv;
+            }
+            // This basically means we're dealing with a theoretically valid
+            // URI... but we have no idea how to load it. (e.g. "christmas:humbug")
+            // It's more likely the user wants to search, and so we
+            // chuck this over to their preferred search provider instead:
+            if (!handlerExists) {
+                NS_RELEASE(*aURI);
+                KeywordToURI(uriString, aPostData, aURI);
+            }
+        }
+    }
     
     if (*aURI) {
         if (aFixupFlags & FIXUP_FLAGS_MAKE_ALTERNATE_URI)
@@ -271,16 +305,10 @@ nsDefaultURIFixup::CreateFixupURI(const nsACString& aStringURI, uint32_t aFixupF
 
     // See if it is a keyword
     // Test whether keywords need to be fixed up
-    bool fixupKeywords = false;
-    if (aFixupFlags & FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP) {
-        nsresult rv = Preferences::GetBool("keyword.enabled", &fixupKeywords);
-        NS_ENSURE_SUCCESS(rv, NS_ERROR_FAILURE);
-        if (fixupKeywords)
-        {
-            KeywordURIFixup(uriString, aPostData, aURI);
-            if(*aURI)
-                return NS_OK;
-        }
+    if (sFixupKeywords && (aFixupFlags & FIXUP_FLAG_ALLOW_KEYWORD_LOOKUP)) {
+        KeywordURIFixup(uriString, aPostData, aURI);
+        if(*aURI)
+            return NS_OK;
     }
 
     // Prune duff protocol schemes
@@ -337,7 +365,7 @@ nsDefaultURIFixup::CreateFixupURI(const nsACString& aStringURI, uint32_t aFixupF
 
     // 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 (!*aURI && fixupKeywords)
+    if (!*aURI && sFixupKeywords)
     {
         KeywordToURI(aStringURI, aPostData, aURI);
         if(*aURI)
@@ -397,21 +425,21 @@ NS_IMETHODIMP nsDefaultURIFixup::KeywordToURI(const nsACString& aKeyword,
         searchSvc->GetDefaultEngine(getter_AddRefs(defaultEngine));
         if (defaultEngine) {
             nsCOMPtr<nsISearchSubmission> submission;
+            nsAutoString responseType;
             // We allow default search plugins to specify alternate
             // parameters that are specific to keyword searches.
             NS_NAMED_LITERAL_STRING(mozKeywordSearch, "application/x-moz-keywordsearch");
             bool supportsResponseType = false;
             defaultEngine->SupportsResponseType(mozKeywordSearch, &supportsResponseType);
-            if (supportsResponseType)
-              defaultEngine->GetSubmission(NS_ConvertUTF8toUTF16(keyword),
-                                           mozKeywordSearch,
-                                           NS_LITERAL_STRING("keyword"),
-                                           getter_AddRefs(submission));
-            else
-              defaultEngine->GetSubmission(NS_ConvertUTF8toUTF16(keyword),
-                                           EmptyString(),
-                                           NS_LITERAL_STRING("keyword"),
-                                           getter_AddRefs(submission));
+            if (supportsResponseType) {
+                responseType.Assign(mozKeywordSearch);
+            }
+
+            defaultEngine->GetSubmission(NS_ConvertUTF8toUTF16(keyword),
+                                         responseType,
+                                         NS_LITERAL_STRING("keyword"),
+                                         getter_AddRefs(submission));
+
             if (submission) {
                 nsCOMPtr<nsIInputStream> postData;
                 submission->GetPostData(getter_AddRefs(postData));

docshell/base/nsDocShell.cpp

@@ -1571,9 +1571,6 @@ nsDocShell::LoadURI(nsIURI * aURI,
     if (aLoadFlags & LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP)
         flags |= INTERNAL_LOAD_FLAGS_ALLOW_THIRD_PARTY_FIXUP;
 
-    if (aLoadFlags & LOAD_FLAGS_FIXUP_SCHEME_TYPOS)
-        flags |= INTERNAL_LOAD_FLAGS_FIXUP_SCHEME_TYPOS;
-
     if (aLoadFlags & LOAD_FLAGS_FIRST_LOAD)
         flags |= INTERNAL_LOAD_FLAGS_FIRST_LOAD;
 

docshell/base/nsIDocShell.idl

@@ -46,7 +46,7 @@ interface nsITabParent;
  
 typedef unsigned long nsLoadFlags;
 
-[scriptable, builtinclass, uuid(2a099e83-44df-415a-be76-c145966074e7)]
+[scriptable, builtinclass, uuid(e5fe5c76-e511-4da3-9709-f8294b8dc5ce)]
 interface nsIDocShell : nsIDocShellTreeItem
 {
   /**
@@ -113,9 +113,10 @@ interface nsIDocShell : nsIDocShellTreeItem
   // Whether the load should be treated as srcdoc load, rather than a URI one.
   const long INTERNAL_LOAD_FLAGS_IS_SRCDOC               = 0x40;
 
-  const long INTERNAL_LOAD_FLAGS_FIXUP_SCHEME_TYPOS      = 0x80;
   const long INTERNAL_LOAD_FLAGS_NO_OPENER               = 0x100;
 
+  // NB: 0x80 is available.
+
   /**
    * Loads the given URI.  This method is identical to loadURI(...) except
    * that its parameter list is broken out instead of being packaged inside

docshell/test/browser/browser.ini

@@ -88,6 +88,8 @@ skip-if = e10s # Bug ?????? - event handler checks event.target is the content d
 skip-if = e10s # Bug ?????? - BrowserSetForcedCharacterSet() in browser.js references docShell
 [browser_bug941562.js]
 skip-if = e10s # Bug ?????? - event handler checks event.target is the content document and test e10s-utils doesn't do that.
+[browser_uriFixupIntegration.js]
+skip-if = e10s
 [browser_loadDisallowInherit.js]
 [browser_loadURI.js]
 skip-if = e10s # Bug ?????? - event handler checks event.target is the content document and test e10s-utils doesn't do that.

docshell/test/browser/browser_uriFixupIntegration.js

@@ -0,0 +1,81 @@
+/* Any copyright is dedicated to the Public Domain.
+ * http://creativecommons.org/publicdomain/zero/1.0/ */
+
+const Cc = Components.classes;
+const Ci = Components.interfaces;
+
+const kSearchEngineID = "browser_urifixup_search_engine";
+const kTest
+const kSearchEngineURL = "http://example.com/?search={searchTerms}";
+Services.search.addEngineWithDetails(kSearchEngineID, "", "", "", "get",
+                                     kSearchEngineURL);
+
+let oldDefaultEngine = Services.search.defaultEngine;
+Services.search.defaultEngine = Services.search.getEngineByName(kSearchEngineID);
+
+let tab;
+let searchParams;
+
+function checkURL() {
+  let escapedParams = encodeURIComponent(searchParams).replace("%20", "+");
+  let expectedURL = kSearchEngineURL.replace("{searchTerms}", escapedParams);
+  is(tab.linkedBrowser.currentURI.spec, expectedURL,
+     "New tab should have loaded with expected url.");
+}
+
+function addPageShowListener(aFunc) {
+  gBrowser.selectedBrowser.addEventListener("pageshow", function loadListener() {
+    gBrowser.selectedBrowser.removeEventListener("pageshow", loadListener, false);
+    aFunc();
+  });
+}
+
+function locationBarEnter(aCallback) {
+  executeSoon(function() {
+    gURLBar.focus();
+    EventUtils.synthesizeKey("VK_RETURN", {});
+    addPageShowListener(aCallback);
+  });
+}
+
+let urlbarInput = [
+  "foo bar",
+  "brokenprotocol:somethingelse"
+];
+function test() {
+  waitForExplicitFinish();
+
+  nextTest();
+}
+
+function nextTest() {
+  searchParams = urlbarInput.pop();
+  tab = gBrowser.selectedTab = gBrowser.addTab();
+
+  gURLBar.value = searchParams;
+  locationBarEnter(function() {
+    checkURL();
+    gBrowser.removeTab(tab);
+    tab = null;
+    if (urlbarInput.length) {
+      nextTest();
+    } else {
+      finish();
+    }
+  });
+}
+
+registerCleanupFunction(function () {
+  if (tab) {
+    gBrowser.removeTab(tab);
+  }
+
+  if (oldDefaultEngine) {
+    Services.search.defaultEngine = oldDefaultEngine;
+  }
+  let engine = Services.search.getEngineByName(kSearchEngineID);
+  if (engine) {
+    Services.search.removeEngine(engine);
+  }
+});
+

docshell/test/unit/test_nsDefaultURIFixup_search.js

@@ -0,0 +1,57 @@
+let urifixup = Cc["@mozilla.org/docshell/urifixup;1"].
+               getService(Ci.nsIURIFixup);
+Components.utils.import("resource://gre/modules/Services.jsm");
+
+Services.prefs.setBoolPref("keyword.enabled", true);
+
+const kSearchEngineID = "test_urifixup_search_engine";
+const kSearchEngineURL = "http://www.example.org/?search={searchTerms}";
+Services.search.addEngineWithDetails(kSearchEngineID, "", "", "", "get",
+                                     kSearchEngineURL);
+
+let oldDefaultEngine = Services.search.defaultEngine;
+Services.search.defaultEngine = Services.search.getEngineByName(kSearchEngineID);
+
+let selectedName = Services.search.defaultEngine.name;
+do_check_eq(selectedName, kSearchEngineID);
+
+do_register_cleanup(function() {
+  if (oldDefaultEngine) {
+    Services.search.defaultEngine = oldDefaultEngine;
+  }
+  let engine = Services.search.getEngineByName(kSearchEngineID);
+  if (engine) {
+    Services.search.removeEngine(engine);
+  }
+  Services.prefs.clearUserPref("keyword.enabled");
+});
+
+let data = [
+  {
+    // Valid should not be changed.
+    wrong: 'https://example.com/this/is/a/test.html',
+    fixed: 'https://example.com/this/is/a/test.html',
+  },
+  {
+    // Unrecognized protocols should be changed.
+    wrong: 'whatever://this/is/a/test.html',
+    fixed: kSearchEngineURL.replace("{searchTerms}", encodeURIComponent('whatever://this/is/a/test.html')),
+  },
+];
+
+
+function run_test() {
+  run_next_test();
+}
+
+let len = data.length;
+// Make sure we fix what needs fixing
+add_task(function test_fix_unknown_schemes() {
+  for (let i = 0; i < len; ++i) {
+    let item = data[i];
+    let result =
+      urifixup.createFixupURI(item.wrong,
+                              urifixup.FIXUP_FLAG_FIX_SCHEME_TYPOS).spec;
+    do_check_eq(result, item.fixed);
+  }
+});

docshell/test/unit/xpcshell.ini

@@ -5,6 +5,8 @@ tail =
 [test_bug414201_jfif.js]
 [test_bug442584.js]
 [test_nsDefaultURIFixup.js]
+[test_nsDefaultURIFixup_search.js]
+skip-if = os == 'android'
 [test_nsIDownloadHistory.js]
 [test_pb_notification.js]
 # Bug 751575: unrelated JS changes cause timeouts on random platforms