Bug 680257 - Always call nsDocShell::ScrollToAnchor on short-circuited loads, so as to make sure the target pseudo-class is updated correctly. r=bz
authorJustin Lebar <justin.lebar@gmail.com>
Mon, 22 Aug 2011 22:39:37 -0400
changeset 75699 ea8d99353056f8e67a22a04e79f9f8b656b950e2
parent 75698 562229c22e9782ef0bf2720bbbb98d71c6eb252f
child 75700 884897e976ad333070bc0a9668a670028ae69318
push id3
push userfelipc@gmail.com
push dateFri, 30 Sep 2011 20:09:13 +0000
reviewersbz
bugs680257
milestone9.0a1
Bug 680257 - Always call nsDocShell::ScrollToAnchor on short-circuited loads, so as to make sure the target pseudo-class is updated correctly. r=bz
docshell/base/nsDocShell.cpp
docshell/test/Makefile.in
docshell/test/file_bug680257.html
docshell/test/test_bug680257.html
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -8447,23 +8447,23 @@ nsDocShell::InternalLoad(nsIURI * aURI,
             // Save the current URI; we need it if we fire a hashchange later.
             nsCOMPtr<nsIURI> oldURI = mCurrentURI;
 
             // Save the position of the scrollers.
             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) {
-                rv = ScrollToAnchor(curHash, newHash, aLoadType);
-                NS_ENSURE_SUCCESS(rv, rv);
-            }
+            // ScrollToAnchor doesn't necessarily cause us to scroll the window;
+            // the function decides whether a scroll is appropriate based on the
+            // arguments it receives.  But even if we don't end up scrolling,
+            // ScrollToAnchor performs other important tasks, such as informing
+            // the presShell that we have a new hash.  See bug 680257.
+            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
              * onLocationChange() notifications to the browser to update
              * back/forward buttons.
--- a/docshell/test/Makefile.in
+++ b/docshell/test/Makefile.in
@@ -115,16 +115,18 @@ include $(topsrcdir)/config/rules.mk
 		bug570341_recordevents.html \
 		test_bug668513.html \
 		bug668513_redirect.html \
 		bug668513_redirect.html^headers^ \
 		test_bug669671.html \
 		file_bug669671.sjs \
 		test_bug675587.html \
 		test_bfcache_plus_hash.html \
+		test_bug680257.html \
+		file_bug680257.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_bug680257.html
@@ -0,0 +1,16 @@
+<!DOCTYPE HTML>
+<html>
+<head>
+  <style type='text/css'>
+    a        { color: black; }
+    a:target { color: red; }
+  </style>
+</head>
+
+<body onload='(opener || parent).popupLoaded()'>
+
+<a id='a' href='#a'>link</a>
+<a id='b' href='#b'>link2</a>
+
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/docshell/test/test_bug680257.html
@@ -0,0 +1,59 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=680257
+-->
+<head>
+  <title>Test for Bug 680257</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.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=680257">Mozilla Bug 680257</a>
+
+<script type="application/javascript;version=1.7">
+
+SimpleTest.waitForExplicitFinish();
+
+var popup = window.open('file_bug680257.html');
+
+// The popup will call into popupLoaded() once it loads.
+function popupLoaded() {
+  // runTests() needs to be called from outside popupLoaded's onload handler.
+  // Otherwise, the navigations we do in runTests won't create new SHEntries.
+  SimpleTest.executeSoon(runTests);
+}
+
+function runTests() {
+  checkPopupLinkStyle(false, 'Initial');
+
+  popup.location.hash = 'a';
+  checkPopupLinkStyle(true, 'After setting hash');
+
+  popup.history.back();
+  checkPopupLinkStyle(false, 'After going back');
+
+  popup.history.forward();
+  checkPopupLinkStyle(true, 'After going forward');
+
+  popup.close();
+  SimpleTest.finish();
+}
+
+function checkPopupLinkStyle(isTarget, desc) {
+  var link = popup.document.getElementById('a');
+  var style = popup.getComputedStyle(link);
+  var color = style.getPropertyValue('color');
+
+  // Color is red if isTarget, black otherwise.
+  if (isTarget) {
+    is(color, 'rgb(255, 0, 0)', desc);
+  }
+  else {
+    is(color, 'rgb(0, 0, 0)', desc);
+  }
+}
+
+</script>
+</body>
+</html>