Bug 629559. Clone child session history entries on anchor navigation. r=dtownsend,smaug

--HG--
rename : dom/tests/mochitest/general/historyframes.html => docshell/test/historyframes.html
rename : dom/tests/mochitest/general/test_framedhistoryframes.html => docshell/test/test_framedhistoryframes.html
rename : dom/tests/mochitest/general/test_windowedhistoryframes.html => docshell/test/test_windowedhistoryframes.html

docshell/base/nsDocShell.cpp

@@ -3323,7 +3323,8 @@ nsDocShell::GetChildSHEntry(PRInt32 aChildOffset, nsISHEntry ** aResult)
 
 NS_IMETHODIMP
 nsDocShell::AddChildSHEntry(nsISHEntry * aCloneRef, nsISHEntry * aNewEntry,
-                            PRInt32 aChildOffset, PRUint32 loadType)
+                            PRInt32 aChildOffset, PRUint32 loadType,
+                            PRBool aCloneChildren)
 {
     nsresult rv;
 
@@ -3367,8 +3368,7 @@ nsDocShell::AddChildSHEntry(nsISHEntry * aCloneRef, nsISHEntry * aNewEntry,
             nsCOMPtr<nsISHEntry> nextEntry;
             aCloneRef->GetID(&cloneID);
             rv = CloneAndReplace(currentEntry, this, cloneID, aNewEntry,
-                                 loadType == LOAD_PUSHSTATE,
-                                 getter_AddRefs(nextEntry));
+                                 aCloneChildren, getter_AddRefs(nextEntry));
 
             if (NS_SUCCEEDED(rv)) {
                 nsCOMPtr<nsISHistoryInternal>
@@ -3384,14 +3384,15 @@ nsDocShell::AddChildSHEntry(nsISHEntry * aCloneRef, nsISHEntry * aNewEntry,
             do_QueryInterface(GetAsSupports(mParent), &rv);
         if (parent) {
             rv = parent->AddChildSHEntry(aCloneRef, aNewEntry, aChildOffset,
-                                         loadType);
+                                         loadType, aCloneChildren);
         }          
     }
     return rv;
 }
 
 nsresult
-nsDocShell::DoAddChildSHEntry(nsISHEntry* aNewEntry, PRInt32 aChildOffset)
+nsDocShell::DoAddChildSHEntry(nsISHEntry* aNewEntry, PRInt32 aChildOffset,
+                              PRBool aCloneChildren)
 {
     /* You will get here when you are in a subframe and
      * a new url has been loaded on you. 
@@ -3412,7 +3413,8 @@ nsDocShell::DoAddChildSHEntry(nsISHEntry* aNewEntry, PRInt32 aChildOffset)
     nsCOMPtr<nsIDocShellHistory> parent =
         do_QueryInterface(GetAsSupports(mParent), &rv);
     if (parent) {
-        rv = parent->AddChildSHEntry(mOSHE, aNewEntry, aChildOffset, mLoadType);
+        rv = parent->AddChildSHEntry(mOSHE, aNewEntry, aChildOffset, mLoadType,
+                                     aCloneChildren);
     }
 
 
@@ -5885,7 +5887,9 @@ nsDocShell::OnStateChange(nsIWebProgress * aProgress, nsIRequest * aRequest,
 
                 // This is a document.write(). Get the made-up url
                 // from the channel and store it in session history.
-                rv = AddToSessionHistory(uri, wcwgChannel, nsnull,
+                // Pass false for aCloneChildren, since we're creating
+                // a new DOM here.
+                rv = AddToSessionHistory(uri, wcwgChannel, nsnull, PR_FALSE,
                                          getter_AddRefs(mLSHE));
                 SetCurrentURI(uri, aRequest, PR_TRUE);
                 // Save history state of the previous page
@@ -7432,7 +7436,8 @@ nsDocShell::CreateContentViewer(const char *aContentType,
             OnLoadingSite(failedChannel, PR_TRUE, PR_FALSE);
         } else if (failedURI) {
             mURIResultedInDocument = PR_TRUE;
-            OnNewURI(failedURI, nsnull, nsnull, mLoadType, PR_TRUE, PR_FALSE);
+            OnNewURI(failedURI, nsnull, nsnull, mLoadType, PR_TRUE, PR_FALSE,
+                     PR_FALSE);
         }
 
         // Be sure to have a correct mLSHE, it may have been cleared by
@@ -8325,7 +8330,10 @@ nsDocShell::InternalLoad(nsIURI * aURI,
             if (mOSHE) {
                 mOSHE->GetOwner(getter_AddRefs(owner));
             }
-            OnNewURI(aURI, nsnull, owner, mLoadType, PR_TRUE, PR_TRUE);
+            // Pass true for aCloneSHChildren, since we're not
+            // changing documents here, so all of our subframes are
+            // still relevant to the new session history entry.
+            OnNewURI(aURI, nsnull, owner, mLoadType, PR_TRUE, PR_TRUE, PR_TRUE);
 
             nsCOMPtr<nsIInputStream> postData;
             PRUint32 pageIdent = PR_UINT32_MAX;
@@ -9295,7 +9303,7 @@ nsDocShell::SetupReferrerFromChannel(nsIChannel * aChannel)
 PRBool
 nsDocShell::OnNewURI(nsIURI * aURI, nsIChannel * aChannel, nsISupports* aOwner,
                      PRUint32 aLoadType, PRBool aFireOnLocationChange,
-                     PRBool aAddToGlobalHistory)
+                     PRBool aAddToGlobalHistory, PRBool aCloneSHChildren)
 {
     NS_PRECONDITION(aURI, "uri is null");
     NS_PRECONDITION(!aChannel || !aOwner, "Shouldn't have both set");
@@ -9466,7 +9474,7 @@ nsDocShell::OnNewURI(nsIURI * aURI, nsIChannel * aChannel, nsISupports* aOwner,
              *.Create a Entry for it and add it to SH, if this is the
              * rootDocShell
              */
-            (void) AddToSessionHistory(aURI, aChannel, aOwner,
+            (void) AddToSessionHistory(aURI, aChannel, aOwner, aCloneSHChildren,
                                        getter_AddRefs(mLSHE));
         }
 
@@ -9522,8 +9530,9 @@ nsDocShell::OnLoadingSite(nsIChannel * aChannel, PRBool aFireOnLocationChange,
     NS_GetFinalChannelURI(aChannel, getter_AddRefs(uri));
     NS_ENSURE_TRUE(uri, PR_FALSE);
 
+    // Pass false for aCloneSHChildren, since we're loading a new page here.
     return OnNewURI(uri, aChannel, nsnull, mLoadType, aFireOnLocationChange,
-                    aAddToGlobalHistory);
+                    aAddToGlobalHistory, PR_FALSE);
 
 }
 
@@ -9781,7 +9790,9 @@ nsDocShell::AddState(nsIVariant *aData, const nsAString& aTitle,
         GetCurScrollPos(ScrollOrientation_Y, &cy);
         mOSHE->SetScrollPosition(cx, cy);
 
-        rv = AddToSessionHistory(newURI, nsnull, nsnull,
+        // Since we're not changing which page we have loaded, pass
+        // true for aCloneChildren.
+        rv = AddToSessionHistory(newURI, nsnull, nsnull, PR_TRUE,
                                  getter_AddRefs(newSHEntry));
         NS_ENSURE_SUCCESS(rv, rv);
 
@@ -9876,7 +9887,8 @@ nsDocShell::ShouldAddToSessionHistory(nsIURI * aURI)
 
 nsresult
 nsDocShell::AddToSessionHistory(nsIURI * aURI, nsIChannel * aChannel,
-                                nsISupports* aOwner, nsISHEntry ** aNewEntry)
+                                nsISupports* aOwner, PRBool aCloneChildren,
+                                nsISHEntry ** aNewEntry)
 {
     NS_PRECONDITION(aURI, "uri is null");
     NS_PRECONDITION(!aChannel || !aOwner, "Shouldn't have both set");
@@ -10004,9 +10016,9 @@ nsDocShell::AddToSessionHistory(nsIURI * aURI, nsIChannel * aChannel,
 
 
     if (root == static_cast<nsIDocShellTreeItem *>(this) && mSessionHistory) {
-        // Bug 629559: Detect if this is an anchor navigation and clone the
-        // session history in that case too
-        if (mLoadType == LOAD_PUSHSTATE && mOSHE) {
+        // If we need to clone our children onto the new session
+        // history entry, do so now.
+        if (aCloneChildren && mOSHE) {
             PRUint32 cloneID;
             mOSHE->GetID(&cloneID);
             nsCOMPtr<nsISHEntry> newEntry;
@@ -10042,7 +10054,7 @@ nsDocShell::AddToSessionHistory(nsIURI * aURI, nsIChannel * aChannel,
         // This is a subframe.
         if (!mOSHE || !LOAD_TYPE_HAS_FLAGS(mLoadType,
                                            LOAD_FLAGS_REPLACE_HISTORY))
-            rv = DoAddChildSHEntry(entry, mChildOffset);
+            rv = DoAddChildSHEntry(entry, mChildOffset, aCloneChildren);
     }
 
     // Return the new SH entry...

docshell/base/nsDocShell.h

@@ -356,10 +356,13 @@ protected:
     // In all other cases PR_FALSE is returned.
     // Either aChannel or aOwner must be null.  If aChannel is
     // present, the owner should be gotten from it.
+    // If OnNewURI calls AddToSessionHistory, it will pass its
+    // aCloneSHChildren argument as aCloneChildren.
     PRBool OnNewURI(nsIURI * aURI, nsIChannel * aChannel, nsISupports* aOwner,
                     PRUint32 aLoadType,
                     PRBool aFireOnLocationChange,
-                    PRBool aAddToGlobalHistory = PR_TRUE);
+                    PRBool aAddToGlobalHistory,
+                    PRBool aCloneSHChildren);
 
     virtual void SetReferrerURI(nsIURI * aURI);
 
@@ -367,10 +370,16 @@ protected:
     virtual PRBool ShouldAddToSessionHistory(nsIURI * aURI);
     // Either aChannel or aOwner must be null.  If aChannel is
     // present, the owner should be gotten from it.
+    // If aCloneChildren is true, then our current session history's
+    // children will be cloned onto the new entry.  This should be
+    // used when we aren't actually changing the document while adding
+    // the new session history entry.
     virtual nsresult AddToSessionHistory(nsIURI * aURI, nsIChannel * aChannel,
                                          nsISupports* aOwner,
+                                         PRBool aCloneChildren,
                                          nsISHEntry ** aNewEntry);
-    nsresult DoAddChildSHEntry(nsISHEntry* aNewEntry, PRInt32 aChildOffset);
+    nsresult DoAddChildSHEntry(nsISHEntry* aNewEntry, PRInt32 aChildOffset,
+                               PRBool aCloneChildren);
 
     NS_IMETHOD LoadHistoryEntry(nsISHEntry * aEntry, PRUint32 aLoadType);
     NS_IMETHOD PersistLayoutHistoryState();

docshell/base/nsIDocShellHistory.idl

@@ -39,7 +39,7 @@
 #include "nsISupports.idl"
 interface nsISHEntry;
 
-[scriptable, uuid(95e425aa-afc6-40a0-9db4-7f210a58310a)]
+[scriptable, uuid(077af5fd-7450-48db-8f03-16617d441141)]
 interface nsIDocShellHistory : nsISupports
 {
   /**
@@ -49,11 +49,14 @@ interface nsIDocShellHistory : nsISupports
 
   /**
    * Add a Child SHEntry for a frameset page, given the child's loadtype.
+   * If aCloneChildren is true, then aCloneReference's children will be
+   * cloned onto aHistoryEntry.
    */
   void addChildSHEntry(in nsISHEntry aCloneReference,
                        in nsISHEntry aHistoryEntry,
                        in long aChildOffset,
-                       in unsigned long aLoadType);
+                       in unsigned long aLoadType,
+                       in boolean aCloneChilden);
 
   /**
    * Whether this docshell should save entries in global history.

docshell/test/Makefile.in

@@ -97,6 +97,9 @@ _TEST_FILES = \
 		test_bug634834.html \
 		file_bug634834.html \
 		test_bug637644.html \
+		test_framedhistoryframes.html \
+		test_windowedhistoryframes.html \
+		historyframes.html \
 		$(NULL)
 
 ifeq ($(MOZ_WIDGET_TOOLKIT),cocoa)

docshell/test/historyframes.html

@@ -0,0 +1,150 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=602256
+-->
+<head>
+  <title>Test for Bug 602256</title>
+</head>
+<body onload="SimpleTest.executeSoon(run_test)">
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=602256">Mozilla Bug 602256</a>
+<div id="content">
+  <iframe id="iframe" src="data:text/html,<p%20id='text'>Start</p>"></iframe>
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 602256 **/
+
+var testWin = window.opener ? window.opener : window.parent;
+
+var SimpleTest = testWin.SimpleTest;
+function is() { testWin.is.apply(testWin, arguments); }
+
+var gFrame = null;
+
+function gState() {
+  return location.hash.replace(/^#/, "");
+}
+
+function waitForLoad(aCallback) {
+  function listener() {
+    gFrame.removeEventListener("load", listener, false);
+    SimpleTest.executeSoon(aCallback);
+  }
+
+  gFrame.addEventListener("load", listener, false);
+}
+
+function loadContent(aURL, aCallback) {
+  waitForLoad(aCallback);
+
+  gFrame.src = aURL;
+}
+
+function getURL() {
+  return gFrame.contentDocument.documentURI;
+}
+
+function getContent() {
+  return gFrame.contentDocument.getElementById("text").textContent;
+}
+
+var START = "data:text/html,<p%20id='text'>Start</p>";
+var URL1 = "data:text/html,<p%20id='text'>Test1</p>";
+var URL2 = "data:text/html,<p%20id='text'>Test2</p>";
+
+function run_test() {
+  window.location.hash = "START";
+
+  gFrame = document.getElementById("iframe");
+
+  test_basic_inner_navigation();
+}
+
+function end_test() {
+  testWin.done();
+}
+
+function test_basic_inner_navigation() {
+  // Navigate the inner frame a few times
+  loadContent(URL1, function() {
+    is(getURL(), URL1, "URL should be correct");
+    is(getContent(), "Test1", "Page should be correct");
+
+    loadContent(URL2, function() {
+      is(getURL(), URL2, "URL should be correct");
+      is(getContent(), "Test2", "Page should be correct");
+
+      // Test that history is working
+      waitForLoad(function() {
+        is(getURL(), URL1, "URL should be correct");
+        is(getContent(), "Test1", "Page should be correct");
+
+        waitForLoad(function() {
+          is(getURL(), URL2, "URL should be correct");
+          is(getContent(), "Test2", "Page should be correct");
+
+          test_state_navigation();
+        });
+        window.history.forward();
+      });
+      window.history.back();
+    });
+  });
+}
+
+function test_state_navigation() {
+  window.location.hash = "STATE1";
+
+  is(getURL(), URL2, "URL should be correct");
+  is(getContent(), "Test2", "Page should be correct");
+
+  window.location.hash = "STATE2";
+
+  is(getURL(), URL2, "URL should be correct");
+  is(getContent(), "Test2", "Page should be correct");
+
+  window.history.back();
+
+  is(gState(), "STATE1", "State should be correct after going back");
+  is(getURL(), URL2, "URL should be correct");
+  is(getContent(), "Test2", "Page should be correct");
+
+  window.history.forward();
+
+  is(gState(), "STATE2", "State should be correct after going forward");
+  is(getURL(), URL2, "URL should be correct");
+  is(getContent(), "Test2", "Page should be correct");
+
+  window.history.back();
+  window.history.back();
+
+  is(gState(), "START", "State should be correct");
+  is(getURL(), URL2, "URL should be correct");
+  is(getContent(), "Test2", "Page should be correct");
+
+  waitForLoad(function() {
+    is(getURL(), URL1, "URL should be correct");
+    is(getContent(), "Test1", "Page should be correct");
+
+    waitForLoad(function() {
+      is(gState(), "START", "State should be correct");
+      is(getURL(), START, "URL should be correct");
+      is(getContent(), "Start", "Page should be correct");
+
+      end_test();
+    });
+
+    window.history.back();
+
+    is(gState(), "START", "State should be correct after going back twice");
+  });
+
+  window.history.back();
+  is(gState(), "START", "State should be correct");
+}
+</script>
+</pre>
+</body>
+</html>

docshell/test/test_framedhistoryframes.html

@@ -0,0 +1,32 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=602256
+-->
+<head>
+  <title>Test for Bug 602256</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <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=602256">Mozilla Bug 602256</a>
+<p id="display"></p>
+<div id="content">
+  <iframe id="iframe" src="historyframes.html"></iframe>
+</div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 602256 **/
+
+SimpleTest.waitForExplicitFinish();
+
+function done() {
+  SimpleTest.finish();
+}
+
+</script>
+</pre>
+</body>
+</html>

docshell/test/test_windowedhistoryframes.html

@@ -0,0 +1,33 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=602256
+-->
+<head>
+  <title>Test for Bug 602256</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <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=602256">Mozilla Bug 602256</a>
+<p id="display"></p>
+<div id="content" style="display: none"></div>
+<pre id="test">
+<script type="application/javascript">
+
+/** Test for Bug 602256 **/
+
+SimpleTest.waitForExplicitFinish();
+
+function done() {
+  subWin.close();
+  SimpleTest.finish();
+}
+
+var subWin = window.open("historyframes.html", "_blank");
+
+</script>
+</pre>
+</body>
+</html>