Bug 484658 - element.focus() handing when called during event suppression, r+sr=jst

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()

content/base/src/nsDocument.cpp

@@ -7484,6 +7484,40 @@ nsDocument::SuppressEventHandling(PRUint32 aIncrease)
   EnumerateSubDocuments(SuppressEventHandlingInDocument, &aIncrease);
 }
 
+static void
+FireOrClearDelayedEvents(nsTArray<nsCOMPtr<nsIDocument> >& aDocuments,
+                         PRBool aFireEvents)
+{
+  for (PRUint32 i = 0; i < aDocuments.Length(); ++i) {
+    if (!aDocuments[i]->EventHandlingSuppressed()) {
+      nsPresShellIterator iter(aDocuments[i]);
+      nsCOMPtr<nsIPresShell> shell;
+      while ((shell = iter.GetNextShell())) {
+        shell->FireOrClearDelayedEvents(aFireEvents);
+      }
+    }
+  }
+}
+
+class nsDelayedEventDispatcher : public nsRunnable
+{
+public:
+  nsDelayedEventDispatcher(nsTArray<nsCOMPtr<nsIDocument> >& aDocuments)
+  {
+    mDocuments.SwapElements(aDocuments);
+  }
+  virtual ~nsDelayedEventDispatcher() {}
+
+  NS_IMETHOD Run()
+  {
+    FireOrClearDelayedEvents(mDocuments, PR_TRUE);
+    return NS_OK;
+  }
+
+private:
+  nsTArray<nsCOMPtr<nsIDocument> > mDocuments;
+};
+
 static PRBool
 GetAndUnsuppressSubDocuments(nsIDocument* aDocument, void* aData)
 {
@@ -7491,8 +7525,9 @@ GetAndUnsuppressSubDocuments(nsIDocument* aDocument, void* aData)
   if (suppression > 0) {
     static_cast<nsDocument*>(aDocument)->DecreaseEventSuppression();
   }
-  nsCOMArray<nsIDocument>* docs = static_cast<nsCOMArray<nsIDocument>* >(aData);
-  docs->AppendObject(aDocument);
+  nsTArray<nsCOMPtr<nsIDocument> >* docs =
+    static_cast<nsTArray<nsCOMPtr<nsIDocument> >* >(aData);
+  docs->AppendElement(aDocument);
   aDocument->EnumerateSubDocuments(GetAndUnsuppressSubDocuments, docs);
   return PR_TRUE;
 }
@@ -7503,17 +7538,15 @@ nsDocument::UnsuppressEventHandlingAndFireEvents(PRBool aFireEvents)
   if (mEventsSuppressed > 0) {
     --mEventsSuppressed;
   }
-  nsCOMArray<nsIDocument> documents;
-  documents.AppendObject(this);
+
+  nsTArray<nsCOMPtr<nsIDocument> > documents;
+  documents.AppendElement(this);
   EnumerateSubDocuments(GetAndUnsuppressSubDocuments, &documents);
-  for (PRInt32 i = 0; i < documents.Count(); ++i) {
-    if (!documents[i]->EventHandlingSuppressed()) {
-      nsPresShellIterator iter(documents[i]);
-      nsCOMPtr<nsIPresShell> shell;
-      while ((shell = iter.GetNextShell())) {
-        shell->FireOrClearDelayedEvents(aFireEvents);
-      }
-    }
+
+  if (aFireEvents) {
+    NS_DispatchToCurrentThread(new nsDelayedEventDispatcher(documents));
+  } else {
+    FireOrClearDelayedEvents(documents, PR_FALSE);
   }
 }
 

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) {

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);
 
 </script>
 </pre>
+<input type="text" onfocus="++focusCount;">
 </body>
 </html>

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<nsINode> node = do_QueryInterface(aTarget);
   nsCOMPtr<nsIDocument> 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<nsMouseEvent*>(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);

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;
 }

dom/base/nsGlobalWindow.cpp

@@ -5732,16 +5732,7 @@ nsGlobalWindow::LeaveModalState()
     if (mSuspendedDoc) {
       nsCOMPtr<nsIDocument> 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;
     }
   }