Bug 662170 - Navigating to anchor '#' should scroll to top of the page. r=bz
authorJustin Lebar <justin.lebar@gmail.com>
Wed, 08 Jun 2011 14:08:29 -0400
changeset 70731 21bd522611ea0c2e8fe71d0b9cd0485414b382b0
parent 70730 ddf82c182c85c69bffdb89dcf0fd0f4788c9ab95
child 70732 a968740930d3bfc22e9c3dd75b35d95697b0791f
push id20401
push userjlebar@mozilla.com
push dateWed, 08 Jun 2011 18:41:37 +0000
treeherdermozilla-central@a968740930d3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbz
bugs662170
milestone7.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 662170 - Navigating to anchor '#' should scroll to top of the page. r=bz
docshell/base/nsDocShell.cpp
docshell/test/Makefile.in
docshell/test/file_bug662170.html
docshell/test/test_bug662170.html
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -8355,21 +8355,17 @@ nsDocShell::InternalLoad(nsIURI * aURI,
             nscoord cx = 0, cy = 0;
             GetCurScrollPos(ScrollOrientation_X, &cx);
             GetCurScrollPos(ScrollOrientation_Y, &cy);
 
             // We scroll whenever we're not doing a history load.  Note that
             // sometimes we might scroll even if we don't fire a hashchange
             // event!  See bug 653741.
             if (!aSHEntry) {
-                // Take the '#' off the hashes before passing them to
-                // ScrollToAnchor.
-                nsDependentCSubstring curHashName(curHash, 1);
-                nsDependentCSubstring newHashName(newHash, 1);
-                rv = ScrollToAnchor(curHashName, newHashName, aLoadType);
+                rv = ScrollToAnchor(curHash, newHash, aLoadType);
                 NS_ENSURE_SUCCESS(rv, rv);
             }
 
             mLoadType = aLoadType;
             mURIResultedInDocument = PR_TRUE;
 
             /* we need to assign mLSHE to aSHEntry right here, so that on History loads,
              * SetCurrentURI() called from OnNewURI() will send proper
@@ -9154,26 +9150,30 @@ nsDocShell::ScrollToAnchor(nsACString & 
     // current anchor and we are doing a history load.  So return if we have no
     // new anchor, and there is no current anchor or the load is not a history
     // load.
     if ((aCurHash.IsEmpty() || aLoadType != LOAD_HISTORY) &&
         aNewHash.IsEmpty()) {
         return NS_OK;
     }
 
+    // Take the '#' off aNewHash to get the ref name.  (aNewHash might be empty,
+    // but that's fine.)
+    nsDependentCSubstring newHashName(aNewHash, 1);
+
     // Both the new and current URIs refer to the same page. We can now
     // browse to the hash stored in the new URI.
 
-    if (!aNewHash.IsEmpty()) {
+    if (!newHashName.IsEmpty()) {
         // anchor is there, but if it's a load from history,
         // we don't have any anchor jumping to do
         PRBool scroll = aLoadType != LOAD_HISTORY &&
                         aLoadType != LOAD_RELOAD_NORMAL;
 
-        char *str = ToNewCString(aNewHash);
+        char *str = ToNewCString(newHashName);
         if (!str) {
             return NS_ERROR_OUT_OF_MEMORY;
         }
 
         // nsUnescape modifies the string that is passed into it.
         nsUnescape(str);
 
         // We assume that the bytes are in UTF-8, as it says in the
@@ -9205,17 +9205,17 @@ nsDocShell::ScrollToAnchor(nsACString & 
             nsCOMPtr<nsITextToSubURI> textToSubURI =
                 do_GetService(NS_ITEXTTOSUBURI_CONTRACTID, &rv);
             NS_ENSURE_SUCCESS(rv, rv);
 
             // Unescape and convert to unicode
             nsXPIDLString uStr;
 
             rv = textToSubURI->UnEscapeAndConvert(PromiseFlatCString(aCharset).get(),
-                                                  PromiseFlatCString(aNewHash).get(),
+                                                  PromiseFlatCString(newHashName).get(),
                                                   getter_Copies(uStr));
             NS_ENSURE_SUCCESS(rv, rv);
 
             // Ignore return value of GoToAnchor, since it will return an error
             // if there is no such anchor in the document, which is actually a
             // success condition for us (we want to update the session history
             // with the new URI no matter whether we actually scrolled
             // somewhere).
--- a/docshell/test/Makefile.in
+++ b/docshell/test/Makefile.in
@@ -104,16 +104,18 @@ include $(topsrcdir)/config/rules.mk
 		test_bug653741.html \
 		file_bug653741.html \
 		test_framedhistoryframes.html \
 		test_windowedhistoryframes.html \
 		historyframes.html \
 		test_bug660404.html \
 		file_bug660404 \
 		file_bug660404^headers^ \
+		test_bug662170.html \
+		file_bug662170.html \
 		$(NULL)
 
 ifeq ($(MOZ_WIDGET_TOOLKIT),cocoa)
 _TEST_FILES += \
 		test_bug511449.html \
 		file_bug511449.html \
 		$(NULL)
 endif
new file mode 100644
--- /dev/null
+++ b/docshell/test/file_bug662170.html
@@ -0,0 +1,13 @@
+<html>
+<body onload='(parent || opener).childLoad()'>
+
+<div style='height:500px; background:yellow'>
+<a id='#top'>Top of the page</a>
+</div>
+
+<div id='bottom'>
+<a id='#bottom'>Bottom of the page</a>
+</div>
+
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/docshell/test/test_bug662170.html
@@ -0,0 +1,52 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=662170
+-->
+<head>
+  <title>Test for Bug 662170</title>
+  <script type="application/javascript" src="/MochiKit/packed.js"></script>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <script type="text/javascript" src="/tests/SimpleTest/WindowSnapshot.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=662170">Mozilla Bug 662170</a>
+
+<script type="application/javascript;version=1.7">
+
+/** Test for Bug 662170 **/
+SimpleTest.waitForExplicitFinish();
+
+function childLoad() {
+  // Spin the event loop so we leave the onload handler.
+  SimpleTest.executeSoon(childLoad2);
+}
+
+function childLoad2() {
+  let cw = $('iframe').contentWindow;
+
+  // When we initially load the page, we should be at the top.
+  is(cw.pageYOffset, 0, 'Initial Y offset should be 0.');
+  
+  // Scroll the iframe to the bottom.
+  cw.scrollTo(0, 300);
+
+  // Did we actually scroll somewhere?
+  isnot(cw.pageYOffset, 0, 'Y offset should be non-zero after scrolling.');
+
+  // Now load file_bug662170.html#, which should take us to the top of the
+  // page.
+  cw.location = cw.location + '#';
+
+  is(cw.pageYOffset, 0, 'Correct Y offset after loading #.');
+  SimpleTest.finish();
+}
+
+</script>
+
+<!-- When the iframe loads, it calls childLoad(). -->
+<iframe height='100px' id='iframe' src='file_bug662170.html'></iframe>
+
+</body>
+</html>