From 5aa9b856d07a5e4200508e2c88768b71ec57e148 Mon Sep 17 00:00:00 2001 From: Olli Pettay Date: Wed, 25 Mar 2009 16:11:11 +0200 Subject: [PATCH] Bug 484658 - element.focus() handing when called during event suppression, r+sr=jst --- content/base/public/nsIDocument.h | 14 +++--- content/base/src/nsDocument.cpp | 57 +++++++++++++++++----- content/base/src/nsXMLHttpRequest.cpp | 4 +- content/base/test/test_bug333198.html | 23 ++++++++- content/events/src/nsEventStateManager.cpp | 33 +++++++++++++ dom/base/nsDOMWindowUtils.cpp | 2 +- dom/base/nsGlobalWindow.cpp | 11 +---- 7 files changed, 109 insertions(+), 35 deletions(-) diff --git a/content/base/public/nsIDocument.h b/content/base/public/nsIDocument.h index 86f8aa0ef7c..06ccd4c145e 100644 --- a/content/base/public/nsIDocument.h +++ b/content/base/public/nsIDocument.h @@ -101,8 +101,8 @@ class nsFrameLoader; // IID for the nsIDocument interface #define NS_IDOCUMENT_IID \ -{ 0x6e467d95, 0x9934, 0x422a, \ - { 0x81, 0x07, 0x3f, 0xff, 0xe1, 0x38, 0xe6, 0x1e } } +{ 0x98a4006e, 0x53c4, 0x4390, \ + { 0xb4, 0x2d, 0x33, 0x68, 0x4a, 0xa9, 0x24, 0x04 } } // Flag for AddStyleSheet(). #define NS_STYLESHEET_FROM_CATALOG (1 << 0) @@ -1123,13 +1123,13 @@ public: */ virtual void SuppressEventHandling(PRUint32 aIncrease = 1) = 0; + /** + * Unsuppress event handling. + * @param aFireEvents If PR_TRUE, delayed events (focus/blur) will be fired + * asynchronously. + */ virtual void UnsuppressEventHandlingAndFireEvents(PRBool aFireEvents) = 0; - void UnsuppressEventHandling() - { - UnsuppressEventHandlingAndFireEvents(PR_TRUE); - } - PRUint32 EventHandlingSuppressed() { return mEventsSuppressed; } protected: ~nsIDocument() diff --git a/content/base/src/nsDocument.cpp b/content/base/src/nsDocument.cpp index 5fa6cf84400..a4474f22400 100644 --- a/content/base/src/nsDocument.cpp +++ b/content/base/src/nsDocument.cpp @@ -7484,6 +7484,40 @@ nsDocument::SuppressEventHandling(PRUint32 aIncrease) EnumerateSubDocuments(SuppressEventHandlingInDocument, &aIncrease); } +static void +FireOrClearDelayedEvents(nsTArray >& aDocuments, + PRBool aFireEvents) +{ + for (PRUint32 i = 0; i < aDocuments.Length(); ++i) { + if (!aDocuments[i]->EventHandlingSuppressed()) { + nsPresShellIterator iter(aDocuments[i]); + nsCOMPtr shell; + while ((shell = iter.GetNextShell())) { + shell->FireOrClearDelayedEvents(aFireEvents); + } + } + } +} + +class nsDelayedEventDispatcher : public nsRunnable +{ +public: + nsDelayedEventDispatcher(nsTArray >& aDocuments) + { + mDocuments.SwapElements(aDocuments); + } + virtual ~nsDelayedEventDispatcher() {} + + NS_IMETHOD Run() + { + FireOrClearDelayedEvents(mDocuments, PR_TRUE); + return NS_OK; + } + +private: + nsTArray > mDocuments; +}; + static PRBool GetAndUnsuppressSubDocuments(nsIDocument* aDocument, void* aData) { @@ -7491,8 +7525,9 @@ GetAndUnsuppressSubDocuments(nsIDocument* aDocument, void* aData) if (suppression > 0) { static_cast(aDocument)->DecreaseEventSuppression(); } - nsCOMArray* docs = static_cast* >(aData); - docs->AppendObject(aDocument); + nsTArray >* docs = + static_cast >* >(aData); + docs->AppendElement(aDocument); aDocument->EnumerateSubDocuments(GetAndUnsuppressSubDocuments, docs); return PR_TRUE; } @@ -7503,17 +7538,15 @@ nsDocument::UnsuppressEventHandlingAndFireEvents(PRBool aFireEvents) if (mEventsSuppressed > 0) { --mEventsSuppressed; } - nsCOMArray documents; - documents.AppendObject(this); + + nsTArray > documents; + documents.AppendElement(this); EnumerateSubDocuments(GetAndUnsuppressSubDocuments, &documents); - for (PRInt32 i = 0; i < documents.Count(); ++i) { - if (!documents[i]->EventHandlingSuppressed()) { - nsPresShellIterator iter(documents[i]); - nsCOMPtr shell; - while ((shell = iter.GetNextShell())) { - shell->FireOrClearDelayedEvents(aFireEvents); - } - } + + if (aFireEvents) { + NS_DispatchToCurrentThread(new nsDelayedEventDispatcher(documents)); + } else { + FireOrClearDelayedEvents(documents, PR_FALSE); } } diff --git a/content/base/src/nsXMLHttpRequest.cpp b/content/base/src/nsXMLHttpRequest.cpp index c2ce3295b84..2fecb30f792 100644 --- a/content/base/src/nsXMLHttpRequest.cpp +++ b/content/base/src/nsXMLHttpRequest.cpp @@ -2829,9 +2829,7 @@ nsXMLHttpRequest::Send(nsIVariant *aBody) } if (suspendedDoc) { - NS_DispatchToCurrentThread( - NS_NEW_RUNNABLE_METHOD(nsIDocument, suspendedDoc.get(), - UnsuppressEventHandling)); + suspendedDoc->UnsuppressEventHandlingAndFireEvents(PR_TRUE); } if (resumeTimeoutRunnable) { diff --git a/content/base/test/test_bug333198.html b/content/base/test/test_bug333198.html index 573df31ee34..4cd096eaaa4 100644 --- a/content/base/test/test_bug333198.html +++ b/content/base/test/test_bug333198.html @@ -21,6 +21,9 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=333198 /** Test for Bug 333198 **/ +var focusTester; +var focusTester2; +var focusCount = 0; var eventCount = 0; function clickHandler() { ++eventCount; @@ -48,6 +51,9 @@ function sendEvents() { } function runTest() { + window.focus(); + focusTester = document.getElementsByTagName("input")[0]; + focusTester.blur(); window.addEventListener("click", clickHandler, true); var ifr = document.getElementById("ifr") ifr.contentWindow.addEventListener("click", clickHandler, true); @@ -59,8 +65,20 @@ function runTest() { suppressEvents(false); sendEvents(); is(eventCount, 4, "Wrong event count(2)"); - if (eventCount != 4) - alert(eventCount); + + is(focusCount, 0, "Wrong focus count (1)"); + var xhr = new XMLHttpRequest(); + xhr.open("GET", window.location, false); + xhr.onload = function() { + focusTester.focus(); + is(focusCount, 1, "Wrong focus count (2)"); + focusTester.blur(); + } + xhr.send(); + + focusTester.focus(); + is(focusCount, 2, "Wrong focus count (3)"); + SimpleTest.finish(); } @@ -69,5 +87,6 @@ addLoadEvent(runTest); + diff --git a/content/events/src/nsEventStateManager.cpp b/content/events/src/nsEventStateManager.cpp index f77829c31e2..3c1994700b3 100644 --- a/content/events/src/nsEventStateManager.cpp +++ b/content/events/src/nsEventStateManager.cpp @@ -260,9 +260,36 @@ PrintDocTreeAll(nsIDocShellTreeItem* aItem) } #endif +static PRUint32 gMayNeedFocusSuppression = 0; + +class nsSuppressUserFocus +{ +public: + nsSuppressUserFocus() : mActive(PR_FALSE) {} + ~nsSuppressUserFocus() { + if (mActive) { + NS_ASSERTION(gMayNeedFocusSuppression, + "Trying to unsuppress too much!"); + --gMayNeedFocusSuppression; + } + } + void MaybeSuppress() + { + if (!mActive) { + mActive = PR_TRUE; + ++gMayNeedFocusSuppression; + } + } +private: + PRBool mActive; +}; + static nsIDocument* EventHandlingSuppressed(nsPIDOMEventTarget* aTarget) { + if (!gMayNeedFocusSuppression) { + return nsnull; + } nsCOMPtr node = do_QueryInterface(aTarget); nsCOMPtr doc; if (node) { @@ -992,6 +1019,8 @@ nsEventStateManager::PreHandleEvent(nsPresContext* aPresContext, nsMouseWheelTransaction::OnEvent(aEvent); + nsSuppressUserFocus userFocus; + switch (aEvent->message) { case NS_MOUSE_BUTTON_DOWN: switch (static_cast(aEvent)->button) { @@ -1084,6 +1113,7 @@ nsEventStateManager::PreHandleEvent(nsPresContext* aPresContext, #ifdef DEBUG_smaug printf("nsEventStateManager::PreHandleEvent, NS_GOTFOCUS \n"); #endif + userFocus.MaybeSuppress(); // This is called when a child widget has received focus. // We need to take care of sending a blur event for the previously // focused content and document, then dispatching a focus @@ -1248,6 +1278,7 @@ nsEventStateManager::PreHandleEvent(nsPresContext* aPresContext, #ifdef DEBUG_smaug printf("nsEventStateManager::PreHandleEvent, NS_LOSTFOCUS \n"); #endif + userFocus.MaybeSuppress(); // Hide the caret if it's visible. if (mPresContext) { nsIPresShell *presShell = mPresContext->GetPresShell(); @@ -1347,6 +1378,7 @@ nsEventStateManager::PreHandleEvent(nsPresContext* aPresContext, #ifdef DEBUG_smaug printf("nsEventStateManager::PreHandleEvent, NS_ACTIVATE \n"); #endif + userFocus.MaybeSuppress(); // If we have a focus controller, and if it has a focused window and a // focused element in its focus memory, then restore the focus to those // objects. @@ -1437,6 +1469,7 @@ nsEventStateManager::PreHandleEvent(nsPresContext* aPresContext, #ifdef DEBUG_smaug printf("nsEventStateManager::PreHandleEvent, NS_DEACTIVATE \n"); #endif + userFocus.MaybeSuppress(); EnsureDocument(aPresContext); nsIMEStateManager::OnDeactivate(aPresContext); diff --git a/dom/base/nsDOMWindowUtils.cpp b/dom/base/nsDOMWindowUtils.cpp index 85f5dd2c5fc..ac0d1576805 100644 --- a/dom/base/nsDOMWindowUtils.cpp +++ b/dom/base/nsDOMWindowUtils.cpp @@ -729,7 +729,7 @@ nsDOMWindowUtils::SuppressEventHandling(PRBool aSuppress) if (aSuppress) { doc->SuppressEventHandling(); } else { - doc->UnsuppressEventHandling(); + doc->UnsuppressEventHandlingAndFireEvents(PR_TRUE); } return NS_OK; } diff --git a/dom/base/nsGlobalWindow.cpp b/dom/base/nsGlobalWindow.cpp index 37fa5f80fc6..7f901370285 100644 --- a/dom/base/nsGlobalWindow.cpp +++ b/dom/base/nsGlobalWindow.cpp @@ -5732,16 +5732,7 @@ nsGlobalWindow::LeaveModalState() if (mSuspendedDoc) { nsCOMPtr currentDoc = do_QueryInterface(topWin->GetExtantDocument()); - if (currentDoc == mSuspendedDoc) { - NS_DispatchToCurrentThread( - NS_NEW_RUNNABLE_METHOD(nsIDocument, mSuspendedDoc.get(), - UnsuppressEventHandling)); - } else { - // Somehow the document was changed. - // Unsuppress event handling in the document but don't even - // try to fire events. - mSuspendedDoc->UnsuppressEventHandlingAndFireEvents(PR_FALSE); - } + mSuspendedDoc->UnsuppressEventHandlingAndFireEvents(currentDoc == mSuspendedDoc); mSuspendedDoc = nsnull; } }