Bug 1422334. replaceState should update all the URI state for the entry being replaced. r=smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Wed, 14 Aug 2019 19:29:49 +0000
changeset 488007 d14199c9c1cc887fb8e67298e970429848a0d591
parent 488006 6e86ec2e3a21791238aa7f0108c0aabecc6a3e15
child 488008 7c287afc3556403098d8f26c382b119aa8d6c3ec
push id36434
push usercbrindusan@mozilla.com
push dateThu, 15 Aug 2019 09:44:30 +0000
treeherdermozilla-central@144fbfb409b7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1422334
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1422334. replaceState should update all the URI state for the entry being replaced. r=smaug If we don't update the resultPrincipalURI, then things that examine it (e.g. Location APIs and the URL bar) will show the wrong (pre-replaceState) URL. I believe there is no effective difference between setting the result principal URI to null and setting it to aNewURI here: the ultimate consumer of it is NS_GetFinalChannelURI, which will fall back to the originalURI if it's null, and in this case the originalURI is aNewURI. Differential Revision: https://phabricator.services.mozilla.com/D41788
docshell/base/nsDocShell.cpp
docshell/test/mochitest/bug1422334_redirect.html
docshell/test/mochitest/bug1422334_redirect.html^headers^
docshell/test/mochitest/mochitest.ini
docshell/test/mochitest/test_bug1422334.html
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -11344,16 +11344,20 @@ nsresult nsDocShell::UpdateURLAndHistory
           aDocument->NodePrincipal(),  // triggeringPrincipal
           nullptr, nullptr, aDocument->GetCsp(), true,
           getter_AddRefs(newSHEntry));
       NS_ENSURE_SUCCESS(rv, rv);
       mOSHE = newSHEntry;
     }
     newSHEntry->SetURI(aNewURI);
     newSHEntry->SetOriginalURI(aNewURI);
+    // Setting the resultPrincipalURI to nullptr is fine here: it will cause
+    // NS_GetFinalChannelURI to use the originalURI as the URI, which is aNewURI
+    // in our case.  We could also set it to aNewURI, with the same result.
+    newSHEntry->SetResultPrincipalURI(nullptr);
     newSHEntry->SetLoadReplace(false);
   }
 
   // Step 2.4 and 3: Modify new/original session history entry and clear its
   // POST data, if there is any.
   newSHEntry->SetStateData(aData);
   newSHEntry->SetPostData(nullptr);
 
new file mode 100644
--- /dev/null
+++ b/docshell/test/mochitest/bug1422334_redirect.html
@@ -0,0 +1,3 @@
+<html>
+  <body>You should never see this</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/docshell/test/mochitest/bug1422334_redirect.html^headers^
@@ -0,0 +1,2 @@
+HTTP 302 Moved Temporarily
+Location: ../navigation/blank.html?x=y
--- a/docshell/test/mochitest/mochitest.ini
+++ b/docshell/test/mochitest/mochitest.ini
@@ -50,16 +50,21 @@ support-files =
   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_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]
 [test_bug404548.html]
 [test_bug413310.html]
 skip-if = true
 # Disabled for too many intermittent failures (bug 719186)
new file mode 100644
--- /dev/null
+++ b/docshell/test/mochitest/test_bug1422334.html
@@ -0,0 +1,40 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Ensure that reload after replaceState after 3xx redirect does the right thing.</title>
+  <script src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" href="/tests/SimpleTest/test.css"/>
+  <script>
+    SimpleTest.waitForExplicitFinish();
+    addLoadEvent(function() {
+      var ifr = document.querySelector("iframe");
+      var win = ifr.contentWindow;
+      is(win.location.href,
+         location.href.replace("mochitest/test_bug1422334.html",
+                               "navigation/blank.html?x=y"),
+         "Should have the right location on initial load");
+
+      win.history.replaceState(null, '', win.location.pathname);
+      is(win.location.href,
+         location.href.replace("mochitest/test_bug1422334.html",
+                               "navigation/blank.html"),
+         "Should have the right location after replaceState call");
+
+      ifr.onload = function() {
+        is(win.location.href,
+           location.href.replace("mochitest/test_bug1422334.html",
+                                 "navigation/blank.html"),
+           "Should have the right location after reload");
+        SimpleTest.finish();
+      }
+      win.location.reload();
+    })
+  </script>
+</head>
+<body>
+<p id="display"><iframe src="bug1422334_redirect.html"></iframe></p>
+<div id="content" style="display: none"></div>
+<pre id="test"></pre>
+</body>
+</html>