Bug 559996. Lazy frame construction can sometimes cause a subdocument to not scroll to ref in url. r=jst

content/base/public/nsIDocument.h

@@ -116,8 +116,8 @@ class Element;
 } // namespace mozilla
 
 #define NS_IDOCUMENT_IID      \
-{ 0x625fe492, 0x0344, 0x406c, \
-  { 0xaf, 0x7f, 0x55, 0xfe, 0xa2, 0x6b, 0x3d, 0x20 } }
+{ 0xeb847679, 0x3b48, 0x411c, \
+  { 0xa9, 0xb8, 0x8a, 0xdc, 0xdb, 0xc6, 0x47, 0xb8 } }
 
 // Flag for AddStyleSheet().
 #define NS_STYLESHEET_FROM_CATALOG                (1 << 0)
@@ -1351,6 +1351,11 @@ public:
    */
   virtual void RegisterFileDataUri(nsACString& aUri) = 0;
 
+  virtual void SetScrollToRef(nsIURI *aDocumentURI) = 0;
+  virtual void ScrollToRef() = 0;
+  virtual void ResetScrolledToRefAlready() = 0;
+  virtual void SetChangeScrollPosWhenScrollingToRef(PRBool aValue) = 0;
+
 protected:
   ~nsIDocument()
   {

content/base/src/nsContentSink.cpp

@@ -291,8 +291,8 @@ nsContentSink::Init(nsIDocument* aDoc,
   if (mDocShell) {
     PRUint32 loadType = 0;
     mDocShell->GetLoadType(&loadType);
-    mChangeScrollPosWhenScrollingToRef =
-      ((loadType & nsIDocShell::LOAD_CMD_HISTORY) == 0);
+    mDocument->SetChangeScrollPosWhenScrollingToRef(
+      (loadType & nsIDocShell::LOAD_CMD_HISTORY) == 0);
   }
 
   // use this to avoid a circular reference sink->document->scriptloader->sink
@@ -1235,54 +1235,7 @@ nsContentSink::ProcessOfflineManifest(const nsAString& aManifestSpec)
 void
 nsContentSink::ScrollToRef()
 {
-  if (mRef.IsEmpty()) {
-    return;
-  }
-
-  if (mScrolledToRefAlready) {
-    return;
-  }
-
-  char* tmpstr = ToNewCString(mRef);
-  if (!tmpstr) {
-    return;
-  }
-
-  nsUnescape(tmpstr);
-  nsCAutoString unescapedRef;
-  unescapedRef.Assign(tmpstr);
-  nsMemory::Free(tmpstr);
-
-  nsresult rv = NS_ERROR_FAILURE;
-  // We assume that the bytes are in UTF-8, as it says in the spec:
-  // http://www.w3.org/TR/html4/appendix/notes.html#h-B.2.1
-  NS_ConvertUTF8toUTF16 ref(unescapedRef);
-
-  nsCOMPtr<nsIPresShell> shell = mDocument->GetPrimaryShell();
-  if (shell) {
-    // Check an empty string which might be caused by the UTF-8 conversion
-    if (!ref.IsEmpty()) {
-      // Note that GoToAnchor will handle flushing layout as needed.
-      rv = shell->GoToAnchor(ref, mChangeScrollPosWhenScrollingToRef);
-    } else {
-      rv = NS_ERROR_FAILURE;
-    }
-
-    // If UTF-8 URI failed then try to assume the string as a
-    // document's charset.
-
-    if (NS_FAILED(rv)) {
-      const nsACString &docCharset = mDocument->GetDocumentCharacterSet();
-
-      rv = nsContentUtils::ConvertStringFromCharset(docCharset, unescapedRef, ref);
-
-      if (NS_SUCCEEDED(rv) && !ref.IsEmpty())
-        rv = shell->GoToAnchor(ref, mChangeScrollPosWhenScrollingToRef);
-    }
-    if (NS_SUCCEEDED(rv)) {
-      mScrolledToRefAlready = PR_TRUE;
-    }
-  }
+  mDocument->ScrollToRef();
 }
 
 void
@@ -1332,27 +1285,7 @@ nsContentSink::StartLayout(PRBool aIgnorePendingSheets)
   // If the document we are loading has a reference or it is a
   // frameset document, disable the scroll bars on the views.
 
-  if (mDocumentURI) {
-    nsCAutoString ref;
-
-    // Since all URI's that pass through here aren't URL's we can't
-    // rely on the nsIURI implementation for providing a way for
-    // finding the 'ref' part of the URI, we'll haveto revert to
-    // string routines for finding the data past '#'
-
-    mDocumentURI->GetSpec(ref);
-
-    nsReadingIterator<char> start, end;
-
-    ref.BeginReading(start);
-    ref.EndReading(end);
-
-    if (FindCharInReadable('#', start, end)) {
-      ++start; // Skip over the '#'
-
-      mRef = Substring(start, end);
-    }
-  }
+  mDocument->SetScrollToRef(mDocumentURI);
 }
 
 void
@@ -1739,7 +1672,7 @@ nsContentSink::WillBuildModelImpl()
     mBeginLoadTime = PR_IntervalToMicroseconds(PR_IntervalNow());
   }
 
-  mScrolledToRefAlready = PR_FALSE;
+  mDocument->ResetScrolledToRefAlready();
 
   if (mProcessLinkHeaderEvent.get()) {
     mProcessLinkHeaderEvent.Revoke();

content/base/src/nsContentSink.h

@@ -315,8 +315,6 @@ protected:
 
   nsCOMArray<nsIScriptElement> mScriptElements;
 
-  nsCString mRef; // ScrollTo #ref
-
   // back off timer notification after count
   PRInt32 mBackoffCount;
 
@@ -330,12 +328,10 @@ protected:
   // Have we already called BeginUpdate for this set of content changes?
   PRUint8 mBeganUpdate : 1;
   PRUint8 mLayoutStarted : 1;
-  PRUint8 mScrolledToRefAlready : 1;
   PRUint8 mCanInterruptParser : 1;
   PRUint8 mDynamicLowerValue : 1;
   PRUint8 mParsing : 1;
   PRUint8 mDroppedTimer : 1;
-  PRUint8 mChangeScrollPosWhenScrollingToRef : 1;
   // If true, we deferred starting layout until sheets load
   PRUint8 mDeferredLayoutStart : 1;
   // If true, we deferred notifications until sheets load

content/base/src/nsDocument.cpp

@@ -177,6 +177,7 @@ static NS_DEFINE_CID(kDOMEventGroupCID, NS_DOMEVENTGROUP_CID);
 #include "nsIPropertyBag2.h"
 #include "nsIDOMPageTransitionEvent.h"
 #include "nsFrameLoader.h"
+#include "nsEscape.h"
 #ifdef MOZ_MEDIA
 #include "nsHTMLMediaElement.h"
 #endif // MOZ_MEDIA
@@ -7677,6 +7678,100 @@ nsDocument::RegisterFileDataUri(nsACString& aUri)
   mFileDataUris.AppendElement(aUri);
 }
 
+void
+nsDocument::SetScrollToRef(nsIURI *aDocumentURI)
+{
+  if (!aDocumentURI) {
+    return;
+  }
+
+  nsCAutoString ref;
+
+  // Since all URI's that pass through here aren't URL's we can't
+  // rely on the nsIURI implementation for providing a way for
+  // finding the 'ref' part of the URI, we'll haveto revert to
+  // string routines for finding the data past '#'
+
+  aDocumentURI->GetSpec(ref);
+
+  nsReadingIterator<char> start, end;
+
+  ref.BeginReading(start);
+  ref.EndReading(end);
+
+  if (FindCharInReadable('#', start, end)) {
+    ++start; // Skip over the '#'
+
+    mScrollToRef = Substring(start, end);
+  }
+}
+
+void
+nsDocument::ScrollToRef()
+{
+  if (mScrolledToRefAlready) {
+    return;
+  }
+
+  if (mScrollToRef.IsEmpty()) {
+    return;
+  }
+
+  char* tmpstr = ToNewCString(mScrollToRef);
+  if (!tmpstr) {
+    return;
+  }
+
+  nsUnescape(tmpstr);
+  nsCAutoString unescapedRef;
+  unescapedRef.Assign(tmpstr);
+  nsMemory::Free(tmpstr);
+
+  nsresult rv = NS_ERROR_FAILURE;
+  // We assume that the bytes are in UTF-8, as it says in the spec:
+  // http://www.w3.org/TR/html4/appendix/notes.html#h-B.2.1
+  NS_ConvertUTF8toUTF16 ref(unescapedRef);
+
+  nsCOMPtr<nsIPresShell> shell = GetPrimaryShell();
+  if (shell) {
+    // Check an empty string which might be caused by the UTF-8 conversion
+    if (!ref.IsEmpty()) {
+      // Note that GoToAnchor will handle flushing layout as needed.
+      rv = shell->GoToAnchor(ref, mChangeScrollPosWhenScrollingToRef);
+    } else {
+      rv = NS_ERROR_FAILURE;
+    }
+
+    // If UTF-8 URI failed then try to assume the string as a
+    // document's charset.
+
+    if (NS_FAILED(rv)) {
+      const nsACString &docCharset = GetDocumentCharacterSet();
+
+      rv = nsContentUtils::ConvertStringFromCharset(docCharset, unescapedRef, ref);
+
+      if (NS_SUCCEEDED(rv) && !ref.IsEmpty()) {
+        rv = shell->GoToAnchor(ref, mChangeScrollPosWhenScrollingToRef);
+      }
+    }
+    if (NS_SUCCEEDED(rv)) {
+      mScrolledToRefAlready = PR_TRUE;
+    }
+  }
+}
+
+void
+nsDocument::ResetScrolledToRefAlready()
+{
+  mScrolledToRefAlready = PR_FALSE;
+}
+
+void
+nsDocument::SetChangeScrollPosWhenScrollingToRef(PRBool aValue)
+{
+  mChangeScrollPosWhenScrollingToRef = aValue;
+}
+
 void
 nsIDocument::RegisterFreezableElement(nsIContent* aContent)
 {

content/base/src/nsDocument.h

@@ -930,6 +930,11 @@ public:
   // Only BlockOnload should call this!
   void AsyncBlockOnload();
 
+  virtual void SetScrollToRef(nsIURI *aDocumentURI);
+  virtual void ScrollToRef();
+  virtual void ResetScrolledToRefAlready();
+  virtual void SetChangeScrollPosWhenScrollingToRef(PRBool aValue);
+
 protected:
   friend class nsNodeUtils;
   void RegisterNamedItems(nsIContent *aContent);
@@ -1188,6 +1193,10 @@ private:
 
   nsCOMPtr<nsIDOMDOMImplementation> mDOMImplementation;
 
+  nsCString mScrollToRef;
+  PRUint8 mScrolledToRefAlready : 1;
+  PRUint8 mChangeScrollPosWhenScrollingToRef : 1;
+
 #ifdef DEBUG
 protected:
   PRBool mWillReparent;

content/test/reftest/bug559996-iframe.html

@@ -0,0 +1,7 @@
+<html>
+<!-- This page is loaded in a 200px-square iframe scrolled to #down. -->
+<body>
+<div id="first" style="height: 150px; background: lightblue;">first</div>
+<div id="down" style="height: 250px; background: lightgreen;">second</div>
+</body>
+</html>

content/test/reftest/bug559996-ref-iframe.html

@@ -0,0 +1,8 @@
+<html>
+<!-- This page is loaded in a 200px-square iframe scrolled to #down. -->
+<body>
+<div id="first" style="height: 150px; background: lightblue;">first</div>
+<div id="down" style="height: 250px; background: lightgreen;">second</div>
+<script>document.documentElement.offsetWidth;</script>
+</body>
+</html>

content/test/reftest/bug559996-ref.html

@@ -0,0 +1,7 @@
+<html>
+<body>
+
+<iframe src="bug559996-ref-iframe.html#down" style="height: 200px; width: 200px"></iframe>
+
+</body>
+</html>

content/test/reftest/bug559996.html

@@ -0,0 +1,7 @@
+<html>
+<body>
+
+<iframe src="bug559996-iframe.html#down" style="height: 200px; width: 200px"></iframe>
+
+</body>
+</html>

content/test/reftest/reftest.list

@@ -3,3 +3,4 @@
 == bug456008.xhtml bug456008-ref.html
 == bug439965.html bug439965-ref.html
 == bug427779.xml bug427779-ref.xml
+== bug559996.html bug559996-ref.html

layout/base/nsDocumentViewer.cpp

@@ -804,6 +804,10 @@ DocumentViewerImpl::InitPresentationStuff(PRBool aDoInitialReflow)
     }
   }
 
+  if (aDoInitialReflow && mDocument) {
+    mDocument->ScrollToRef();
+  }
+
   return NS_OK;
 }
 

layout/base/nsPresShell.cpp

@@ -3555,7 +3555,9 @@ PresShell::GoToAnchor(const nsAString& aAnchorName, PRBool aScroll)
   if (!mDocument) {
     return NS_ERROR_FAILURE;
   }
-  
+
+  NS_ASSERTION(mDidInitialReflow, "should have done initial reflow by now");
+
   // Hold a reference to the ESM in case event dispatch tears us down.
   nsCOMPtr<nsIEventStateManager> esm = mPresContext->EventStateManager();
 
@@ -3812,6 +3814,8 @@ PresShell::GoToAnchor(const nsAString& aAnchorName, PRBool aScroll)
 nsresult
 PresShell::ScrollToAnchor()
 {
+  NS_ASSERTION(mDidInitialReflow, "should have done initial reflow by now");
+
   if (!mLastAnchorScrolledTo)
     return NS_OK;
 
@@ -4011,6 +4015,8 @@ PresShell::ScrollContentIntoView(nsIContent* aContent,
   nsCOMPtr<nsIDocument> currentDoc = content->GetCurrentDoc();
   NS_ENSURE_STATE(currentDoc);
 
+  NS_ASSERTION(mDidInitialReflow, "should have done initial reflow by now");
+
   mContentToScrollTo = aContent;
   mContentScrollVPosition = aVPercent;
   mContentScrollHPosition = aHPercent;
@@ -4037,6 +4043,8 @@ PresShell::DoScrollContentIntoView(nsIContent* aContent,
                                    PRIntn      aVPercent,
                                    PRIntn      aHPercent)
 {
+  NS_ASSERTION(mDidInitialReflow, "should have done initial reflow by now");
+
   nsIFrame* frame = aContent->GetPrimaryFrame();
   if (!frame) {
     mContentToScrollTo = nsnull;