Bug 317189. Do the second-scroll-to-anchor on onload (i.e. subresources have loaded), not just when the document has finished loading. Also, don't do it at all if the user has scrolled manually between the first anchor scroll and the second. r+sr=dbaron

content/base/src/nsContentSink.cpp

@@ -293,7 +293,7 @@ nsContentSink::StyleSheetLoaded(nsICSSStyleSheet* aSheet,
       }
 
       // Go ahead and try to scroll to our ref if we have one
-      TryToScrollToRef();
+      ScrollToRef();
     }
     
     mScriptLoader->RemoveExecuteBlocker();
@@ -885,7 +885,9 @@ nsContentSink::ScrollToRef()
     return;
   }
 
-  PRBool didScroll = PR_FALSE;
+  if (mScrolledToRefAlready) {
+    return;
+  }
 
   char* tmpstr = ToNewCString(mRef);
   if (!tmpstr) {
@@ -1039,20 +1041,6 @@ nsContentSink::StartLayout(PRBool aIgnorePendingSheets)
   }
 }
 
-void
-nsContentSink::TryToScrollToRef()
-{
-  if (mRef.IsEmpty()) {
-    return;
-  }
-
-  if (mScrolledToRefAlready) {
-    return;
-  }
-
-  ScrollToRef();
-}
-
 void
 nsContentSink::NotifyAppend(nsIContent* aContainer, PRUint32 aStartIndex)
 {
@@ -1120,7 +1108,7 @@ nsContentSink::Notify(nsITimer *timer)
 
     // Now try and scroll to the reference
     // XXX Should we scroll unconditionally for history loads??
-    TryToScrollToRef();
+    ScrollToRef();
   }
 
   mNotificationTimer = nsnull;
@@ -1180,7 +1168,7 @@ nsContentSink::WillInterruptImpl()
                     "run out time; backoff count: %d", mBackoffCount));
         result = FlushTags();
         if (mDroppedTimer) {
-          TryToScrollToRef();
+          ScrollToRef();
           mDroppedTimer = PR_FALSE;
         }
       } else if (!mNotificationTimer) {

content/base/src/nsContentSink.h

@@ -173,6 +173,8 @@ protected:
                     PRBool aExplicit);
   void ProcessOfflineManifest(nsIContent *aElement);
 
+  // Tries to scroll to the URI's named anchor. Once we've successfully
+  // done that, further calls to this method will be ignored.
   void ScrollToRef();
   nsresult RefreshIfEnabled(nsIViewManager* vm);
 
@@ -210,8 +212,6 @@ protected:
 
   virtual nsresult FlushTags() = 0;
 
-  void TryToScrollToRef();
-
   // Later on we might want to make this more involved somehow
   // (e.g. stop waiting after some timeout or whatnot).
   PRBool WaitForPendingSheets() { return mPendingSheetCount > 0; }

layout/base/nsDocumentViewer.cpp

@@ -1011,6 +1011,7 @@ DocumentViewerImpl::LoadComplete(nsresult aStatus)
   if (mPresShell && !mStopped) {
     nsCOMPtr<nsIPresShell> shellDeathGrip(mPresShell); // bug 378682
     mPresShell->UnsuppressPainting();
+    mPresShell->ScrollToAnchor();
   }
 
   nsJSContext::LoadEnd();

layout/base/nsIPresShell.h

@@ -102,10 +102,10 @@ class gfxContext;
 typedef short SelectionType;
 typedef PRUint32 nsFrameState;
 
-// ff2bdd39-75ed-4392-92b8-8b650c4db574
+// 228a7d67-811b-4d75-85c0-1ee22c0d2af0
 #define NS_IPRESSHELL_IID \
-{ 0xff2bdd39, 0x75ed, 0x4392, \
-  { 0x92, 0xb8, 0x8b, 0x65, 0x0c, 0x4d, 0xb5, 0x74 } }
+{ 0x228a7d67, 0x811b, 0x4d75, \
+  { 0x85, 0xc0, 0x1e, 0xe2, 0x2c, 0x0d, 0x2a, 0xf0 } }
 
 // Constants for ScrollContentIntoView() function
 #define NS_PRESSHELL_SCROLL_TOP      0
@@ -429,6 +429,16 @@ public:
    */
   NS_IMETHOD GoToAnchor(const nsAString& aAnchorName, PRBool aScroll) = 0;
 
+  /**
+   * Tells the presshell to scroll again to the last anchor scrolled to by
+   * GoToAnchor, if any. This scroll only happens if the scroll
+   * position has not changed since the last GoToAnchor. This is called
+   * by nsDocumentViewer::LoadComplete. This clears the last anchor
+   * scrolled to by GoToAnchor (we don't want to keep it alive if it's
+   * removed from the DOM), so don't call this more than once.
+   */
+  NS_IMETHOD ScrollToAnchor() = 0;
+
   /**
    * Scrolls the view of the document so that the primary frame of the content
    * is displayed at the top of the window. Layout is flushed before scrolling.

layout/base/nsPresShell.cpp

@@ -835,6 +835,7 @@ public:
   NS_IMETHOD CreateRenderingContext(nsIFrame *aFrame,
                                     nsIRenderingContext** aContext);
   NS_IMETHOD GoToAnchor(const nsAString& aAnchorName, PRBool aScroll);
+  NS_IMETHOD ScrollToAnchor();
 
   NS_IMETHOD ScrollContentIntoView(nsIContent* aContent,
                                    PRIntn      aVPercent,
@@ -1134,6 +1135,8 @@ protected:
   nsVoidArray mCurrentEventFrameStack;
   nsCOMArray<nsIContent> mCurrentEventContentStack;
 
+  nsCOMPtr<nsIContent>          mLastAnchorScrolledTo;
+  nscoord                       mLastAnchorScrollPositionY;
   nsCOMPtr<nsICaret>            mCaret;
   nsCOMPtr<nsICaret>            mOriginalCaret;
   PRInt16                       mSelectionFlags;
@@ -3620,11 +3623,16 @@ PresShell::GoToAnchor(const nsAString& aAnchorName, PRBool aScroll)
   esm->SetContentState(content, NS_EVENT_STATE_URLTARGET);
 
   if (content) {
-    // Flush notifications so we scroll to the right place
     if (aScroll) {
       rv = ScrollContentIntoView(content, NS_PRESSHELL_SCROLL_TOP,
                                  NS_PRESSHELL_SCROLL_ANYWHERE);
       NS_ENSURE_SUCCESS(rv, rv);
+
+      nsIScrollableFrame* rootScroll = GetRootScrollFrameAsScrollable();
+      if (rootScroll) {
+        mLastAnchorScrolledTo = content;
+        mLastAnchorScrollPositionY = rootScroll->GetScrollPosition().y;
+      }
     }
 
     // Should we select the target? This action is controlled by a
@@ -3708,6 +3716,23 @@ PresShell::GoToAnchor(const nsAString& aAnchorName, PRBool aScroll)
   return rv;
 }
 
+NS_IMETHODIMP
+PresShell::ScrollToAnchor()
+{
+  if (!mLastAnchorScrolledTo)
+    return NS_OK;
+
+  nsIScrollableFrame* rootScroll = GetRootScrollFrameAsScrollable();
+  if (!rootScroll ||
+      mLastAnchorScrollPositionY != rootScroll->GetScrollPosition().y)
+    return NS_OK;
+
+  nsresult rv = ScrollContentIntoView(mLastAnchorScrolledTo, NS_PRESSHELL_SCROLL_TOP,
+                                      NS_PRESSHELL_SCROLL_ANYWHERE);
+  mLastAnchorScrolledTo = nsnull;
+  return rv;
+}
+
 /*
  * Helper (per-continuation) for ScrollContentIntoView.
  *