From 0f51d83377384d8384f2888abddb667d09bd8161 Mon Sep 17 00:00:00 2001 From: Timothy Nikkel Date: Tue, 11 May 2010 19:30:40 -0500 Subject: [PATCH] Bug 559996. Lazy frame construction can sometimes cause a subdocument to not scroll to ref in url. r=jst --- content/base/public/nsIDocument.h | 9 +- content/base/src/nsContentSink.cpp | 77 +-------------- content/base/src/nsContentSink.h | 4 - content/base/src/nsDocument.cpp | 95 +++++++++++++++++++ content/base/src/nsDocument.h | 9 ++ content/test/reftest/bug559996-iframe.html | 7 ++ .../test/reftest/bug559996-ref-iframe.html | 8 ++ content/test/reftest/bug559996-ref.html | 7 ++ content/test/reftest/bug559996.html | 7 ++ content/test/reftest/reftest.list | 1 + layout/base/nsDocumentViewer.cpp | 4 + layout/base/nsPresShell.cpp | 10 +- 12 files changed, 159 insertions(+), 79 deletions(-) create mode 100644 content/test/reftest/bug559996-iframe.html create mode 100644 content/test/reftest/bug559996-ref-iframe.html create mode 100644 content/test/reftest/bug559996-ref.html create mode 100644 content/test/reftest/bug559996.html diff --git a/content/base/public/nsIDocument.h b/content/base/public/nsIDocument.h index 9ab96442e62..0f3705de885 100644 --- a/content/base/public/nsIDocument.h +++ b/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() { diff --git a/content/base/src/nsContentSink.cpp b/content/base/src/nsContentSink.cpp index 4f31eea0c4e..fcb985da86b 100644 --- a/content/base/src/nsContentSink.cpp +++ b/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 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 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(); diff --git a/content/base/src/nsContentSink.h b/content/base/src/nsContentSink.h index 3da5502c696..24c9749f78c 100644 --- a/content/base/src/nsContentSink.h +++ b/content/base/src/nsContentSink.h @@ -315,8 +315,6 @@ protected: nsCOMArray 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 diff --git a/content/base/src/nsDocument.cpp b/content/base/src/nsDocument.cpp index 92010ce271b..d343b0ad63e 100644 --- a/content/base/src/nsDocument.cpp +++ b/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 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 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) { diff --git a/content/base/src/nsDocument.h b/content/base/src/nsDocument.h index 0baa52eddca..401976eb198 100644 --- a/content/base/src/nsDocument.h +++ b/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 mDOMImplementation; + nsCString mScrollToRef; + PRUint8 mScrolledToRefAlready : 1; + PRUint8 mChangeScrollPosWhenScrollingToRef : 1; + #ifdef DEBUG protected: PRBool mWillReparent; diff --git a/content/test/reftest/bug559996-iframe.html b/content/test/reftest/bug559996-iframe.html new file mode 100644 index 00000000000..b576d1d2231 --- /dev/null +++ b/content/test/reftest/bug559996-iframe.html @@ -0,0 +1,7 @@ + + + +
first
+
second
+ + diff --git a/content/test/reftest/bug559996-ref-iframe.html b/content/test/reftest/bug559996-ref-iframe.html new file mode 100644 index 00000000000..2b13b8b343a --- /dev/null +++ b/content/test/reftest/bug559996-ref-iframe.html @@ -0,0 +1,8 @@ + + + +
first
+
second
+ + + diff --git a/content/test/reftest/bug559996-ref.html b/content/test/reftest/bug559996-ref.html new file mode 100644 index 00000000000..4133bf0c6f2 --- /dev/null +++ b/content/test/reftest/bug559996-ref.html @@ -0,0 +1,7 @@ + + + + + + + diff --git a/content/test/reftest/bug559996.html b/content/test/reftest/bug559996.html new file mode 100644 index 00000000000..766bb7bfdbe --- /dev/null +++ b/content/test/reftest/bug559996.html @@ -0,0 +1,7 @@ + + + + + + + diff --git a/content/test/reftest/reftest.list b/content/test/reftest/reftest.list index 47612cb2438..ad5d4d839e2 100644 --- a/content/test/reftest/reftest.list +++ b/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 diff --git a/layout/base/nsDocumentViewer.cpp b/layout/base/nsDocumentViewer.cpp index 90205327b33..41068686d6a 100644 --- a/layout/base/nsDocumentViewer.cpp +++ b/layout/base/nsDocumentViewer.cpp @@ -804,6 +804,10 @@ DocumentViewerImpl::InitPresentationStuff(PRBool aDoInitialReflow) } } + if (aDoInitialReflow && mDocument) { + mDocument->ScrollToRef(); + } + return NS_OK; } diff --git a/layout/base/nsPresShell.cpp b/layout/base/nsPresShell.cpp index bd4bf050d61..d18e8941b8e 100644 --- a/layout/base/nsPresShell.cpp +++ b/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 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 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;