Bug 1203059 part.1 nsXBLWincowKeyHandler mark WidgetEvent::mFlags if it's reserved by chrome before the event is dispatched into the content r=smaug
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 16 Mar 2016 10:58:28 +0900
changeset 326757 f72453a034c39e94ed561851334c122bc26a3ffa
parent 326756 37f3e53ede1f3b3788b5060d49aa122337680d46
child 326758 4838a51a99b7717e0aa6464e83e100f1dc7311a9
push id6048
push userkmoir@mozilla.com
push dateMon, 06 Jun 2016 19:02:08 +0000
treeherdermozilla-beta@46d72a56c57d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerssmaug
bugs1203059
milestone48.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 1203059 part.1 nsXBLWincowKeyHandler mark WidgetEvent::mFlags if it's reserved by chrome before the event is dispatched into the content r=smaug
dom/xbl/nsXBLService.cpp
dom/xbl/nsXBLWindowKeyHandler.cpp
dom/xbl/nsXBLWindowKeyHandler.h
widget/BasicEvents.h
--- a/dom/xbl/nsXBLService.cpp
+++ b/dom/xbl/nsXBLService.cpp
@@ -559,16 +559,26 @@ nsXBLService::AttachGlobalKeyHandler(Eve
   // listen to these events
   manager->AddEventListenerByType(handler, NS_LITERAL_STRING("keydown"),
                                   TrustedEventsAtSystemGroupBubble());
   manager->AddEventListenerByType(handler, NS_LITERAL_STRING("keyup"),
                                   TrustedEventsAtSystemGroupBubble());
   manager->AddEventListenerByType(handler, NS_LITERAL_STRING("keypress"),
                                   TrustedEventsAtSystemGroupBubble());
 
+  // For marking each keyboard event as if it's reserved by chrome,
+  // nsXBLWindowKeyHandlers need to listen each keyboard events before
+  // web contents.
+  manager->AddEventListenerByType(handler, NS_LITERAL_STRING("keydown"),
+                                  TrustedEventsAtCapture());
+  manager->AddEventListenerByType(handler, NS_LITERAL_STRING("keyup"),
+                                  TrustedEventsAtCapture());
+  manager->AddEventListenerByType(handler, NS_LITERAL_STRING("keypress"),
+                                  TrustedEventsAtCapture());
+
   // The capturing listener is only used for XUL keysets to properly handle
   // shortcut keys in a multi-process environment.
   manager->AddEventListenerByType(handler, NS_LITERAL_STRING("keydown"),
                                   TrustedEventsAtSystemGroupCapture());
   manager->AddEventListenerByType(handler, NS_LITERAL_STRING("keyup"),
                                   TrustedEventsAtSystemGroupCapture());
   manager->AddEventListenerByType(handler, NS_LITERAL_STRING("keypress"),
                                   TrustedEventsAtSystemGroupCapture());
@@ -614,16 +624,23 @@ nsXBLService::DetachGlobalKeyHandler(Eve
   manager->RemoveEventListenerByType(handler, NS_LITERAL_STRING("keydown"),
                                      TrustedEventsAtSystemGroupBubble());
   manager->RemoveEventListenerByType(handler, NS_LITERAL_STRING("keyup"),
                                      TrustedEventsAtSystemGroupBubble());
   manager->RemoveEventListenerByType(handler, NS_LITERAL_STRING("keypress"),
                                      TrustedEventsAtSystemGroupBubble());
 
   manager->RemoveEventListenerByType(handler, NS_LITERAL_STRING("keydown"),
+                                     TrustedEventsAtCapture());
+  manager->RemoveEventListenerByType(handler, NS_LITERAL_STRING("keyup"),
+                                     TrustedEventsAtCapture());
+  manager->RemoveEventListenerByType(handler, NS_LITERAL_STRING("keypress"),
+                                     TrustedEventsAtCapture());
+
+  manager->RemoveEventListenerByType(handler, NS_LITERAL_STRING("keydown"),
                                      TrustedEventsAtSystemGroupCapture());
   manager->RemoveEventListenerByType(handler, NS_LITERAL_STRING("keyup"),
                                      TrustedEventsAtSystemGroupCapture());
   manager->RemoveEventListenerByType(handler, NS_LITERAL_STRING("keypress"),
                                      TrustedEventsAtSystemGroupCapture());
 
   contentNode->DeleteProperty(nsGkAtoms::listener);
 
--- a/dom/xbl/nsXBLWindowKeyHandler.cpp
+++ b/dom/xbl/nsXBLWindowKeyHandler.cpp
@@ -301,65 +301,90 @@ NS_IMETHODIMP
 nsXBLWindowKeyHandler::HandleEvent(nsIDOMEvent* aEvent)
 {
   nsCOMPtr<nsIDOMKeyEvent> keyEvent(do_QueryInterface(aEvent));
   NS_ENSURE_TRUE(keyEvent, NS_ERROR_INVALID_ARG);
 
   uint16_t eventPhase;
   aEvent->GetEventPhase(&eventPhase);
   if (eventPhase == nsIDOMEvent::CAPTURING_PHASE) {
-    HandleEventOnCapture(keyEvent);
+    if (aEvent->WidgetEventPtr()->mFlags.mInSystemGroup) {
+      HandleEventOnCaptureInSystemEventGroup(keyEvent);
+    } else {
+      HandleEventOnCaptureInDefaultEventGroup(keyEvent);
+    }
     return NS_OK;
   }
 
   nsAutoString eventType;
   aEvent->GetType(eventType);
   nsCOMPtr<nsIAtom> eventTypeAtom = do_GetAtom(eventType);
   NS_ENSURE_TRUE(eventTypeAtom, NS_ERROR_OUT_OF_MEMORY);
 
   return WalkHandlers(keyEvent, eventTypeAtom);
 }
 
 void
-nsXBLWindowKeyHandler::HandleEventOnCapture(nsIDOMKeyEvent* aEvent)
+nsXBLWindowKeyHandler::HandleEventOnCaptureInDefaultEventGroup(
+                         nsIDOMKeyEvent* aEvent)
+{
+  WidgetKeyboardEvent* widgetKeyboardEvent =
+    aEvent->AsEvent()->WidgetEventPtr()->AsKeyboardEvent();
+
+  if (widgetKeyboardEvent->mFlags.mOnlySystemGroupDispatchInContent) {
+    MOZ_RELEASE_ASSERT(
+      widgetKeyboardEvent->mFlags.mNoCrossProcessBoundaryForwarding);
+    return;
+  }
+
+  bool isReserved = false;
+  if (HasHandlerForEvent(aEvent, &isReserved) && isReserved) {
+    // For reserved commands (such as Open New Tab), we don't to wait for
+    // the content to answer (so mWantReplyFromContentProcess remains false),
+    // neither to give a chance for content to override its behavior.
+    widgetKeyboardEvent->mFlags.mNoCrossProcessBoundaryForwarding = true;
+    // If the key combination is reserved by chrome, we shouldn't expose the
+    // keyboard event to web contents because such keyboard events shouldn't be
+    // cancelable.  So, it's not good behavior to fire keyboard events but
+    // to ignore the defaultPrevented attribute value in chrome.
+    widgetKeyboardEvent->mFlags.mOnlySystemGroupDispatchInContent = true;
+  }
+}
+
+void
+nsXBLWindowKeyHandler::HandleEventOnCaptureInSystemEventGroup(
+                         nsIDOMKeyEvent* aEvent)
 {
   WidgetKeyboardEvent* widgetEvent =
     aEvent->AsEvent()->WidgetEventPtr()->AsKeyboardEvent();
 
-  if (widgetEvent->mFlags.mNoCrossProcessBoundaryForwarding) {
+  if (widgetEvent->mFlags.mNoCrossProcessBoundaryForwarding ||
+      widgetEvent->mFlags.mOnlySystemGroupDispatchInContent) {
     return;
   }
 
   nsCOMPtr<mozilla::dom::Element> originalTarget =
     do_QueryInterface(aEvent->AsEvent()->WidgetEventPtr()->originalTarget);
   if (!EventStateManager::IsRemoteTarget(originalTarget)) {
     return;
   }
 
-  bool aReservedForChrome = false;
-  if (!HasHandlerForEvent(aEvent, &aReservedForChrome)) {
+  if (!HasHandlerForEvent(aEvent)) {
     return;
   }
 
-  if (aReservedForChrome) {
-    // For reserved commands (such as Open New Tab), we don't to wait for
-    // the content to answer (so mWantReplyFromContentProcess remains false),
-    // neither to give a chance for content to override its behavior.
-    widgetEvent->mFlags.mNoCrossProcessBoundaryForwarding = true;
-  } else {
-    // Inform the child process that this is a event that we want a reply
-    // from.
-    widgetEvent->mFlags.mWantReplyFromContentProcess = true;
-
-    // If this event hadn't been marked as mNoCrossProcessBoundaryForwarding
-    // yet, it means it wasn't processed by content. We'll not call any
-    // of the handlers at this moment, and will wait for the event to be
-    // redispatched with mNoCrossProcessBoundaryForwarding = 1 to process it.
-    aEvent->AsEvent()->StopPropagation();
-  }
+  // Inform the child process that this is a event that we want a reply
+  // from.
+  widgetEvent->mFlags.mWantReplyFromContentProcess = true;
+  // If this event hadn't been marked as mNoCrossProcessBoundaryForwarding
+  // yet, it means it wasn't processed by content. We'll not call any
+  // of the handlers at this moment, and will wait for the event to be
+  // redispatched with mNoCrossProcessBoundaryForwarding = 1 to process it.
+  // XXX Why not StopImmediatePropagation()?
+  aEvent->AsEvent()->StopPropagation();
 }
 
 //
 // EventMatched
 //
 // See if the given handler cares about this particular key event
 //
 bool
--- a/dom/xbl/nsXBLWindowKeyHandler.h
+++ b/dom/xbl/nsXBLWindowKeyHandler.h
@@ -50,18 +50,20 @@ protected:
   // it if aExecute = true.
   bool WalkHandlersAndExecute(nsIDOMKeyEvent* aKeyEvent, nsIAtom* aEventType,
                               nsXBLPrototypeHandler* aHandler,
                               uint32_t aCharCode,
                               const IgnoreModifierState& aIgnoreModifierState,
                               bool aExecute,
                               bool* aOutReservedForChrome = nullptr);
 
-  // HandleEvent function for the capturing phase.
-  void HandleEventOnCapture(nsIDOMKeyEvent* aEvent);
+  // HandleEvent function for the capturing phase in the default event group.
+  void HandleEventOnCaptureInDefaultEventGroup(nsIDOMKeyEvent* aEvent);
+  // HandleEvent function for the capturing phase in the system event group.
+  void HandleEventOnCaptureInSystemEventGroup(nsIDOMKeyEvent* aEvent);
 
   // Check if any handler would handle the given event. Optionally returns
   // whether the command handler for the event is marked with the "reserved"
   // attribute.
   bool HasHandlerForEvent(nsIDOMKeyEvent* aEvent,
                           bool* aOutReservedForChrome = nullptr);
 
   // lazily load the handlers. Overridden to handle being attached
--- a/widget/BasicEvents.h
+++ b/widget/BasicEvents.h
@@ -98,19 +98,24 @@ public:
   bool    mNoCrossProcessBoundaryForwarding : 1;
   // If mNoContentDispatch is true, the event is never dispatched to the
   // event handlers which are added to the contents, onfoo attributes and
   // properties.  Note that this flag is ignored when
   // EventChainPreVisitor::mForceContentDispatch is set true.  For exapmle,
   // window and document object sets it true.  Therefore, web applications
   // can handle the event if they add event listeners to the window or the
   // document.
+  // XXX This is an ancient and broken feature, don't use this for new bug
+  //     as far as possible.
   bool    mNoContentDispatch : 1;
   // If mOnlyChromeDispatch is true, the event is dispatched to only chrome.
   bool    mOnlyChromeDispatch : 1;
+  // If mOnlySystemGroupDispatchInContent is true, event listeners added to
+  // the default group for non-chrome EventTarget won't be called.
+  bool    mOnlySystemGroupDispatchInContent : 1;
   // If mWantReplyFromContentProcess is true, the event will be redispatched
   // in the parent process after the content process has handled it. Useful
   // for when the parent process need the know first how the event was used
   // by content before handling it itself.
   bool mWantReplyFromContentProcess : 1;
   // The event's action will be handled by APZ. The main thread should not
   // perform its associated action. This is currently only relevant for
   // wheel and touch events.