Bug 881487. Make anchor scrolls on wyciwyg documents work correctly. r=smaug
authorBoris Zbarsky <bzbarsky@mit.edu>
Mon, 13 Jan 2014 15:08:55 -0500
changeset 163213 4142d03082e1a76483be6b6736539edd749103ef
parent 163210 d2fe5f5b558ed93ba2cc6bd34a595842f0cb2527
child 163214 863b9cd6c44deef792909adaff2c4343a547418e
push id25986
push userkwierso@gmail.com
push dateTue, 14 Jan 2014 04:44:33 +0000
treeherdermozilla-central@b114534a9386 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs881487
milestone29.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 881487. Make anchor scrolls on wyciwyg documents work correctly. r=smaug
docshell/base/nsDocShell.cpp
docshell/test/file_anchor_scroll_after_document_open.html
docshell/test/mochitest.ini
docshell/test/test_anchor_scroll_after_document_open.html
--- a/docshell/base/nsDocShell.cpp
+++ b/docshell/base/nsDocShell.cpp
@@ -9110,23 +9110,31 @@ nsDocShell::InternalLoad(nsIURI * aURI,
     mURIResultedInDocument = false;  // reset the clock...
 
     if (aLoadType == LOAD_NORMAL ||
         aLoadType == LOAD_STOP_CONTENT ||
         LOAD_TYPE_HAS_FLAGS(aLoadType, LOAD_FLAGS_REPLACE_HISTORY) ||
         aLoadType == LOAD_HISTORY ||
         aLoadType == LOAD_LINK) {
 
-        // Split mCurrentURI and aURI on the '#' character.  Make sure we read
+        nsCOMPtr<nsIURI> currentURI;
+        if (sURIFixup && mCurrentURI) {
+            rv = sURIFixup->CreateExposableURI(mCurrentURI,
+                                               getter_AddRefs(currentURI));
+            NS_ENSURE_SUCCESS(rv, rv);
+        } else {
+            currentURI = mCurrentURI;
+        }
+        // Split currentURI and aURI on the '#' character.  Make sure we read
         // the return values of SplitURIAtHash; if it fails, we don't want to
         // allow a short-circuited navigation.
         nsAutoCString curBeforeHash, curHash, newBeforeHash, newHash;
         nsresult splitRv1, splitRv2;
-        splitRv1 = mCurrentURI ?
-            nsContentUtils::SplitURIAtHash(mCurrentURI,
+        splitRv1 = currentURI ?
+            nsContentUtils::SplitURIAtHash(currentURI,
                                            curBeforeHash, curHash) :
             NS_ERROR_FAILURE;
         splitRv2 = nsContentUtils::SplitURIAtHash(aURI, newBeforeHash, newHash);
 
         bool sameExceptHashes = NS_SUCCEEDED(splitRv1) &&
                                   NS_SUCCEEDED(splitRv2) &&
                                   curBeforeHash.Equals(newBeforeHash);
 
@@ -9174,19 +9182,16 @@ nsDocShell::InternalLoad(nsIURI * aURI,
             //   - load a.html
             //   - start loading b.html
             //   - load a.html#h
             // we break the web if we cancel the load of b.html.
             if (aSHEntry && mDocumentRequest) {
                 mDocumentRequest->Cancel(NS_BINDING_ABORTED);
             }
 
-            // 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);
 
             // 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,
@@ -9337,25 +9342,25 @@ nsDocShell::InternalLoad(nsIURI * aURI,
                 // Fire a hashchange event URIs differ, and only in their hashes.
                 bool doHashchange = sameExceptHashes && !curHash.Equals(newHash);
 
                 if (historyNavBetweenSameDoc || doHashchange) {
                     win->DispatchSyncPopState();
                 }
 
                 if (doHashchange) {
-                    // Make sure to use oldURI here, not mCurrentURI, because by
-                    // now, mCurrentURI has changed!
-                    win->DispatchAsyncHashchange(oldURI, aURI);
+                    // Note that currentURI hasn't changed because it's on the
+                    // stack, so we can just use it directly as the old URI.
+                    win->DispatchAsyncHashchange(currentURI, aURI);
                 }
             }
 
             // Inform the favicon service that the favicon for oldURI also
             // applies to aURI.
-            CopyFavicon(oldURI, aURI, mInPrivateBrowsing);
+            CopyFavicon(currentURI, aURI, mInPrivateBrowsing);
 
             return NS_OK;
         }
     }
     
     // mContentViewer->PermitUnload can destroy |this| docShell, which
     // causes the next call of CanSavePresentation to crash. 
     // Hold onto |this| until we return, to prevent a crash from happening. 
new file mode 100644
--- /dev/null
+++ b/docshell/test/file_anchor_scroll_after_document_open.html
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<script>
+  if (location.hash == "#target") {
+    parent.postMessage("haveHash", "*");
+  } else {
+    document.addEventListener("DOMContentLoaded", function() {
+      document.open();
+      document.write("<!DOCTYPE html><html style='height: 100%'><body style='height: 100%'><div style='height: 200%'></div><div id='target'></div></body></html>");
+      document.close();
+      // Notify parent via postMessage, since otherwise exceptions will not get
+      // caught by its onerror handler.
+      parent.postMessage("doTest", "*");
+    });
+  }
+</script>
--- a/docshell/test/mochitest.ini
+++ b/docshell/test/mochitest.ini
@@ -6,16 +6,17 @@ support-files =
   bug404548-subframe.html
   bug413310-post.sjs
   bug413310-subframe.html
   bug529119-window.html
   bug570341_recordevents.html
   bug668513_redirect.html
   bug668513_redirect.html^headers^
   bug691547_frame.html
+  file_anchor_scroll_after_document_open.html
   file_bug385434_1.html
   file_bug385434_2.html
   file_bug385434_3.html
   file_bug475636.sjs
   file_bug509055.html
   file_bug540462.html
   file_bug580069_1.html
   file_bug580069_2.sjs
@@ -28,16 +29,17 @@ support-files =
   file_bug660404^headers^
   file_bug662170.html
   file_bug669671.sjs
   file_bug680257.html
   file_bug703855.html
   file_bug728939.html
   historyframes.html
 
+[test_anchor_scroll_after_document_open.html]
 [test_bfcache_plus_hash.html]
 [test_bug123696.html]
 [test_bug369814.html]
 [test_bug384014.html]
 [test_bug385434.html]
 [test_bug387979.html]
 [test_bug402210.html]
 [test_bug404548.html]
new file mode 100644
--- /dev/null
+++ b/docshell/test/test_anchor_scroll_after_document_open.html
@@ -0,0 +1,55 @@
+<!DOCTYPE HTML>
+<html>
+<!--
+https://bugzilla.mozilla.org/show_bug.cgi?id=881487
+-->
+<head>
+  <meta charset="utf-8">
+  <title>Test for Bug 881487</title>
+  <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
+  <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
+  <script type="application/javascript">
+
+  /** Test for Bug 881487 **/
+  SimpleTest.waitForExplicitFinish();
+  // Child needs to invoke us, otherwise our onload will fire before the child
+  // has done the write/close bit.
+  var gotOnload = false;
+  addLoadEvent(function() {
+    gotOnload = true;
+  });
+  onmessage = function handleMessage(msg) {
+    if (msg.data == "doTest") {
+      if (!gotOnload) {
+        addLoadEvent(function() { handleMessage(msg); });
+        return;
+      }
+      frames[0].onscroll = function() {
+        ok(true, "Got a scroll event");
+        SimpleTest.finish();
+      }
+      frames[0].location.hash = "#target";
+      return;
+    }
+    if (msg.data == "haveHash") {
+      ok(false, "Child got reloaded");
+    } else {
+      ok(false, "Unexpected message");
+    }
+    SimpleTest.finish();
+  }
+
+  </script>
+</head>
+<body>
+<a target="_blank" href="https://bugzilla.mozilla.org/show_bug.cgi?id=881487">Mozilla Bug 881487</a>
+<p id="display">
+  <!-- iframe goes here so it can scroll -->
+<iframe src="file_anchor_scroll_after_document_open.html"></iframe>
+</p>
+<div id="content" style="display: none">
+</div>
+<pre id="test">
+</pre>
+</body>
+</html>