Bug 836567 - Part 2: Replace inapplicable tests with a web-platform-test for reloading after setting javascript: URI, and fix other tests relying on javascript: URI. r=Gijs,miker

Some notes about the changes:

Both test_bug384014.html and test_bug123696.html were testing reloading of
javascript: URI. The expected result of the iframes after reloading would
become about:blank. I deleted both file and instead wrote with
web-platform-test to cover reloading of javascript: URI since wpt is more
preferable.

storage-cache-error.html was utilizing javascript: URI to test bug 1262766.
javascript: URI would cause CacheStorage::Keys to throw a dom security
exception because the URI's principal (which did not match the principal of the
document in this case!) was a nullpricipal.  With my patches the iframe's URL
would no longer be the javascript: URI, so it's no longer applicable for the
test case. Instead we can test what bug 1262766 was originally about - that
CacheStorage::Keys would throw a dom security exception if it's in a private
browsing window.

Differential Revision: https://phabricator.services.mozilla.com/D59465

--HG--
extra : moz-landing-system : lando

browser/base/content/test/general/browser_newWindowDrop.js

@@ -41,10 +41,10 @@ add_task(async function single_url() {
   await dropText("mochi.test/first", ["http://www.mochi.test/first"]);
 });
 add_task(async function single_javascript() {
-  await dropText("javascript:'bad'", ["javascript:'bad'"]);
+  await dropText("javascript:'bad'", ["about:blank"]);
 });
 add_task(async function single_javascript_capital() {
-  await dropText("jAvascript:'bad'", ["javascript:'bad'"]);
+  await dropText("jAvascript:'bad'", ["about:blank"]);
 });
 add_task(async function single_url2() {
   await dropText("mochi.test/second", ["http://www.mochi.test/second"]);
@@ -65,7 +65,7 @@ add_task(async function multiple_urls() {
 });
 add_task(async function multiple_urls_javascript() {
   await dropText("javascript:'bad1'\nmochi.test/3", [
-    "javascript:'bad1'",
+    "about:blank",
     "http://www.mochi.test/3",
   ]);
 });

browser/components/urlbar/tests/browser/browser_keywordBookmarklets.js

@@ -7,7 +7,7 @@ add_task(async function setup() {
   let bm = await PlacesUtils.bookmarks.insert({
     parentGuid: PlacesUtils.bookmarks.unfiledGuid,
     title: "bookmarklet",
-    url: "javascript:'%s'%20",
+    url: "javascript:'%sx'%20",
   });
   await PlacesUtils.keywords.insert({ keyword: "bm", url: bm.url });
   registerCleanupFunction(async function() {
@@ -20,47 +20,47 @@ add_task(async function setup() {
       gURLBar.value = "bm";
       gURLBar.focus();
       EventUtils.synthesizeKey("KEY_Enter");
-      return "javascript:''%20";
+      return "x";
     },
     function() {
       info("Type keyword with searchstring and immediately press enter");
       gURLBar.value = "bm a";
       gURLBar.focus();
       EventUtils.synthesizeKey("KEY_Enter");
-      return "javascript:'a'%20";
+      return "ax";
     },
     async function() {
       info("Search keyword, then press enter");
       await promiseAutocompleteResultPopup("bm");
       let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
-      Assert.equal(result.title, "javascript:'' ", "Check title");
+      Assert.equal(result.title, "javascript:'x' ", "Check title");
       EventUtils.synthesizeKey("KEY_Enter");
-      return "javascript:''%20";
+      return "x";
     },
     async function() {
       info("Search keyword with searchstring, then press enter");
       await promiseAutocompleteResultPopup("bm a");
       let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
-      Assert.equal(result.title, "javascript:'a' ", "Check title");
+      Assert.equal(result.title, "javascript:'ax' ", "Check title");
       EventUtils.synthesizeKey("KEY_Enter");
-      return "javascript:'a'%20";
+      return "ax";
     },
     async function() {
       await promiseAutocompleteResultPopup("bm");
       let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
-      Assert.equal(result.title, "javascript:'' ", "Check title");
+      Assert.equal(result.title, "javascript:'x' ", "Check title");
       let element = UrlbarTestUtils.getSelectedRow(window);
       EventUtils.synthesizeMouseAtCenter(element, {});
-      return "javascript:''%20";
+      return "x";
     },
     async function() {
       info("Search keyword with searchstring, then click");
       await promiseAutocompleteResultPopup("bm a");
       let result = await UrlbarTestUtils.getDetailsOfResultAt(window, 0);
-      Assert.equal(result.title, "javascript:'a' ", "Check title");
+      Assert.equal(result.title, "javascript:'ax' ", "Check title");
       let element = UrlbarTestUtils.getSelectedRow(window);
       EventUtils.synthesizeMouseAtCenter(element, {});
-      return "javascript:'a'%20";
+      return "ax";
     },
   ];
   for (let testFn of testFns) {
@@ -78,10 +78,15 @@ async function do_test(loadFn) {
       let originalPrincipalURI = await getPrincipalURI(browser);
 
       let promise = BrowserTestUtils.waitForContentEvent(browser, "pageshow");
-      let expectedUrl = await loadFn();
+      const expectedTextContent = await loadFn();
       info("Awaiting pageshow event");
       await promise;
-      Assert.equal(gBrowser.currentURI.spec, expectedUrl);
+      // URI should not change when we run a javascript: URL.
+      Assert.equal(gBrowser.currentURI.spec, "about:blank");
+      const textContent = await ContentTask.spawn(browser, [], function() {
+        return content.document.documentElement.textContent;
+      });
+      Assert.equal(textContent, expectedTextContent);
 
       let newPrincipalURI = await getPrincipalURI(browser);
       Assert.equal(

browser/components/urlbar/tests/browser/browser_urlbarCopying.js

@@ -200,17 +200,16 @@ var tests = [
     copyExpected: "http://example.com/%E3%80%80%E3%80%80",
   },
 
-  // data: and javsacript: URIs shouldn't be encoded
+  // Loading of javascript: URI results in previous URI, so if the previous
+  // entry changes, change this one too!
   {
     loadURL: "javascript:('%C3%A9%20%25%50')",
-    expectedURL: "javascript:('%C3%A9 %25P')",
-    copyExpected: "javascript:('%C3%A9 %25P')",
-  },
-  {
-    copyVal: "<javascript:(>'%C3%A9 %25P')",
-    copyExpected: "javascript:(",
+    expectedLoad: "http://example.com/%E3%80%80%E3%80%80",
+    expectedURL: "example.com/%E3%80%80%E3%80%80",
+    copyExpected: "http://example.com/%E3%80%80%E3%80%80",
   },
 
+  // data: URIs shouldn't be encoded
   {
     loadURL: "data:text/html,(%C3%A9%20%25%50)",
     expectedURL: "data:text/html,(%C3%A9 %25P)",
@@ -264,7 +263,8 @@ function runTest(testCase, cb) {
 
   if (testCase.loadURL) {
     info(`Loading : ${testCase.loadURL}\n`);
-    loadURL(testCase.loadURL, doCheck);
+    let expectedLoad = testCase.expectedLoad || testCase.loadURL;
+    loadURL(testCase.loadURL, expectedLoad, doCheck);
   } else {
     if (testCase.setURL) {
       gURLBar.value = testCase.setURL;
@@ -323,9 +323,11 @@ function testCopy(copyVal, targetValue, cb) {
   );
 }
 
-function loadURL(aURL, aCB) {
+function loadURL(aURL, aExpectedLoad, aCB) {
   BrowserTestUtils.loadURI(gBrowser.selectedBrowser, aURL);
-  BrowserTestUtils.browserLoaded(gBrowser.selectedBrowser, false, aURL).then(
-    aCB
-  );
+  BrowserTestUtils.browserLoaded(
+    gBrowser.selectedBrowser,
+    false,
+    aExpectedLoad
+  ).then(aCB);
 }

browser/components/urlbar/tests/browser/browser_urlbar_blanking.js

@@ -21,6 +21,12 @@ add_task(async function() {
 });
 
 add_task(async function() {
+  // The test was originally to check that reloading of a javascript: URL could
+  // throw an error and empty the URL bar. This situation can no longer happen
+  // as in bug 836567 we set document.URL to active document's URL on navigation
+  // to a javascript: URL; reloading after that will simply reload the original
+  // active document rather than the javascript: URL itself. But we can still
+  // verify that the URL bar's value is correct.
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);
   is(gURLBar.value, TEST_URL, "The URL bar should match the URI");
   let browserLoaded = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
@@ -28,18 +34,21 @@ add_task(async function() {
     content.document.querySelector("a").click();
   });
   await browserLoaded;
-  ok(
-    gURLBar.value.startsWith("javascript"),
-    "The URL bar should have the JS URI"
+  is(
+    gURLBar.value,
+    TEST_URL,
+    "The URL bar should be the previous active document's URI."
   );
-  // When reloading, the javascript: uri we're using will throw an exception.
-  // That's deliberate, so we need to tell mochitest to ignore it:
   await SpecialPowers.spawn(tab.linkedBrowser, [], async function() {
     // This is sync, so by the time we return we should have changed the URL bar.
     content.location.reload();
   }).catch(e => {
     // Ignore expected exception.
   });
-  ok(!!gURLBar.value, "URL bar should not be blank.");
+  is(
+    gURLBar.value,
+    TEST_URL,
+    "The URL bar should still be the previous active document's URI."
+  );
   BrowserTestUtils.removeTab(tab);
 });

devtools/client/storage/test/browser_storage_cache_error.js

@@ -7,9 +7,22 @@
 // Test handling errors in CacheStorage
 
 add_task(async function() {
-  await openTabAndSetupStorage(MAIN_DOMAIN + "storage-cache-error.html");
+  // Open the URL in a private browsing window.
+  const win = await BrowserTestUtils.openNewBrowserWindow({ private: true });
+  const tab = win.gBrowser.selectedBrowser;
+  const systemPrincipal = Services.scriptSecurityManager.getSystemPrincipal();
+  tab.loadURI(MAIN_DOMAIN + "storage-cache-error.html", {
+    triggeringPrincipal: systemPrincipal,
+  });
+  await BrowserTestUtils.browserLoaded(tab);
 
-  const cacheItemId = ["Cache", "javascript:parent.frameContent"];
+  // On enumerating cache storages, CacheStorage::Keys would throw a
+  // DOM security exception. We'd like to verify storage panel still work in
+  // this case.
+  const target = await TargetFactory.forTab(win.gBrowser.selectedTab);
+  await openStoragePanel(null, target);
+
+  const cacheItemId = ["Cache", "http://test2.example.org"];
 
   await selectTreeItem(cacheItemId);
   ok(
@@ -17,5 +30,6 @@ add_task(async function() {
     `The item ${cacheItemId.join(" > ")} is present in the tree`
   );
 
+  await BrowserTestUtils.closeWindow(win);
   await finishTests();
 });

devtools/client/storage/test/storage-cache-error.html

@@ -4,17 +4,8 @@
   <meta charset="utf-8">
   <title>Storage inspector test for handling errors in CacheStorage</title>
 </head>
+<!-- The test case would load this page in a private browsing window -->
 <body>
-<script type="application/javascript">
-"use strict";
-
-// Create an iframe with a javascript: source URL. Such iframes are
-// considered untrusted by the CacheStorage.
-const frameEl = document.createElement("iframe");
-document.body.appendChild(frameEl);
-
-window.frameContent = 'Hello World';
-frameEl.contentWindow.location.href = "javascript:parent.frameContent";
-</script>
+  <iframe src="http://test2.example.org"></iframe>
 </body>
 </html>

docshell/test/mochitest/bug123696-subframe.html

@@ -1,16 +0,0 @@
-<!DOCTYPE html>
-<html>
-  <head>
-    <script>
-      function change() {
-        return "<html><body>change</body></html>";
-      }
-      function change2() {
-        return "<html><body>change2</body></html>";
-      }
-    </script>
-  </head>
-  <body>
-    <iframe src="javascript:parent.change()"></iframe>
-  </body>
-</html>

docshell/test/mochitest/mochitest.ini

@@ -1,6 +1,5 @@
 [DEFAULT]
 support-files =
-  bug123696-subframe.html
   bug404548-subframe.html
   bug404548-subframe_window.html
   bug413310-post.sjs
@@ -56,13 +55,11 @@ support-files =
 
 [test_anchor_scroll_after_document_open.html]
 [test_bfcache_plus_hash.html]
-[test_bug123696.html]
 [test_bug1422334.html]
 support-files =
   bug1422334_redirect.html
   bug1422334_redirect.html^headers^
   !/docshell/test/navigation/blank.html
-[test_bug384014.html]
 [test_bug385434.html]
 [test_bug387979.html]
 [test_bug402210.html]

docshell/test/mochitest/test_bug123696.html

@@ -1,46 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<!--
-https://bugzilla.mozilla.org/show_bug.cgi?id=123696
--->
-<head>
-  <title>Test for Bug 123696</title>
-  <script src="/tests/SimpleTest/SimpleTest.js"></script>
-  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
-</head>
-<body>
-<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=123696">Mozilla Bug 123696</a>
-<p id="display">
-  <iframe src="bug123696-subframe.html"></iframe>
-</p>
-<div id="content" style="display: none">
-  
-</div>
-<pre id="test">
-<script type="application/javascript">
-
-/** Test for Bug 123696 **/
-SimpleTest.waitForExplicitFinish();
-
-function finishTest() {
-  is(window.frames[0].frames[0].document.documentElement.textContent,
-     "change2", "Reload should have reloaded correctly!");
-  SimpleTest.finish();
-}
-
-function doReload() {
-  window.frames[0].frameElement.onload = finishTest;
-  window.frames[0].location.reload();
-}
-
-addLoadEvent(function() {
-  window.frames[0].frames[0].frameElement.onload = doReload;
-  window.frames[0].frames[0].frameElement.src = "javascript:parent.change2()";
-});
-
-
-
-</script>
-</pre>
-</body>
-</html>

docshell/test/mochitest/test_bug384014.html

@@ -1,41 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<!--
-https://bugzilla.mozilla.org/show_bug.cgi?id=384014
--->
-<head>
-  <title>Test for Bug 384014</title>
-  <script src="/tests/SimpleTest/SimpleTest.js"></script>
-  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css" />
-</head>
-<body>
-<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=384014">Mozilla Bug 384014</a>
-<p id="display">
-<iframe id="f" src="javascript:try { window.x = 'PASS'; s = 'PASS' } catch(e) { s = 'FAIL' } s;"></iframe>
-</p>
-<div id="content" style="display: none">
-  
-</div>
-<pre id="test">
-<script class="testbody" type="text/javascript">
-
-/** Test for Bug 384014 **/
-SimpleTest.waitForExplicitFinish();
-
-function runTest() {
-  $("f").onload = function() {
-    is($("f").contentDocument.documentElement.textContent, "PASS",
-      "We fail");
-    SimpleTest.finish();
-  };
-
-  $("f").contentWindow.location.reload();
-}
-
-addLoadEvent(runTest);
-
-</script>
-</pre>
-</body>
-</html>
-

testing/web-platform/meta/html/semantics/embedded-content/the-iframe-element/iframe_javascript_url_01.htm.ini

@@ -1,4 +0,0 @@
-[iframe_javascript_url_01.htm]
-  [javascript: URL creating a document in an about:blank iframe]
-    expected: FAIL
-

testing/web-platform/tests/html/browsers/history/the-location-interface/location_reload_javascript_url.html

@@ -0,0 +1,60 @@
+<!DOCTYPE HTML>
+<html>
+  <head>
+    <title>location_reload_javascript_url</title>
+    <script src="/resources/testharness.js"></script>
+    <script src="/resources/testharnessreport.js"></script>
+  </head>
+  <body>
+    <div id="log"></div>
+
+    <iframe></iframe>
+
+    <script>
+    async_test(function(t) {
+      const URL = "/common/blank.html";
+      const URL2 = "/common/blank.html#foo";
+      const JS_URL_TEXT = "javascript generated page";
+      const JS_URL = "javascript:'<html>" + JS_URL_TEXT + "</html>'";
+
+      var iframe = document.querySelector("iframe");
+      var count = 0;
+      iframe.onload = t.step_func(function() {
+        // The URL should initially be "blank.html", and then "blank.html#foo";
+        // The textContent of the iframe's document should initially be blank,
+        // then become js generated text, and then be blank again after reload.
+        switch (count) {
+        case 0:
+          assert_equals(iframe.contentWindow.document.URL,
+                        location.href.replace(location.pathname, URL),
+                        "iframe url (" + count + ")");
+          assert_equals(iframe.contentDocument.body.textContent, "",
+                        "text of blank page");
+          iframe.contentWindow.location = JS_URL;
+          iframe.contentWindow.location = URL2;
+          break;
+        case 1:
+          assert_equals(iframe.contentWindow.document.URL,
+                        location.href.replace(location.pathname, URL2),
+                        "iframe url (" + count + ")");
+          assert_equals(iframe.contentDocument.body.textContent,
+                        JS_URL_TEXT, "text of js generated page");
+          iframe.contentWindow.location.reload();
+          break;
+        case 2:
+          assert_equals(iframe.contentWindow.document.URL,
+                          location.href.replace(location.pathname, URL2),
+                          "iframe url (" + count + ")");
+          assert_equals(iframe.contentDocument.body.textContent, "",
+                        "text of blank page");
+          t.done();
+          break;
+        }
+        count++;
+      });
+      iframe.src = URL;
+    });
+    </script>
+
+  </body>
+</html>