Bug 680344: Properly fix up a pending history navigation in nsSHistory::RemoveDuplicate. r=smaug sr=bz
authorKyle Huey <khuey@kylehuey.com>
Wed, 24 Aug 2011 08:48:17 -0400
changeset 75800 4599be47c692263897ab82272469fb7045e2b70c
parent 75799 e0acef471ab29856f4c7883dc2d1bb31afdfff77
child 75801 ac653960b153fc8f96be4277e5edaae88461b123
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewerssmaug, bz
bugs680344
milestone9.0a1
Bug 680344: Properly fix up a pending history navigation in nsSHistory::RemoveDuplicate. r=smaug sr=bz
docshell/shistory/src/nsSHistory.cpp
--- a/docshell/shistory/src/nsSHistory.cpp
+++ b/docshell/shistory/src/nsSHistory.cpp
@@ -1304,16 +1304,18 @@ PRBool IsSameTree(nsISHEntry* aEntry1, n
   
   return PR_TRUE;
 }
 
 PRBool
 nsSHistory::RemoveDuplicate(PRInt32 aIndex, PRBool aKeepNext)
 {
   NS_ASSERTION(aIndex >= 0, "aIndex must be >= 0!");
+  NS_ASSERTION(aIndex != 0 || aKeepNext,
+               "If we're removing index 0 we must be keeping the next");
   NS_ASSERTION(aIndex != mIndex, "Shouldn't remove mIndex!");
   PRInt32 compareIndex = aKeepNext ? aIndex + 1 : aIndex - 1;
   nsCOMPtr<nsIHistoryEntry> rootHE1, rootHE2;
   GetEntryAtIndex(aIndex, PR_FALSE, getter_AddRefs(rootHE1));
   GetEntryAtIndex(compareIndex, PR_FALSE, getter_AddRefs(rootHE2));
   nsCOMPtr<nsISHEntry> root1 = do_QueryInterface(rootHE1);
   nsCOMPtr<nsISHEntry> root2 = do_QueryInterface(rootHE2);
   if (IsSameTree(root1, root2)) {
@@ -1340,20 +1342,35 @@ nsSHistory::RemoveDuplicate(PRInt32 aInd
       NS_ASSERTION(txToRemove == mListRoot,
                    "Transaction at index 0 should be mListRoot!");
       // We're removing the very first session history transaction!
       mListRoot = txToKeep;
     }
     if (mRootDocShell) {
       static_cast<nsDocShell*>(mRootDocShell)->HistoryTransactionRemoved(aIndex);
     }
+
+    // Adjust our indices to reflect the removed transaction
     if (mIndex > aIndex) {
       mIndex = mIndex - 1;
     }
-    if (mRequestedIndex > aIndex) {
+
+    // NB: If the transaction we are removing is the transaction currently
+    // being navigated to (mRequestedIndex) then we adjust the index
+    // only if we're not keeping the next entry (because if we are keeping
+    // the next entry (because the current is a duplicate of the next), then
+    // that entry slides into the spot that we're currently pointing to.
+    // We don't do this adjustment for mIndex because mIndex cannot equal
+    // aIndex.
+
+    // NB: We don't need to guard on mRequestedIndex being nonzero here,
+    // because either they're strictly greater than aIndex which is at least
+    // zero, or they are equal to aIndex in which case aKeepNext must be true
+    // if aIndex is zero.
+    if (mRequestedIndex > aIndex || (mRequestedIndex == aIndex && !aKeepNext)) {
       mRequestedIndex = mRequestedIndex - 1;
     }
     --mLength;
     return PR_TRUE;
   }
   return PR_FALSE;
 }