Bug 836567 - Part 3: Replace inapplicable tests with a web-platform-test for reloading after setting javascript: URI, and fix other tests relying on javascript: URI. r?bz draft
authorSamael Wang <freesamael@gmail.com>
Wed, 24 May 2017 18:30:46 +0800
changeset 704051 183877c9e68798c3143bbd5bb3e1b48cc83d4de7
parent 704050 284f75f1cb0b2fd41403f78af57ed1649034aee1
child 741977 c36defe4b096d218e6b41f3045751f9506dfb984
push id91050
push userbmo:sawang@mozilla.com
push dateTue, 28 Nov 2017 06:08:27 +0000
reviewersbz
bugs836567, 384014, 123696, 1262766
milestone58.0a1
Bug 836567 - Part 3: Replace inapplicable tests with a web-platform-test for reloading after setting javascript: URI, and fix other tests relying on javascript: URI. r?bz 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 a 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 throw a dom security exception as it's null principal. 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. MozReview-Commit-ID: JfqqJHBpApq
browser/base/content/test/urlbar/browser_urlbarCopying.js
browser/base/content/test/urlbar/browser_urlbar_blanking.js
devtools/client/storage/test/browser_storage_cache_error.js
devtools/client/storage/test/head.js
devtools/client/storage/test/storage-cache-error.html
docshell/test/mochitest.ini
docshell/test/test_bug123696.html
docshell/test/test_bug384014.html
testing/web-platform/meta/MANIFEST.json
testing/web-platform/meta/html/semantics/embedded-content/the-iframe-element/iframe_javascript_url_01.htm.ini
testing/web-platform/tests/html/browsers/history/the-location-interface/location_reload_javascript_url.html
--- a/browser/base/content/test/urlbar/browser_urlbarCopying.js
+++ b/browser/base/content/test/urlbar/browser_urlbarCopying.js
@@ -165,27 +165,25 @@ var tests = [
     copyExpected: "http://example.com/?\xf7"
   },
   {
     loadURL: "http://example.com/%20%20%20",
     expectedURL: "example.com/   ",
     copyExpected: "http://example.com/%20%20%20"
   },
 
-  // data: and javsacript: URIs shouldn't be encoded
+  // Loading of javascript: URI results in previous URI.
   {
     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/%20%20%20",
+    expectedURL: "example.com/   ",
+    copyExpected: "http://example.com/%20%20%20"
   },
 
+  // data: URIs shouldn't be encoded
   {
     loadURL: "data:text/html,(%C3%A9%20%25%50)",
     expectedURL: "data:text/html,(%C3%A9 %25P)",
     copyExpected: "data:text/html,(%C3%A9 %25P)",
   },
   {
     copyVal: "<data:text/html,(>%C3%A9 %25P)",
     copyExpected: "data:text/html,("
@@ -225,17 +223,19 @@ function runTest(testCase, cb) {
   }
 
   if (testCase.setup) {
     testCase.setup();
   }
 
   if (testCase.loadURL) {
     info(`Loading : ${testCase.loadURL}\n`);
-    loadURL(testCase.loadURL, doCheck);
+    let expectedLoad = testCase.expectedLoad ?
+      testCase.expectedLoad : testCase.loadURL;
+    loadURL(testCase.loadURL, expectedLoad, doCheck);
   } else {
     if (testCase.setURL)
       gURLBar.value = testCase.setURL;
     doCheck();
   }
 }
 
 function testCopy(copyVal, targetValue, cb) {
@@ -278,12 +278,12 @@ function testCopy(copyVal, targetValue, 
     } else {
       gURLBar.select();
     }
 
     goDoCommand("cmd_copy");
   }, cb, 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);
 }
--- a/browser/base/content/test/urlbar/browser_urlbar_blanking.js
+++ b/browser/base/content/test/urlbar/browser_urlbar_blanking.js
@@ -8,28 +8,31 @@ add_task(async function() {
     }
     let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, page);
     ok(!gURLBar.value, "The URL bar should be empty if we load a plain " + page + " page.");
     await BrowserTestUtils.removeTab(tab);
   }
 });
 
 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.
   const URI = "http://www.example.com/browser/browser/base/content/test/urlbar/file_blank_but_not_blank.html";
   let tab = await BrowserTestUtils.openNewForegroundTab(gBrowser, URI);
   is(gURLBar.value, URI, "The URL bar should match the URI");
   let browserLoaded = BrowserTestUtils.browserLoaded(tab.linkedBrowser);
   ContentTask.spawn(tab.linkedBrowser, null, function() {
     content.document.querySelector("a").click();
   });
   await browserLoaded;
-  ok(gURLBar.value.startsWith("javascript"), "The URL bar should have the JS 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:
-  SimpleTest.expectUncaughtException(true);
+  is(gURLBar.value, URI, "The URL bar should be the previous active document's URI.");
   await ContentTask.spawn(tab.linkedBrowser, null, async function() {
     // This is sync, so by the time we return we should have changed the URL bar.
     content.location.reload();
   });
-  ok(!!gURLBar.value, "URL bar should not be blank.");
+  is(gURLBar.value, URI, "The URL bar should still be the previous active document's URI.");
   await BrowserTestUtils.removeTab(tab);
   SimpleTest.expectUncaughtException(false);
 });
--- a/devtools/client/storage/test/browser_storage_cache_error.js
+++ b/devtools/client/storage/test/browser_storage_cache_error.js
@@ -2,18 +2,28 @@
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
 // Test handling errors in CacheStorage
 
 add_task(function* () {
-  yield openTabAndSetupStorage(MAIN_DOMAIN + "storage-cache-error.html");
+  // Open the URL in a private browsing window.
+  let win = yield BrowserTestUtils.openNewBrowserWindow({ private: true });
+  let tab = win.gBrowser.selectedBrowser;
+  tab.loadURI(MAIN_DOMAIN + "storage-cache-error.html");
+  yield 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.
+  yield openStoragePanel(null, win.gBrowser);
+
+  const cacheItemId = ["Cache", "http://test2.example.org"];
 
   yield selectTreeItem(cacheItemId);
   ok(gUI.tree.isSelected(cacheItemId),
     `The item ${cacheItemId.join(" > ")} is present in the tree`);
 
+  yield BrowserTestUtils.closeWindow(win);
   yield finishTests();
 });
--- a/devtools/client/storage/test/head.js
+++ b/devtools/client/storage/test/head.js
@@ -130,22 +130,23 @@ function* openTabAndSetupStorage(url, op
   return yield openStoragePanel();
 }
 
 /**
  * Open the toolbox, with the storage tool visible.
  *
  * @param cb {Function} Optional callback, if you don't want to use the returned
  *                      promise
+ * @param tabbrowser {Object} tabbrowser to use. Default to gBrowser.
  *
  * @return {Promise} a promise that resolves when the storage inspector is ready
  */
-var openStoragePanel = Task.async(function* (cb) {
+var openStoragePanel = Task.async(function* (cb, tabbrowser = gBrowser) {
   info("Opening the storage inspector");
-  let target = TargetFactory.forTab(gBrowser.selectedTab);
+  let target = TargetFactory.forTab(tabbrowser.selectedTab);
 
   let storage, toolbox;
 
   // Checking if the toolbox and the storage are already loaded
   // The storage-updated event should only be waited for if the storage
   // isn't loaded yet
   toolbox = gDevTools.getToolbox(target);
   if (toolbox) {
--- a/devtools/client/storage/test/storage-cache-error.html
+++ b/devtools/client/storage/test/storage-cache-error.html
@@ -1,20 +1,11 @@
 <!DOCTYPE HTML>
 <html>
 <head>
   <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.
-let 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>
--- a/docshell/test/mochitest.ini
+++ b/docshell/test/mochitest.ini
@@ -49,19 +49,17 @@ support-files =
   file_pushState_after_document_open.html
   historyframes.html
   start_historyframe.html
   url1_historyframe.html
   url2_historyframe.html
 
 [test_anchor_scroll_after_document_open.html]
 [test_bfcache_plus_hash.html]
-[test_bug123696.html]
 [test_bug369814.html]
-[test_bug384014.html]
 [test_bug385434.html]
 [test_bug387979.html]
 [test_bug402210.html]
 [test_bug404548.html]
 [test_bug413310.html]
 skip-if = true
 # Disabled for too many intermittent failures (bug 719186)
 [test_bug475636.html]
deleted file mode 100644
--- a/docshell/test/test_bug123696.html
+++ /dev/null
@@ -1,46 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<!--
-https://bugzilla.mozilla.org/show_bug.cgi?id=123696
--->
-<head>
-  <title>Test for Bug 123696</title>
-  <script type="application/javascript" 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>
deleted file mode 100644
--- a/docshell/test/test_bug384014.html
+++ /dev/null
@@ -1,41 +0,0 @@
-<!DOCTYPE HTML>
-<html>
-<!--
-https://bugzilla.mozilla.org/show_bug.cgi?id=384014
--->
-<head>
-  <title>Test for Bug 384014</title>
-  <script type="text/javascript" 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>
-
--- a/testing/web-platform/meta/MANIFEST.json
+++ b/testing/web-platform/meta/MANIFEST.json
@@ -319161,16 +319161,22 @@
     ]
    ],
    "html/browsers/history/the-location-interface/location_reload.html": [
     [
      "/html/browsers/history/the-location-interface/location_reload.html",
      {}
     ]
    ],
+   "html/browsers/history/the-location-interface/location_reload_javascript_url.html": [
+    [
+     "/html/browsers/history/the-location-interface/location_reload_javascript_url.html",
+     {}
+    ]
+   ],
    "html/browsers/history/the-location-interface/location_replace.html": [
     [
      "/html/browsers/history/the-location-interface/location_replace.html",
      {}
     ]
    ],
    "html/browsers/history/the-location-interface/location_search.html": [
     [
@@ -541454,16 +541460,20 @@
   "html/browsers/history/the-location-interface/location_reload-iframe.html": [
    "ba74c65e3fd2eb7c1320cadb43b53b928b1c7e46",
    "support"
   ],
   "html/browsers/history/the-location-interface/location_reload.html": [
    "502a912cd4539ae90703cb3ad687be49c574ae04",
    "testharness"
   ],
+  "html/browsers/history/the-location-interface/location_reload_javascript_url.html": [
+   "d76b98e068d8e459b751df0832a3de3d5bd61b79",
+   "testharness"
+  ],
   "html/browsers/history/the-location-interface/location_replace.html": [
    "017ad5ff9ece1de23ef5b48a2d18192eccf27e2c",
    "testharness"
   ],
   "html/browsers/history/the-location-interface/location_search.html": [
    "48192ef9970a0e258e790e69174755c3872604f1",
    "testharness"
   ],
deleted file mode 100644
--- a/testing/web-platform/meta/html/semantics/embedded-content/the-iframe-element/iframe_javascript_url_01.htm.ini
+++ /dev/null
@@ -1,5 +0,0 @@
-[iframe_javascript_url_01.htm]
-  type: testharness
-  [javascript: URL creating a document in an about:blank iframe]
-    expected: FAIL
-
new file mode 100644
--- /dev/null
+++ b/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>