Fix longstanding issue where anchor scrolls on a POST result page could lead to a silent re-POST during history traversal. Bug 413310, r+sr=biesi, a=dsicore,beltzner,righteousness.
authorbzbarsky@mit.edu
Thu, 28 Feb 2008 20:21:39 -0800
changeset 12420 66a61530eb92f90183c616b020cd9d31213e3965
parent 12419 71d337164ea4d28edd778078c9f1f097b99b6632
child 12421 0eab09a63e8c583959a1f74af1315813b9bbc31b
push idunknown
push userunknown
push dateunknown
reviewersdsicore, beltzner, righteousness
bugs413310
milestone1.9b4pre
Fix longstanding issue where anchor scrolls on a POST result page could lead to a silent re-POST during history traversal. Bug 413310, r+sr=biesi, a=dsicore,beltzner,righteousness.
docshell/base/nsDocShell.cpp
docshell/test/Makefile.in
docshell/test/bug413310-post.sjs
docshell/test/bug413310-subframe.html
docshell/test/test_bug413310.html
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -6875,43 +6875,50 @@ nsDocShell::InternalLoad(nsIURI * aURI,
 
             /* This is a anchor traversal with in the same page.
              * call OnNewURI() so that, this traversal will be 
              * recorded in session and global history.
              */
             OnNewURI(aURI, nsnull, mLoadType, PR_TRUE);
             nsCOMPtr<nsIInputStream> postData;
             PRUint32 pageIdent = PR_UINT32_MAX;
+            nsCOMPtr<nsISupports> cacheKey;
             
             if (mOSHE) {
                 /* save current position of scroller(s) (bug 59774) */
                 mOSHE->SetScrollPosition(cx, cy);
                 // Get the postdata and page ident from the current page, if
                 // the new load is being done via normal means.  Note that
                 // "normal means" can be checked for just by checking for
                 // LOAD_CMD_NORMAL, given the loadType and allowScroll check
                 // above -- it filters out some LOAD_CMD_NORMAL cases that we
                 // wouldn't want here.
                 if (aLoadType & LOAD_CMD_NORMAL) {
                     mOSHE->GetPostData(getter_AddRefs(postData));
                     mOSHE->GetPageIdentifier(&pageIdent);
+                    mOSHE->GetCacheKey(getter_AddRefs(cacheKey));
                 }
             }
             
             /* Assign mOSHE to mLSHE. This will either be a new entry created
              * by OnNewURI() for normal loads or aSHEntry for history loads.
              */
             if (mLSHE) {
                 SetHistoryEntry(&mOSHE, mLSHE);
                 // Save the postData obtained from the previous page
                 // in to the session history entry created for the 
                 // anchor page, so that any history load of the anchor
                 // page will restore the appropriate postData.
                 if (postData)
                     mOSHE->SetPostData(postData);
+
+                // Make sure we won't just repost without hitting the
+                // cache first
+                if (cacheKey)
+                    mOSHE->SetCacheKey(cacheKey);
                 
                 // Propagate our page ident to the new mOSHE so that
                 // we'll know it just differed by a scroll on the page.
                 if (pageIdent != PR_UINT32_MAX)
                     mOSHE->SetPageIdentifier(pageIdent);
             }
 
             /* restore previous position of scroller(s), if we're moving
--- a/docshell/test/Makefile.in
+++ b/docshell/test/Makefile.in
@@ -58,12 +58,15 @@ include $(topsrcdir)/config/rules.mk
 		bug94514-postpage.html \
 		test_bug344861.html \
 		test_bug369814.html \
 		bug369814.zip       \
 		test_bug384014.html \
 		test_bug387979.html \
 		test_bug404548.html \
 		bug404548-subframe.html \
+		test_bug413310.html \
+		bug413310-subframe.html \
+		bug413310-post.sjs \
 		$(NULL)
 
 libs:: $(_TEST_FILES)
 	$(INSTALL) $(foreach f,$^,"$f") $(DEPTH)/_tests/testing/mochitest/tests/$(relativesrcdir)
new file mode 100644
--- /dev/null
+++ b/docshell/test/bug413310-post.sjs
@@ -0,0 +1,7 @@
+function handleRequest(request, response) {
+  response.setHeader("Content-Type", "text/html");
+  response.write("<body onload='window.parent.onloadCount++'>" +
+                 request.method + " " +
+		 Date.now() +
+		 "</body>");
+}
new file mode 100644
--- /dev/null
+++ b/docshell/test/bug413310-subframe.html
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<html>
+  <body onload="window.parent.onloadCount++">
+    <form action="bug413310-post.sjs" method="POST">
+    </form>
+  </body>
+</html>
new file mode 100644
--- /dev/null
+++ b/docshell/test/test_bug413310.html
@@ -0,0 +1,98 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=413310
+-->
+<head>
+  <title>Test for Bug 413310</title>
+  <script type="text/javascript" src="/MochiKit/MochiKit.js"></script>
+  <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=413310">Mozilla Bug 413310</a>
+<p id="display">
+<iframe id="i" src="bug413310-subframe.html" onload="setTimeout(doNextStep, 20)">
+</iframe>
+</p>
+<div id="content" style="display: none">
+  
+</div>
+<pre id="test">
+<script class="testbody" type="text/javascript">
+
+/** Test for Bug 413310 **/
+
+// NOTE: If we ever make subframes do bfcache stuff, this test will need to be
+// modified accordingly!  It assumes that subframes do not get bfcached.
+var onloadCount = 0;
+
+var step = -1;  // One increment will come from the initial subframe onload.
+
+var textContent;
+
+SimpleTest.waitForExplicitFinish();
+
+addLoadEvent(doNextStep);
+
+function doNextStep() {
+  ++step;
+  switch (step) {
+    case 1:
+      is(onloadCount, 1, "Loaded initial page");
+      is($("i").contentWindow.location.href,
+         location.href.replace(/test_bug413310.html/,
+                               "bug413310-subframe.html"),
+         "Unexpected subframe location after initial load");
+      $("i").contentDocument.forms[0].submit();
+      break;
+    case 2:
+      is(onloadCount, 2, "Loaded POST result");
+
+      is($("i").contentWindow.location.href,
+         location.href.replace(/test_bug413310.html/,
+                               "bug413310-post.sjs"),
+         "Unexpected subframe location after POST load");
+
+      textContent = $("i").contentDocument.body.textContent;
+      isDeeply(textContent.match(/^POST /), ["POST "], "Not a POST?");
+
+      $("i").contentWindow.location.hash = "foo";
+      setTimeout(doNextStep, 0);
+      break;
+    case 3: 
+      is(onloadCount, 2, "Anchor scroll should not fire onload");
+      is($("i").contentWindow.location.href,
+         location.href.replace(/test_bug413310.html/,
+                               "bug413310-post.sjs#foo"),
+         "Unexpected subframe location after anchor scroll");
+      is(textContent, $("i").contentDocument.body.textContent,
+         "Did a load when scrolling?");
+      $("i").contentWindow.location.href = "bug413310-subframe.html";;
+      break;
+    case 4:
+      is(onloadCount, 3, "Done new load");
+      is($("i").contentWindow.location.href,
+         location.href.replace(/test_bug413310.html/,
+                               "bug413310-subframe.html"),
+         "Unexpected subframe location after new load");
+      history.back();
+      break;
+    case 5:
+      is(onloadCount, 4,
+         "History traversal didn't fire onload: bfcache issues!");
+      is($("i").contentWindow.location.href,
+         location.href.replace(/test_bug413310.html/,
+                               "bug413310-post.sjs#foo"),
+         "Unexpected subframe location");
+      is(textContent, $("i").contentDocument.body.textContent,
+         "Did a load when going back?");
+      SimpleTest.finish();
+      break;
+  }
+}
+</script>
+</pre>
+</body>
+</html>
+