Bug 1528305 - Don't show an error page for unknown protocols for page…

…-triggered navigations that replace a document. r=mattwoodrow Pages apparently do this to try to open to native apps, and that on Firefox causes an error page to be shown if the app is not installed, which is pretty bad. Differential Revision: https://phabricator.services.mozilla.com/D68471 --HG-- extra : moz-landing-system : lando
mozilla · Mar 27, 2020 · 266c1d0 · 266c1d0
1 parent 9e89b53
commit 266c1d0
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 1 deletion.
diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp
@@ -6354,7 +6354,6 @@ nsresult nsDocShell::EndPageLoad(nsIWebProgress* aProgress,
                aStatus == NS_ERROR_PROXY_GATEWAY_TIMEOUT ||
                aStatus == NS_ERROR_REDIRECT_LOOP ||
                aStatus == NS_ERROR_UNKNOWN_SOCKET_TYPE ||
-               aStatus == NS_ERROR_UNKNOWN_PROTOCOL ||
                aStatus == NS_ERROR_NET_INTERRUPT ||
                aStatus == NS_ERROR_NET_RESET ||
                aStatus == NS_ERROR_PROXY_BAD_GATEWAY ||
@@ -6371,6 +6370,33 @@ nsresult nsDocShell::EndPageLoad(nsIWebProgress* aProgress,
                NS_ERROR_GET_MODULE(aStatus) == NS_ERROR_MODULE_SECURITY) {
       // Errors to be shown for any frame
       DisplayLoadError(aStatus, url, nullptr, aChannel);
+    } else if (aStatus == NS_ERROR_UNKNOWN_PROTOCOL) {
+      // For unknown protocols we only display an error if the load is triggered
+      // by the browser itself, or we're replacing the initial document (and
+      // nothing else). Showing the error for page-triggered navigations causes
+      // annoying behavior for users, see bug 1528305.
+      //
+      // We could, maybe, try to detect if this is in response to some user
+      // interaction (like clicking a link, or something else) and maybe show
+      // the error page in that case. But this allows for ctrl+clicking and such
+      // to see the error page.
+      nsCOMPtr<nsILoadInfo> info = aChannel->LoadInfo();
+      Document* doc = GetDocument();
+      if (!info->TriggeringPrincipal()->IsSystemPrincipal() &&
+          StaticPrefs::dom_no_unknown_protocol_error_enabled() &&
+          doc && !doc->IsInitialDocument()) {
+        nsTArray<nsString> params;
+        if (NS_FAILED(NS_GetSanitizedURIStringFromURI(
+                url, *params.AppendElement()))) {
+          params.LastElement().AssignLiteral(u"(unknown uri)");
+        }
+        nsContentUtils::ReportToConsole(
+            nsIScriptError::warningFlag, NS_LITERAL_CSTRING("DOM"), doc,
+            nsContentUtils::eDOM_PROPERTIES,
+            "UnknownProtocolNavigationPrevented", params);
+      } else {
+        DisplayLoadError(aStatus, url, nullptr, aChannel);
+      }
     } else if (aStatus == NS_ERROR_DOCUMENT_NOT_CACHED) {
       // Non-caching channels will simply return NS_ERROR_OFFLINE.
       // Caching channels would have to look at their flags to work

diff --git a/dom/base/test/file_location_href_unknown_protocol.html b/dom/base/test/file_location_href_unknown_protocol.html
@@ -0,0 +1,15 @@
+<!doctype html>
+<script>
+onbeforeunload = function() {
+  opener.onChildBeforeUnload();
+};
+onload = function() {
+  location.href = "this-protocol-is-unlikely-to-exist://foo";
+  setTimeout(function() {
+    opener.onChildLoadTimedOut();
+  }, 1000);
+};
+onunload = function() {
+  opener.onChildUnload();
+};
+</script>
diff --git a/dom/base/test/mochitest.ini b/dom/base/test/mochitest.ini
@@ -695,6 +695,9 @@ skip-if = (verify && (os == 'win' || os == 'mac'))
 skip-if = !e10s # Track Bug 1281415
 [test_link_preload.html]
 [test_link_stylesheet.html]
+[test_location_href_unknown_protocol.html]
+support-files = file_location_href_unknown_protocol.html
+skip-if = fission # bug 1625530
 [test_messagemanager_targetchain.html]
 [test_meta_refresh_referrer.html]
 [test_meta_viewport0.html]

diff --git a/dom/base/test/test_location_href_unknown_protocol.html b/dom/base/test/test_location_href_unknown_protocol.html
@@ -0,0 +1,27 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>Test for window.location setter to an unknown protocol (bug 1528305).</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<script>
+SimpleTest.waitForExplicitFinish();
+let beforeunload = false;
+let unload = false;
+
+window.onChildBeforeUnload = function() {
+  beforeunload = true;
+};
+
+window.onChildUnload = function() {
+  unload = true;
+};
+
+let win;
+window.onChildLoadTimedOut = function() {
+  ok(!unload, "shouldn't have unloaded child window");
+  ok(beforeunload, "should've fired a beforeunload event");
+  win.close();
+  SimpleTest.finish();
+};
+
+win = window.open("file_location_href_unknown_protocol.html");
+</script>
diff --git a/dom/locales/en-US/chrome/dom/dom.properties b/dom/locales/en-US/chrome/dom/dom.properties
@@ -392,3 +392,5 @@ MathML_DeprecatedStyleAttributeWarning=MathML attributes “background”, “co
 MathML_DeprecatedXLinkAttributeWarning=XLink attributes “href”, “type”, “show” and “actuate” are deprecated on MathML elements and will be removed at a future date.
 WebShareAPI_Failed=The share operation has failed.
 WebShareAPI_Aborted=The share operation was aborted.
+# LOCALIZATION NOTE (UnknownProtocolNavigationPrevented): %1$S is the destination URL.
+UnknownProtocolNavigationPrevented=Prevented navigation to “%1$S” due to an unknown protocol.
diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml
@@ -2076,6 +2076,13 @@
   value: @IS_ANDROID@
   mirror: always
 
+# Whether we shouldn't show an error page for unknown protocols (and should
+# show a console warning instead).
+- name: dom.no_unknown_protocol_error.enabled
+  type: bool
+  value: true
+  mirror: always
+
 # Is support for Window.paintWorklet enabled?
 - name: dom.paintWorklet.enabled
   type: bool