Bug 1435530 - part 2: Make nsXBLWindowKeyHandler treat eAccessKeyNotFound as eKeyPress event r?enndeakin draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 05 Feb 2018 18:27:30 +0900
changeset 751145 ede53263cc5d849bbd3b5cd255d9dfbb1eaf0e7a
parent 751144 c44d2fbfb5f510a5b9f73e243a601ebfd7219766
child 751146 0cc089010f8a62e11015848a8b0dc40005ab31ee
push id97874
push usermasayuki@d-toybox.com
push dateMon, 05 Feb 2018 14:54:49 +0000
reviewersenndeakin
bugs1435530
milestone60.0a1
Bug 1435530 - part 2: Make nsXBLWindowKeyHandler treat eAccessKeyNotFound as eKeyPress event r?enndeakin Modifiers of shortcut keys may be same as modifier of content access keys. When focus is in the main process, such eKeyPress event is sent to remote content first. Then, and if it's not handled in the remote content, eAccessKeyNotFound is dispatched into the DOM tree in the main process. However, nsXBLWindowKeyHandler doesn't handle it as eKeyPress event. So, it causes that shortcut keys whose modifier conflicts with content access key won't be handled. This patch just makes nsXBLWindowKeyHandler treat eAccessKeyNotFound as eKeyPress event even though other shortcut keys which are handled by JS won't be executed. Perhaps, we should stop using eAccessKeyNotFound but it's too risky change for now. MozReview-Commit-ID: IJltg5gwBc5
dom/ipc/TabParent.cpp
dom/xbl/nsXBLWindowKeyHandler.cpp
--- a/dom/ipc/TabParent.cpp
+++ b/dom/ipc/TabParent.cpp
@@ -2239,16 +2239,17 @@ TabParent::RecvAccessKeyNotHandled(const
 
   // This is called only when this process had focus and HandleAccessKey
   // message was posted to all remote process and each remote process didn't
   // execute any content access keys.
   // XXX If there were two or more remote processes, this may be called
   //     twice or more for a keyboard event, that must be a bug.  But how to
   //     detect if received event has already been handled?
 
+  MOZ_ASSERT(aEvent.mMessage == eKeyPress);
   WidgetKeyboardEvent localEvent(aEvent);
   localEvent.MarkAsHandledInRemoteProcess();
   localEvent.mMessage = eAccessKeyNotFound;
 
   // Here we convert the WidgetEvent that we received to an nsIDOMEvent
   // to be able to dispatch it to the <browser> element as the target element.
   nsIDocument* doc = mFrameElement->OwnerDoc();
   nsIPresShell* presShell = doc->GetShell();
--- a/dom/xbl/nsXBLWindowKeyHandler.cpp
+++ b/dom/xbl/nsXBLWindowKeyHandler.cpp
@@ -351,16 +351,22 @@ nsXBLWindowKeyHandler::InstallKeyboardEv
                            this, NS_LITERAL_STRING("keydown"),
                            TrustedEventsAtSystemGroupBubble());
   aEventListenerManager->AddEventListenerByType(
                            this, NS_LITERAL_STRING("keyup"),
                            TrustedEventsAtSystemGroupBubble());
   aEventListenerManager->AddEventListenerByType(
                            this, NS_LITERAL_STRING("keypress"),
                            TrustedEventsAtSystemGroupBubble());
+  // mozaccesskeynotfound event is fired when modifiers of keypress event
+  // matches with modifier of content access key but it's not consumed by
+  // remote content.
+  aEventListenerManager->AddEventListenerByType(
+                           this, NS_LITERAL_STRING("mozaccesskeynotfound"),
+                           TrustedEventsAtSystemGroupBubble());
   aEventListenerManager->AddEventListenerByType(
                            this, NS_LITERAL_STRING("mozkeydownonplugin"),
                            TrustedEventsAtSystemGroupBubble());
   aEventListenerManager->AddEventListenerByType(
                            this, NS_LITERAL_STRING("mozkeyuponplugin"),
                            TrustedEventsAtSystemGroupBubble());
 }
 
@@ -405,16 +411,19 @@ nsXBLWindowKeyHandler::RemoveKeyboardEve
                            TrustedEventsAtSystemGroupBubble());
   aEventListenerManager->RemoveEventListenerByType(
                            this, NS_LITERAL_STRING("keyup"),
                            TrustedEventsAtSystemGroupBubble());
   aEventListenerManager->RemoveEventListenerByType(
                            this, NS_LITERAL_STRING("keypress"),
                            TrustedEventsAtSystemGroupBubble());
   aEventListenerManager->RemoveEventListenerByType(
+                           this, NS_LITERAL_STRING("mozaccesskeynotfound"),
+                           TrustedEventsAtSystemGroupBubble());
+  aEventListenerManager->RemoveEventListenerByType(
                            this, NS_LITERAL_STRING("mozkeydownonplugin"),
                            TrustedEventsAtSystemGroupBubble());
   aEventListenerManager->RemoveEventListenerByType(
                            this, NS_LITERAL_STRING("mozkeyuponplugin"),
                            TrustedEventsAtSystemGroupBubble());
 }
 
 /* static */ KeyboardMap
@@ -460,17 +469,23 @@ nsXBLWindowKeyHandler::ConvertEventToDOM
                          const WidgetKeyboardEvent& aWidgetKeyboardEvent) const
 {
   if (aWidgetKeyboardEvent.IsKeyDownOrKeyDownOnPlugin()) {
     return nsGkAtoms::keydown;
   }
   if (aWidgetKeyboardEvent.IsKeyUpOrKeyUpOnPlugin()) {
     return nsGkAtoms::keyup;
   }
-  if (aWidgetKeyboardEvent.mMessage == eKeyPress) {
+  // eAccessKeyNotFound event is always created from eKeyPress event and
+  // the original eKeyPress event has stopped its propagation before dispatched
+  // into the DOM tree in this process and not matched with remote content's
+  // access keys.  So, we should treat it as an eKeyPress event and execute
+  // a command if it's registered as a shortcut key.
+  if (aWidgetKeyboardEvent.mMessage == eKeyPress ||
+      aWidgetKeyboardEvent.mMessage == eAccessKeyNotFound) {
     return nsGkAtoms::keypress;
   }
   MOZ_ASSERT_UNREACHABLE(
     "All event messages which this instance listens to should be handled");
   return nullptr;
 }
 
 NS_IMETHODIMP