diff --git a/docshell/base/nsDocShell.cpp b/docshell/base/nsDocShell.cpp index 89c937c51b4..c548b4945b0 100644 --- a/docshell/base/nsDocShell.cpp +++ b/docshell/base/nsDocShell.cpp @@ -7910,7 +7910,7 @@ nsDocShell::InternalLoad(nsIURI * aURI, do_QueryInterface(mScriptGlobal); if (window) - window->DispatchAsyncHashchange(); + window->DispatchSyncHashchange(); } return NS_OK; diff --git a/docshell/base/nsDocShell.h b/docshell/base/nsDocShell.h index 01c1a9bcf6d..f6ba8ccd8df 100644 --- a/docshell/base/nsDocShell.h +++ b/docshell/base/nsDocShell.h @@ -364,12 +364,6 @@ protected: PRUint32 aLoadType, nscoord *cx, nscoord *cy, PRBool * aDoHashchange); - // Dispatches the hashchange event to the current thread, if the document's - // readystate is "complete". - nsresult DispatchAsyncHashchange(); - - nsresult FireHashchange(); - // Returns PR_TRUE if would have called FireOnLocationChange, // but did not because aFireOnLocationChange was false on entry. // In this case it is the caller's responsibility to ensure diff --git a/docshell/test/test_bug385434.html b/docshell/test/test_bug385434.html index 108680a5b9b..9e5092953e5 100644 --- a/docshell/test/test_bug385434.html +++ b/docshell/test/test_bug385434.html @@ -25,6 +25,7 @@ SimpleTest.waitForExplicitFinish(); var gNumHashchanges = 0; var gCallbackOnIframeLoad = false; +var gCallbackOnHashchange = false; function statusMsg(msg) { var msgElem = document.createElement("p"); @@ -42,7 +43,14 @@ function longWait() { // event which was fired. function onIframeHashchange() { gNumHashchanges++; - gGen.next(); + if (gCallbackOnHashchange) { + gCallbackOnHashchange = false; + gGen.next(); + } +} + +function enableHashchangeCallback() { + gCallbackOnHashchange = true; } function onIframeLoad() { @@ -113,36 +121,35 @@ function run_test() { noEventExpected("No hashchange expected initially."); + enableHashchangeCallback(); sendMouseEvent({type: "click"}, "link1", frameCw); yield; eventExpected("Clicking link1 should trigger a hashchange."); + enableHashchangeCallback(); sendMouseEvent({type: "click"}, "link1", frameCw); longWait(); yield; // succeed if a hashchange event wasn't triggered while we were waiting noEventExpected("Clicking link1 again should not trigger a hashchange."); + enableHashchangeCallback(); sendMouseEvent({type: "click"}, "link2", frameCw); yield; eventExpected("Clicking link2 should trigger a hashchange."); frameCw.history.go(-1); - yield; eventExpected("Going back should trigger a hashchange."); frameCw.history.go(1); - yield; eventExpected("Going forward should trigger a hashchange."); frameCw.window.location.hash = ""; - yield; eventExpected("Changing window.location.hash should trigger a hashchange."); // window.location has a trailing '#' right now, so we append "link1", not // "#link1". frameCw.window.location = frameCw.window.location + "link1"; - yield; eventExpected("Assigning to window.location should trigger a hashchange."); // Set up history in the iframe which looks like: @@ -177,9 +184,8 @@ function run_test() { yield; frameCw.document.location = "file_bug385434_2.html#foo"; - yield; - eventExpected("frame onhashchange should fire events."); + // iframe should set gSampleEvent is(gSampleEvent.target, frameCw, "The hashchange event should be targeted to the window."); @@ -195,6 +201,7 @@ function run_test() { * hashchange is dispatched if the current document readyState is * not "complete" (bug 504837). */ + enableHashchangeCallback(); frameCw.document.location = "file_bug385434_3.html"; yield; eventExpected("Hashchange should fire even if the document " + diff --git a/docshell/test/test_bug509055.html b/docshell/test/test_bug509055.html index 038e02e5530..76cd27ab27c 100644 --- a/docshell/test/test_bug509055.html +++ b/docshell/test/test_bug509055.html @@ -29,12 +29,6 @@ function shortWait() { setTimeout(function() { gGen.next(); }, 0, false); } -function onChildHashchange(e) { - // gGen might be undefined when we refresh the page, so we have to check here - if(gGen) - gGen.next(); -} - function onChildLoad(e) { if(gGen) gGen.next(); @@ -46,7 +40,6 @@ function runTest() { var popup = window.open("file_bug509055.html", "popup 0", "height=200,width=200,location=yes," + "menubar=yes,status=yes,toolbar=yes,dependent=yes"); - popup.hashchangeCallback = onChildHashchange; popup.onload = onChildLoad; yield; // wait for load @@ -55,11 +48,10 @@ function runTest() { shortWait(); yield; + // Both setting location.hash and calling history.back() happen + // synchronously, so there's no need to yield here. popup.location.hash = "#1"; - yield; // wait for hashchange - popup.history.back(); - yield; // wait for hashchange popup.document.title = "Changed"; @@ -68,8 +60,8 @@ function runTest() { yield; var sh = popup.QueryInterface(Components.interfaces.nsIInterfaceRequestor) - .getInterface(Components.interfaces.nsIWebNavigation) - .sessionHistory; + .getInterface(Components.interfaces.nsIWebNavigation) + .sessionHistory; // Get the title of the inner popup's current SHEntry var sheTitle = sh.getEntryAtIndex(sh.index, false).title; diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index da6e733a2d5..8bfc3cbca9a 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -6925,20 +6925,11 @@ nsGlobalWindow::PageHidden() } nsresult -nsGlobalWindow::DispatchAsyncHashchange() +nsGlobalWindow::DispatchSyncHashchange() { - FORWARD_TO_INNER(DispatchAsyncHashchange, (), NS_OK); - - nsCOMPtr event = - NS_NEW_RUNNABLE_METHOD(nsGlobalWindow, this, FireHashchange); - - return NS_DispatchToCurrentThread(event); -} - -nsresult -nsGlobalWindow::FireHashchange() -{ - NS_ENSURE_TRUE(IsInnerWindow(), NS_ERROR_FAILURE); + FORWARD_TO_INNER(DispatchSyncHashchange, (), NS_OK); + NS_ASSERTION(nsContentUtils::IsSafeToRunScript(), + "Must be safe to run script here."); // Don't do anything if the window is frozen. if (IsFrozen()) diff --git a/dom/base/nsGlobalWindow.h b/dom/base/nsGlobalWindow.h index 77b90f16b69..b26241e6311 100644 --- a/dom/base/nsGlobalWindow.h +++ b/dom/base/nsGlobalWindow.h @@ -443,7 +443,7 @@ public: virtual PRBool TakeFocus(PRBool aFocus, PRUint32 aFocusMethod); virtual void SetReadyForFocus(); virtual void PageHidden(); - virtual nsresult DispatchAsyncHashchange(); + virtual nsresult DispatchSyncHashchange(); virtual nsresult SetArguments(nsIArray *aArguments, nsIPrincipal *aOrigin); static PRBool DOMWindowDumpEnabled(); @@ -574,8 +574,7 @@ protected: const nsAString &aPopupWindowName, const nsAString &aPopupWindowFeatures); void FireOfflineStatusEvent(); - nsresult FireHashchange(); - + void FlushPendingNotifications(mozFlushType aType); void EnsureReflowFlushAndPaint(); nsresult CheckSecurityWidthAndHeight(PRInt32* width, PRInt32* height); diff --git a/dom/base/nsPIDOMWindow.h b/dom/base/nsPIDOMWindow.h index 59942b37480..4c90823e231 100644 --- a/dom/base/nsPIDOMWindow.h +++ b/dom/base/nsPIDOMWindow.h @@ -454,10 +454,9 @@ public: virtual void PageHidden() = 0; /** - * Instructs this window to asynchronously dispatch a hashchange event. This - * method must be called on an inner window. + * Instructs this window to synchronously dispatch a hashchange event. */ - virtual nsresult DispatchAsyncHashchange() = 0; + virtual nsresult DispatchSyncHashchange() = 0; /** diff --git a/dom/tests/mochitest/general/test_bug504220.html b/dom/tests/mochitest/general/test_bug504220.html index a0c1412ca52..b84c3e2e049 100644 --- a/dom/tests/mochitest/general/test_bug504220.html +++ b/dom/tests/mochitest/general/test_bug504220.html @@ -23,6 +23,8 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=504220 /** Test for Bug 504220 **/ function run_test() { + SimpleTest.waitForExplicitFinish(); + ok("onhashchange" in document.body, "document.body should contain 'onhashchange'."); @@ -35,30 +37,20 @@ function run_test() { // Likewise, document.body.hashchange should mirror window.onhashchange numEvents = 0; - var func2 = function() { numEvents++; gGen.next() }; + var func2 = function() { numEvents++; }; window.onhashchange = func2; is(document.body.onhashchange, func2); - SimpleTest.waitForExplicitFinish(); + // Change the document's hash. If we've been running this test manually, + // the hash might already be "#foo", so we need to check in order to be + // sure we trigger a hashchange. + if (location.hash != "#foo") + location.hash = "#foo"; + else + location.hash = "#bar"; - function waitForHashchange() { - // Change the document's hash. If we've been running this test manually, - // the hash might already be "#foo", so we need to check in order to be - // sure we trigger a hashchange. - if (location.hash != "#foo") - location.hash = "#foo"; - else - location.hash = "#bar"; - - yield; - - is(numEvents, 1, "Exactly one hashchange should have been fired."); - SimpleTest.finish(); - yield; - } - - var gGen = waitForHashchange(); - gGen.next(); + is(numEvents, 1, "Exactly one hashchange should have been fired."); + SimpleTest.finish(); }