Bug 1316330 - Cancel delayed keypress events if last keydown was canceled. r=smaug, a=lizzard
authorJessica Jong <jjong@mozilla.com>
Fri, 03 Feb 2017 05:16:00 -0500
changeset 376067 13ef12e80f26e0c1cec5e862fe2faa9940306c7d
parent 376066 81b9af9143f37eb828a8dc744c0c0793739a0fa5
child 376068 63752d3df9fa09ecd3283eaccb950cc5607fbab1
push id6996
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 20:48:21 +0000
treeherdermozilla-beta@d89512dab048 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug, lizzard
bugs1316330
milestone53.0a2
Bug 1316330 - Cancel delayed keypress events if last keydown was canceled. r=smaug, a=lizzard
dom/base/nsFocusManager.cpp
dom/base/nsGlobalWindow.cpp
dom/tests/browser/browser.ini
dom/tests/browser/browser_test_focus_after_modal_state.js
dom/tests/browser/focus_after_prompt.html
dom/xhr/XMLHttpRequestMainThread.cpp
dom/xhr/XMLHttpRequestMainThread.h
layout/base/PresShell.cpp
layout/base/PresShell.h
toolkit/components/passwordmgr/test/subtst_master_pass.html
--- a/dom/base/nsFocusManager.cpp
+++ b/dom/base/nsFocusManager.cpp
@@ -2122,25 +2122,17 @@ nsFocusManager::SendFocusOrBlurEvent(Eve
   // set aRelatedTarget to null if it's not in the same document as eventTarget
   if (eventTargetDoc != relatedTargetDoc) {
     aRelatedTarget = nullptr;
   }
 
   bool dontDispatchEvent =
     eventTargetDoc && nsContentUtils::IsUserFocusIgnored(eventTargetDoc);
 
-  // for focus events, if this event was from a mouse or key and event
-  // handling on the document is suppressed, queue the event and fire it
-  // later. For blur events, a non-zero value would be set for aFocusMethod.
-  if (aFocusMethod && !dontDispatchEvent &&
-      aDocument && aDocument->EventHandlingSuppressed()) {
-    // aFlags is always 0 when aWindowRaised is true so this won't be called
-    // on a window raise.
-    NS_ASSERTION(!aWindowRaised, "aWindowRaised should not be set");
-
+  if (!dontDispatchEvent && aDocument && aDocument->EventHandlingSuppressed()) {
     for (uint32_t i = mDelayedBlurFocusEvents.Length(); i > 0; --i) {
       // if this event was already queued, remove it and append it to the end
       if (mDelayedBlurFocusEvents[i - 1].mEventMessage == aEventMessage &&
           mDelayedBlurFocusEvents[i - 1].mPresShell == aPresShell &&
           mDelayedBlurFocusEvents[i - 1].mDocument == aDocument &&
           mDelayedBlurFocusEvents[i - 1].mTarget == eventTarget &&
           mDelayedBlurFocusEvents[i - 1].mRelatedTarget == aRelatedTarget) {
         mDelayedBlurFocusEvents.RemoveElementAt(i - 1);
--- a/dom/base/nsGlobalWindow.cpp
+++ b/dom/base/nsGlobalWindow.cpp
@@ -8940,17 +8940,17 @@ nsGlobalWindow::EnterModalState()
     nsIPresShell::SetCapturingContent(nullptr, 0);
   }
 
   if (topWin->mModalStateDepth == 0) {
     NS_ASSERTION(!topWin->mSuspendedDoc, "Shouldn't have mSuspendedDoc here!");
 
     topWin->mSuspendedDoc = topDoc;
     if (topDoc) {
-      topDoc->SuppressEventHandling(nsIDocument::eAnimationsOnly);
+      topDoc->SuppressEventHandling(nsIDocument::eEvents);
     }
 
     nsGlobalWindow* inner = topWin->GetCurrentInnerWindowInternal();
     if (inner) {
       topWin->GetCurrentInnerWindowInternal()->Suspend();
     }
   }
   topWin->mModalStateDepth++;
@@ -8977,17 +8977,17 @@ nsGlobalWindow::LeaveModalState()
 
   if (topWin->mModalStateDepth == 0) {
     if (inner) {
       inner->Resume();
     }
 
     if (topWin->mSuspendedDoc) {
       nsCOMPtr<nsIDocument> currentDoc = topWin->GetExtantDoc();
-      topWin->mSuspendedDoc->UnsuppressEventHandlingAndFireEvents(nsIDocument::eAnimationsOnly,
+      topWin->mSuspendedDoc->UnsuppressEventHandlingAndFireEvents(nsIDocument::eEvents,
                                                                   currentDoc == topWin->mSuspendedDoc);
       topWin->mSuspendedDoc = nullptr;
     }
   }
 
   // Remember the time of the last dialog quit.
   if (inner) {
     inner->mLastDialogQuitTime = TimeStamp::Now();
--- a/dom/tests/browser/browser.ini
+++ b/dom/tests/browser/browser.ini
@@ -23,30 +23,36 @@ skip-if = !e10s
 [browser_beforeunload_between_chrome_content.js]
 skip-if = !e10s
 [browser_bug396843.js]
 [browser_bug1004814.js]
 [browser_bug1008941_dismissGeolocationHanger.js]
 [browser_bug1238427.js]
 [browser_bug1316330.js]
 skip-if = !e10s
+[browser_cancel_keydown_keypress_event.js]
+support-files =
+  prevent_return_key.html
 [browser_ConsoleAPI_originAttributes.js]
 [browser_ConsoleAPITests.js]
 skip-if = e10s
 [browser_ConsoleStorageAPITests.js]
 [browser_ConsoleStoragePBTest_perwindowpb.js]
 [browser_focus_steal_from_chrome.js]
 [browser_focus_steal_from_chrome_during_mousedown.js]
 [browser_frame_elements.js]
 [browser_largeAllocation_win32.js]
 skip-if = !e10s || os != "win" || processor != "x86" # Large-Allocation requires e10s
 [browser_largeAllocation_non_win32.js]
 skip-if = !e10s || (os == "win" && processor == "x86") # Large-Allocation requires e10s
 [browser_localStorage_privatestorageevent.js]
 [browser_test__content.js]
+[browser_test_focus_after_modal_state.js]
+support-files =
+  focus_after_prompt.html
 [browser_test_new_window_from_content.js]
 tags = openwindow
 skip-if = toolkit == 'android'  || (os == "linux" && debug) # see bug 1261495 for Linux debug time outs
 support-files =
   test_new_window_from_content_child.html
 [browser_test_toolbars_visibility.js]
 support-files =
   test_new_window_from_content_child.html
new file mode 100644
--- /dev/null
+++ b/dom/tests/browser/browser_test_focus_after_modal_state.js
@@ -0,0 +1,69 @@
+const TEST_URL =
+  "https://example.com/browser/dom/tests/browser/focus_after_prompt.html";
+
+function awaitAndClosePrompt() {
+  return new Promise(resolve => {
+    function onDialogShown(node) {
+      Services.obs.removeObserver(onDialogShown, "tabmodal-dialog-loaded");
+      let button = node.ui.button0;
+      button.click();
+      resolve();
+    }
+    Services.obs.addObserver(onDialogShown, "tabmodal-dialog-loaded");
+  });
+}
+
+let lastMessageReceived = "";
+function waitForMessage(message) {
+  return new Promise((resolve, reject) => {
+    messageManager.addMessageListener(message, function() {
+      ok(true, "Received message: " + message);
+      lastMessageReceived = message;
+      resolve();
+    });
+  });
+}
+
+add_task(function* () {
+  let tab = yield BrowserTestUtils.openNewForegroundTab(gBrowser, TEST_URL);
+  let browser = tab.linkedBrowser;
+
+  // Focus on editable iframe.
+  yield BrowserTestUtils.synthesizeMouseAtCenter("#edit", {}, browser);
+  yield ContentTask.spawn(browser, null, function* () {
+    is(content.document.activeElement, content.document.getElementById("edit"),
+       "Focus should be on iframe element");
+  });
+
+  yield ContentTask.spawn(browser, null, function* () {
+    content.document.getElementById("edit").contentDocument.addEventListener(
+      "focus", function(event) {
+        sendAsyncMessage("Test:FocusReceived");
+      });
+
+    content.document.getElementById("edit").contentDocument.addEventListener(
+      "blur", function(event) {
+        sendAsyncMessage("Test:BlurReceived");
+      });
+  });
+
+  // Click on div that triggers a prompt, and then check that focus is back on
+  // the editable iframe.
+  let dialogShown = awaitAndClosePrompt();
+  let waitForBlur = waitForMessage("Test:BlurReceived");
+  let waitForFocus = waitForMessage("Test:FocusReceived");
+  yield ContentTask.spawn(tab.linkedBrowser, null, function*() {
+    let div = content.document.getElementById("clickMeDiv");
+    div.click();
+  });
+  yield dialogShown;
+  yield Promise.all([waitForBlur, waitForFocus]);
+  yield ContentTask.spawn(browser, null, function* () {
+    is(content.document.activeElement, content.document.getElementById("edit"),
+       "Focus should be back on iframe element");
+  });
+  is(lastMessageReceived, "Test:FocusReceived",
+     "Should receive blur and then focus event");
+
+  yield BrowserTestUtils.removeTab(tab);
+});
new file mode 100644
--- /dev/null
+++ b/dom/tests/browser/focus_after_prompt.html
@@ -0,0 +1,18 @@
+<!DOCTYPE html>
+<html>
+<head>
+  <meta charset="utf-8">
+  <title>Cursor should not be lost after prompt</title>
+  <script type="application/javascript">
+  function init() {
+    document.getElementById("edit").contentWindow.document.designMode = "on";
+  }
+  </script>
+</head>
+
+<body onload="init()">
+  <div id="clickMeDiv" onclick="prompt('This is a dummy prompt!');"
+                       onmousedown="return false;">Click me!</div>
+  <iframe id="edit"></iframe>
+</body>
+</html>
--- a/dom/xhr/XMLHttpRequestMainThread.cpp
+++ b/dom/xhr/XMLHttpRequestMainThread.cpp
@@ -2271,16 +2271,20 @@ XMLHttpRequestMainThread::ChangeStateToD
              "ChangeStateToDone() called before async HTML parsing is done.");
 
   mFlagSend = false;
 
   if (mTimeoutTimer) {
     mTimeoutTimer->Cancel();
   }
 
+  if (mFlagSynchronous) {
+    UnsuppressEventHandlingAndResume();
+  }
+
   // Per spec, fire the last download progress event, if any,
   // before readystatechange=4/done. (Note that 0-sized responses
   // will have not sent a progress event yet, so one must be sent here).
   if (!mFlagSynchronous &&
       (!mLoadTransferred || mProgressSinceLastProgressEvent)) {
     DispatchProgressEvent(this, ProgressEventType::progress,
                           mLoadTransferred, mLoadTotal);
     mProgressSinceLastProgressEvent = false;
@@ -2860,16 +2864,33 @@ XMLHttpRequestMainThread::Send(nsIVarian
 
   nsString string;
   string.Adopt(data, len);
 
   RequestBody<const nsAString> body(&string);
   return SendInternal(&body);
 }
 
+void
+XMLHttpRequestMainThread::UnsuppressEventHandlingAndResume()
+{
+  MOZ_ASSERT(mFlagSynchronous);
+
+  if (mSuspendedDoc) {
+    mSuspendedDoc->UnsuppressEventHandlingAndFireEvents(nsIDocument::eEvents,
+                                                         true);
+    mSuspendedDoc = nullptr;
+  }
+
+  if (mResumeTimeoutRunnable) {
+    NS_DispatchToCurrentThread(mResumeTimeoutRunnable);
+    mResumeTimeoutRunnable = nullptr;
+  }
+}
+
 nsresult
 XMLHttpRequestMainThread::SendInternal(const RequestBodyBase* aBody)
 {
   NS_ENSURE_TRUE(mPrincipal, NS_ERROR_NOT_INITIALIZED);
 
   // Step 1
   if (mState != State::opened) {
     return NS_ERROR_DOM_INVALID_STATE_XHR_MUST_BE_OPENED;
@@ -2986,65 +3007,56 @@ XMLHttpRequestMainThread::SendInternal(c
 
   // Step 8
   mFlagSend = true;
 
   // If we're synchronous, spin an event loop here and wait
   if (mFlagSynchronous) {
     mFlagSyncLooping = true;
 
-    nsCOMPtr<nsIDocument> suspendedDoc;
-    nsCOMPtr<nsIRunnable> resumeTimeoutRunnable;
     if (GetOwner()) {
       if (nsCOMPtr<nsPIDOMWindowOuter> topWindow = GetOwner()->GetOuterWindow()->GetTop()) {
         if (nsCOMPtr<nsPIDOMWindowInner> topInner = topWindow->GetCurrentInnerWindow()) {
-          suspendedDoc = topWindow->GetExtantDoc();
-          if (suspendedDoc) {
-            suspendedDoc->SuppressEventHandling(nsIDocument::eEvents);
+          mSuspendedDoc = topWindow->GetExtantDoc();
+          if (mSuspendedDoc) {
+            mSuspendedDoc->SuppressEventHandling(nsIDocument::eEvents);
           }
           topInner->Suspend();
-          resumeTimeoutRunnable = new nsResumeTimeoutsEvent(topInner);
+          mResumeTimeoutRunnable = new nsResumeTimeoutsEvent(topInner);
         }
       }
     }
 
     StopProgressEventTimer();
 
     SyncTimeoutType syncTimeoutType = MaybeStartSyncTimeoutTimer();
     if (syncTimeoutType == eErrorOrExpired) {
       Abort();
       rv = NS_ERROR_DOM_NETWORK_ERR;
     }
 
     if (NS_SUCCEEDED(rv)) {
-      nsAutoSyncOperation sync(suspendedDoc);
+      nsAutoSyncOperation sync(mSuspendedDoc);
       nsIThread *thread = NS_GetCurrentThread();
       while (mFlagSyncLooping) {
         if (!NS_ProcessNextEvent(thread)) {
           rv = NS_ERROR_UNEXPECTED;
           break;
         }
       }
 
       // Time expired... We should throw.
       if (syncTimeoutType == eTimerStarted && !mSyncTimeoutTimer) {
         rv = NS_ERROR_DOM_NETWORK_ERR;
       }
 
       CancelSyncTimeoutTimer();
     }
 
-    if (suspendedDoc) {
-      suspendedDoc->UnsuppressEventHandlingAndFireEvents(nsIDocument::eEvents,
-                                                         true);
-    }
-
-    if (resumeTimeoutRunnable) {
-      NS_DispatchToCurrentThread(resumeTimeoutRunnable);
-    }
+    UnsuppressEventHandlingAndResume();
   } else {
     // Now that we've successfully opened the channel, we can change state.  Note
     // that this needs to come after the AsyncOpen() and rv check, because this
     // can run script that would try to restart this request, and that could end
     // up doing our AsyncOpen on a null channel if the reentered AsyncOpen fails.
     StopProgressEventTimer();
 
     // Upload phase beginning; start the progress event timer if necessary.
--- a/dom/xhr/XMLHttpRequestMainThread.h
+++ b/dom/xhr/XMLHttpRequestMainThread.h
@@ -325,16 +325,18 @@ private:
   bool IsCrossSiteCORSRequest() const;
   bool IsDeniedCrossSiteCORSRequest();
 
   // Tell our channel what network interface ID we were told to use.
   // If it's an HTTP channel and we were told to use a non-default
   // interface ID.
   void PopulateNetworkInterfaceId();
 
+  void UnsuppressEventHandlingAndResume();
+
 public:
   virtual void
   Send(JSContext* /*aCx*/, ErrorResult& aRv) override
   {
     aRv = SendInternal(nullptr);
   }
 
   virtual void
@@ -717,16 +719,19 @@ protected:
 
   // Timeout support
   PRTime mRequestSentTime;
   uint32_t mTimeoutMilliseconds;
   nsCOMPtr<nsITimer> mTimeoutTimer;
   void StartTimeoutTimer();
   void HandleTimeoutCallback();
 
+  nsCOMPtr<nsIDocument> mSuspendedDoc;
+  nsCOMPtr<nsIRunnable> mResumeTimeoutRunnable;
+
   nsCOMPtr<nsITimer> mSyncTimeoutTimer;
 
   enum SyncTimeoutType {
     eErrorOrExpired,
     eTimerStarted,
     eNoTimerNeeded
   };
 
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -8065,16 +8065,19 @@ PresShell::HandleEventInternal(WidgetEve
           mIsLastChromeOnlyEscapeKeyConsumed = false;
         } else {
           if (aEvent->mFlags.mOnlyChromeDispatch &&
               aEvent->mFlags.mDefaultPreventedByChrome) {
             mIsLastChromeOnlyEscapeKeyConsumed = true;
           }
         }
       }
+      if (aEvent->mMessage == eKeyDown) {
+        mIsLastKeyDownCanceled = aEvent->mFlags.mDefaultPrevented;
+      }
       break;
     }
     case eMouseUp:
       // reset the capturing content now that the mouse button is up
       SetCapturingContent(nullptr, 0);
       break;
     case eMouseMove:
       nsIPresShell::AllowMouseCapture(false);
@@ -8854,16 +8857,19 @@ PresShell::FireOrClearDelayedEvents(bool
   }
 
   if (mDocument) {
     nsCOMPtr<nsIDocument> doc = mDocument;
     while (!mIsDestroying && mDelayedEvents.Length() &&
            !doc->EventHandlingSuppressed()) {
       nsAutoPtr<DelayedEvent> ev(mDelayedEvents[0].forget());
       mDelayedEvents.RemoveElementAt(0);
+      if (ev->IsKeyPressEvent() && mIsLastKeyDownCanceled) {
+        continue;
+      }
       ev->Dispatch();
     }
     if (!doc->EventHandlingSuppressed()) {
       mDelayedEvents.Clear();
     }
   }
 }
 
@@ -9661,16 +9667,22 @@ PresShell::DelayedKeyEvent::DelayedKeyEv
                             aEvent->mMessage,
                             aEvent->mWidget);
   keyEvent->AssignKeyEventData(*aEvent, false);
   keyEvent->mFlags.mIsSynthesizedForTests = aEvent->mFlags.mIsSynthesizedForTests;
   keyEvent->mFlags.mIsSuppressedOrDelayed = true;
   mEvent = keyEvent;
 }
 
+bool
+PresShell::DelayedKeyEvent::IsKeyPressEvent()
+{
+  return mEvent->mMessage == eKeyPress;
+}
+
 // Start of DEBUG only code
 
 #ifdef DEBUG
 
 static void
 LogVerifyMessage(nsIFrame* k1, nsIFrame* k2, const char* aMsg)
 {
   nsAutoString n1, n2;
--- a/layout/base/PresShell.h
+++ b/layout/base/PresShell.h
@@ -595,16 +595,17 @@ protected:
     return rv;
   }
 
   class DelayedEvent
   {
   public:
     virtual ~DelayedEvent() { }
     virtual void Dispatch() { }
+    virtual bool IsKeyPressEvent() { return false; }
   };
 
   class DelayedInputEvent : public DelayedEvent
   {
   public:
     virtual void Dispatch() override;
 
   protected:
@@ -619,16 +620,17 @@ protected:
   public:
     explicit DelayedMouseEvent(mozilla::WidgetMouseEvent* aEvent);
   };
 
   class DelayedKeyEvent : public DelayedInputEvent
   {
   public:
     explicit DelayedKeyEvent(mozilla::WidgetKeyboardEvent* aEvent);
+    virtual bool IsKeyPressEvent() override;
   };
 
   // Check if aEvent is a mouse event and record the mouse location for later
   // synth mouse moves.
   void RecordMouseLocation(mozilla::WidgetGUIEvent* aEvent);
   class nsSynthMouseMoveEvent final : public nsARefreshObserver {
   public:
     nsSynthMouseMoveEvent(PresShell* aPresShell, bool aFromScroll)
@@ -898,14 +900,16 @@ protected:
   bool                      mScaleToResolution : 1;
 
   // Whether the last chrome-only escape key event is consumed.
   bool                      mIsLastChromeOnlyEscapeKeyConsumed : 1;
 
   // Whether the widget has received a paint message yet.
   bool                      mHasReceivedPaintMessage : 1;
 
+  bool                      mIsLastKeyDownCanceled : 1;
+
   static bool               sDisableNonTestMouseEvents;
 };
 
 } // namespace mozilla
 
 #endif // mozilla_PresShell_h
--- a/toolkit/components/passwordmgr/test/subtst_master_pass.html
+++ b/toolkit/components/passwordmgr/test/subtst_master_pass.html
@@ -1,12 +1,7 @@
 <h2>MP subtest</h2>
 This form triggers a MP and gets filled in.<br>
 <form>
 Username: <input type="text"     id="userfield" name="u"><br>
-Password: <input type="password" id="passfield" name="p"><br>
-<script>
-    // Only notify when we fill in the password field.
-    document.getElementById("passfield").addEventListener("input", function() {
-        parent.postMessage("filled", "*");
-    });
-</script>
+Password: <input type="password" id="passfield" name="p"
+                 oninput="parent.postMessage('filled', '*');"><br>
 </form>