Bug 1433101 - part 1: Add new pref which disables keypress event for non-printable keys only for the default event group in web content r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 25 Jan 2018 23:27:07 +0900
changeset 747409 e2bd30fc811755d4d74322b890c932242280141c
parent 737939 6602576987baec9edbaaad117114ba5227db6261
child 747410 77473715899e985c8baeee1893568fc334aa15e5
push id96905
push usermasayuki@d-toybox.com
push dateFri, 26 Jan 2018 01:54:19 +0000
reviewerssmaug
bugs1433101
milestone60.0a1
Bug 1433101 - part 1: Add new pref which disables keypress event for non-printable keys only for the default event group in web content r?smaug UI Events declares that keypress event should be fired when the keypress event causes some text input. However, we're keeping our traditional behavior for historical reasons because our internal event handlers (including event handlers of Thunderbird) handles keypress events for any keys. Therefore, for minimizing the side effect, we should stop kicking keypress event handlers in the default event group in web content. This patch adds new pref for enabling the standard behavior in web content. Additionally, creates WidgetKeyboardEvent::IsInputtingText() for sharing the check logic between TextEventDispatcher and TextEditor/HTMLEditor. MozReview-Commit-ID: 3rtXdLBPeVC
editor/libeditor/HTMLEditor.cpp
editor/libeditor/TextEditor.cpp
modules/libpref/init/all.js
widget/TextEventDispatcher.cpp
widget/TextEventDispatcher.h
widget/TextEvents.h
--- a/editor/libeditor/HTMLEditor.cpp
+++ b/editor/libeditor/HTMLEditor.cpp
@@ -674,21 +674,17 @@ HTMLEditor::HandleKeyPressEvent(WidgetKe
       if (aKeyboardEvent->IsShift()) {
         // only inserts a br node
         return TypedText(EmptyString(), eTypedBR);
       }
       // uses rules to figure out what to insert
       return TypedText(EmptyString(), eTypedBreak);
   }
 
-  // NOTE: On some keyboard layout, some characters are inputted with Control
-  // key or Alt key, but at that time, widget sets FALSE to these keys.
-  if (!aKeyboardEvent->mCharCode || aKeyboardEvent->IsControl() ||
-      aKeyboardEvent->IsAlt() || aKeyboardEvent->IsMeta() ||
-      aKeyboardEvent->IsOS()) {
+  if (!aKeyboardEvent->IsInputtingText()) {
     // we don't PreventDefault() here or keybindings like control-x won't work
     return NS_OK;
   }
   aKeyboardEvent->PreventDefault();
   nsAutoString str(aKeyboardEvent->mCharCode);
   return TypedText(str, eTypedText);
 }
 
--- a/editor/libeditor/TextEditor.cpp
+++ b/editor/libeditor/TextEditor.cpp
@@ -379,21 +379,17 @@ TextEditor::HandleKeyPressEvent(WidgetKe
           aKeyboardEvent->IsAlt() || aKeyboardEvent->IsMeta() ||
           aKeyboardEvent->IsOS()) {
         return NS_OK;
       }
       aKeyboardEvent->PreventDefault();
       return TypedText(EmptyString(), eTypedBreak);
   }
 
-  // NOTE: On some keyboard layout, some characters are inputted with Control
-  // key or Alt key, but at that time, widget sets FALSE to these keys.
-  if (!aKeyboardEvent->mCharCode || aKeyboardEvent->IsControl() ||
-      aKeyboardEvent->IsAlt() || aKeyboardEvent->IsMeta() ||
-      aKeyboardEvent->IsOS()) {
+  if (!aKeyboardEvent->IsInputtingText()) {
     // we don't PreventDefault() here or keybindings like control-x won't work
     return NS_OK;
   }
   aKeyboardEvent->PreventDefault();
   nsAutoString str(aKeyboardEvent->mCharCode);
   return TypedText(str, eTypedText);
 }
 
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -201,16 +201,21 @@ pref("dom.gamepad.non_standard_events.en
 pref("dom.gamepad.extensions.enabled", true);
 pref("dom.gamepad.haptic_feedback.enabled", true);
 
 // If this is true, TextEventDispatcher dispatches keydown and keyup events
 // even during composition (keypress events are never fired during composition
 // even if this is true).
 pref("dom.keyboardevent.dispatch_during_composition", false);
 
+// If this is true, TextEventDispatcher dispatches keypress event with setting
+// WidgetEvent::mFlags::mOnlySystemGroupDispatchInContent to true if it won't
+// cause inputting printable character.
+pref("dom.keyboardevent.keypress.dispatch_non_printable_keys_only_system_group_in_content", false);
+
 // Whether to run add-on code in different compartments from browser code. This
 // causes a separate compartment for each (addon, global) combination, which may
 // significantly increase the number of compartments in the system.
 pref("dom.compartment_per_addon", true);
 
 // Whether to enable the JavaScript start-up cache. This causes one of the first
 // execution to record the bytecode of the JavaScript function used, and save it
 // in the existing cache entry. On the following loads of the same script, the
--- a/widget/TextEventDispatcher.cpp
+++ b/widget/TextEventDispatcher.cpp
@@ -17,16 +17,18 @@
 namespace mozilla {
 namespace widget {
 
 /******************************************************************************
  * TextEventDispatcher
  *****************************************************************************/
 
 bool TextEventDispatcher::sDispatchKeyEventsDuringComposition = false;
+bool TextEventDispatcher::sDispatchKeyPressEventsOnlySystemGroupInContent =
+       false;
 
 TextEventDispatcher::TextEventDispatcher(nsIWidget* aWidget)
   : mWidget(aWidget)
   , mDispatchingEvent(0)
   , mInputTransactionType(eNoInputTransaction)
   , mIsComposing(false)
   , mIsHandlingComposition(false)
   , mHasFocus(false)
@@ -34,16 +36,21 @@ TextEventDispatcher::TextEventDispatcher
   MOZ_RELEASE_ASSERT(mWidget, "aWidget must not be nullptr");
 
   static bool sInitialized = false;
   if (!sInitialized) {
     Preferences::AddBoolVarCache(
       &sDispatchKeyEventsDuringComposition,
       "dom.keyboardevent.dispatch_during_composition",
       false);
+    Preferences::AddBoolVarCache(
+      &sDispatchKeyPressEventsOnlySystemGroupInContent,
+      "dom.keyboardevent.keypress."
+        "dispatch_non_printable_keys_only_system_group_in_content",
+      false);
     sInitialized = true;
   }
 
   ClearNotificationRequests();
 }
 
 nsresult
 TextEventDispatcher::BeginInputTransaction(
@@ -682,16 +689,22 @@ TextEventDispatcher::DispatchKeyboardEve
                    static_cast<WidgetKeyboardEvent&>(original).mCodeNameIndex);
       MOZ_ASSERT(keyEvent.mKeyValue ==
                    static_cast<WidgetKeyboardEvent&>(original).mKeyValue);
       MOZ_ASSERT(keyEvent.mCodeValue ==
                    static_cast<WidgetKeyboardEvent&>(original).mCodeValue);
     }
   }
 
+  if (sDispatchKeyPressEventsOnlySystemGroupInContent &&
+      keyEvent.mMessage == eKeyPress &&
+      !keyEvent.IsInputtingText()) {
+    keyEvent.mFlags.mOnlySystemGroupDispatchInContent = true;
+  }
+
   DispatchInputEvent(mWidget, keyEvent, aStatus);
   return true;
 }
 
 bool
 TextEventDispatcher::MaybeDispatchKeypressEvents(
                        const WidgetKeyboardEvent& aKeyboardEvent,
                        nsEventStatus& aStatus,
--- a/widget/TextEventDispatcher.h
+++ b/widget/TextEventDispatcher.h
@@ -449,16 +449,19 @@ private:
 
   // true while NOTIFY_IME_OF_FOCUS is received but NOTIFY_IME_OF_BLUR has not
   // received yet.  Otherwise, false.
   bool mHasFocus;
 
   // If this is true, keydown and keyup events are dispatched even when there
   // is a composition.
   static bool sDispatchKeyEventsDuringComposition;
+  // If this is true, keypress events for non-printable keys are dispatched only
+  // for event listeners of the system event group in web content.
+  static bool sDispatchKeyPressEventsOnlySystemGroupInContent;
 
   nsresult BeginInputTransactionInternal(
              TextEventDispatcherListener* aListener,
              InputTransactionType aType);
 
   /**
    * InitEvent() initializes aEvent.  This must be called before dispatching
    * the event.
--- a/widget/TextEvents.h
+++ b/widget/TextEvents.h
@@ -223,16 +223,29 @@ public:
   {
     return aMessage == eKeyDownOnPlugin || aMessage == eKeyUpOnPlugin;
   }
   bool IsKeyEventOnPlugin() const
   {
     return IsKeyEventOnPlugin(mMessage);
   }
 
+  bool IsInputtingText() const
+  {
+    // NOTE: On some keyboard layout, some characters are inputted with Control
+    //       key or Alt key, but at that time, widget unset the modifier flag
+    //       from eKeyPress event.
+    return mMessage == eKeyPress &&
+           mCharCode &&
+           !(mModifiers & (MODIFIER_ALT |
+                           MODIFIER_CONTROL |
+                           MODIFIER_META |
+                           MODIFIER_OS));
+  }
+
   virtual WidgetEvent* Duplicate() const override
   {
     MOZ_ASSERT(mClass == eKeyboardEventClass,
                "Duplicate() must be overridden by sub class");
     // Not copying widget, it is a weak reference.
     WidgetKeyboardEvent* result =
       new WidgetKeyboardEvent(false, mMessage, nullptr);
     result->AssignKeyEventData(*this, true);