Bug 1438133 - Ctrl + Enter should cause keypress event even though the key combination doesn't input any character r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Wed, 14 Feb 2018 21:21:18 +0900
changeset 756822 66fea4efd87884e9ffd2381218b75e89acc05ab7
parent 756821 e7438140bb208aba474a74bdab428fc55962491c
child 756823 699f4c6027018dc7434245a079227d87a7f84855
push id99557
push usermasayuki@d-toybox.com
push dateSun, 18 Feb 2018 15:52:31 +0000
reviewerssmaug
bugs1438133
milestone60.0a1
Bug 1438133 - Ctrl + Enter should cause keypress event even though the key combination doesn't input any character r?smaug Currently, we dispatch keypress event when Enter is pressed without modifiers or only with Shift key. However, the other browsers dispatch keypress event for Ctrl + Enter in any platforms even if it doesn't cause any text input. So, we should fire keypress event for Ctrl + Enter even in strict keypress dispatching mode. Note that with other modifiers, it depends on browser and/or platform. So, anyway, web developers shouldn't use keypress event to catch Alt + Enter, Meta + Enter and two or more modifiers + Enter. MozReview-Commit-ID: 3uUMkhL5VfJ
dom/events/test/test_dom_keyboard_event.html
widget/TextEventDispatcher.cpp
widget/TextEvents.h
--- a/dom/events/test/test_dom_keyboard_event.html
+++ b/dom/events/test/test_dom_keyboard_event.html
@@ -3,16 +3,19 @@
 <head>
   <title>Test for DOM KeyboardEvent</title>
   <script type="application/javascript" src="/tests/SimpleTest/SimpleTest.js"></script>
   <script type="application/javascript" src="/tests/SimpleTest/EventUtils.js"></script>
   <link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
 </head>
 <body>
 <p id="display"></p>
+<p><input type="text" id="input"></p>
+<p><input type="text" id="input_readonly" readonly></p>
+<p><textarea id="textarea"></textarea></p>
 <div id="content" style="display: none">
   
 </div>
 <pre id="test">
 <script type="application/javascript">
 
 SimpleTest.waitForExplicitFinish();
 SimpleTest.waitForFocus(runTests, window);
@@ -372,19 +375,172 @@ function testSynthesizedKeyLocation()
     ok(events.keyup, description + "keyup event wasn't fired");
   }
 
   window.removeEventListener("keydown", handler, true);
   window.removeEventListener("keypress", handler, true);
   window.removeEventListener("keyup", handler, true);
 }
 
+// We're using TextEventDispatcher to decide if we should keypress event
+// on content in the default event group.  So, we can test if keypress
+// event is NOT fired unexpectedly with synthesizeKey().
+function testEnterKeyPressEvent()
+{
+  let keydownFired, keypressFired, beforeinputFired;
+  function onEvent(aEvent) {
+    switch (aEvent.type) {
+      case "keydown":
+        keydownFired = true;
+        return;
+      case "keypress":
+        keypressFired = true;
+        return;
+      case "beforeinput":
+        beforeinputFired = true;
+        return;
+    }
+  }
+
+  for (let targetId of ["input", "textarea", "input_readonly"]) {
+    let target = document.getElementById(targetId);
+
+    function reset() {
+      keydownFired = keypressFired = beforeinputFired = false;
+      target.value = "";
+    }
+
+    target.addEventListener("keydown", onEvent);
+    target.addEventListener("keypress", onEvent);
+    target.addEventListener("beforeinput", onEvent);
+
+    const kDescription = "<" + targetId.replace("_", " ") + ">: ";
+    let isEditable = kDescription.includes("readonly");
+    let isTextarea = kDescription.includes("textarea");
+
+    target.focus();
+
+    reset();
+    synthesizeKey("KEY_Enter", {});
+    is(keydownFired, true,
+       kDescription + "keydown event should be fired when Enter key is pressed");
+    is(keypressFired, true,
+       kDescription + "keypress event should be fired when Enter key is pressed");
+    if (isEditable) {
+      todo_is(beforeinputFired, true,
+              kDescription + "beforeinput event should be fired when Enter key is pressed");
+    } else {
+      is(beforeinputFired, false,
+         kDescription + "beforeinput event shouldn't be fired when Enter key is pressed");
+    }
+    if (isTextarea) {
+      is(target.value, "\n",
+         kDescription + "Enter key should cause inputting a line break in <textarea>");
+    } else {
+      is(target.value, "",
+         kDescription + "Enter key should not cause inputting a line break");
+    }
+
+    reset();
+    synthesizeKey("KEY_Enter", {shiftKey: true});
+    is(keydownFired, true,
+       kDescription + "keydown event should be fired when Shift + Enter key is pressed");
+    is(keypressFired, true,
+       kDescription + "keypress event should be fired when Shift + Enter key is pressed");
+    if (isEditable) {
+      todo_is(beforeinputFired, true,
+              kDescription + "beforeinput event should be fired when Shift + Enter key is pressed");
+    } else {
+      is(beforeinputFired, false,
+         kDescription + "beforeinput event shouldn't be fired when Shift + Enter key is pressed");
+    }
+    if (isTextarea) {
+      is(target.value, "\n",
+         kDescription + "Shift + Enter key should cause inputting a line break in <textarea>");
+    } else {
+      is(target.value, "",
+         kDescription + "Shift + Enter key should not cause inputting a line break");
+    }
+
+    reset();
+    synthesizeKey("KEY_Enter", {ctrlKey: true});
+    is(keydownFired, true,
+       kDescription + "keydown event should be fired when Ctrl + Enter key is pressed");
+    is(keypressFired, true,
+       kDescription + "keypress event should be fired when Ctrl + Enter key is pressed");
+    is(beforeinputFired, false,
+       kDescription + "beforeinput event shouldn't be fired when Ctrl + Enter key is pressed");
+    is(target.value, "",
+       kDescription + "Ctrl + Enter key should not cause inputting a line break");
+
+    reset();
+    synthesizeKey("KEY_Enter", {altKey: true});
+    is(keydownFired, true,
+       kDescription + "keydown event should be fired when Alt + Enter key is pressed");
+    is(keypressFired, !kStrictKeyPressEvents,
+       kDescription + "keypress event shouldn't be fired when Alt + Enter key is pressed in strict keypress dispatching mode");
+    is(beforeinputFired, false,
+       kDescription + "beforeinput event shouldn't be fired when Alt + Enter key is pressed");
+    is(target.value, "",
+       kDescription + "Alt + Enter key should not cause inputting a line break");
+
+    reset();
+    synthesizeKey("KEY_Enter", {metaKey: true});
+    is(keydownFired, true,
+       kDescription + "keydown event should be fired when Meta + Enter key is pressed");
+    is(keypressFired, !kStrictKeyPressEvents,
+       kDescription + "keypress event shouldn't be fired when Meta + Enter key is pressed in strict keypress dispatching mode");
+    is(beforeinputFired, false,
+       kDescription + "beforeinput event shouldn't be fired when Meta + Enter key is pressed");
+    is(target.value, "",
+       kDescription + "Meta + Enter key should not cause inputting a line break");
+
+    reset();
+    synthesizeKey("KEY_Enter", {shiftKey: true, ctrlKey: true});
+    is(keydownFired, true,
+       kDescription + "keydown event should be fired when Ctrl + Shift + Enter key is pressed");
+    is(keypressFired, !kStrictKeyPressEvents,
+       kDescription + "keypress event shouldn't be fired when Ctrl + Shift + Enter key is pressed in strict keypress dispatching mode");
+    is(beforeinputFired, false,
+       kDescription + "beforeinput event shouldn't be fired when Ctrl + Shift + Enter key is pressed");
+    is(target.value, "",
+       kDescription + "Ctrl + Shift + Enter key should not cause inputting a line break");
+
+    reset();
+    synthesizeKey("KEY_Enter", {shiftKey: true, altKey: true});
+    is(keydownFired, true,
+       kDescription + "keydown event should be fired when Alt + Shift + Enter key is pressed");
+    is(keypressFired, !kStrictKeyPressEvents,
+       kDescription + "keypress event shouldn't be fired when Alt + Shift + Enter key is pressed in strict keypress dispatching mode");
+    is(beforeinputFired, false,
+       kDescription + "beforeinput event shouldn't be fired when Alt + Shift + Enter key is pressed");
+    is(target.value, "",
+       kDescription + "Alt + Shift + Enter key should not cause inputting a line break");
+
+    reset();
+    synthesizeKey("KEY_Enter", {shiftKey: true, metaKey: true});
+    is(keydownFired, true,
+       kDescription + "keydown event should be fired when Meta + Shift + Enter key is pressed");
+    is(keypressFired, !kStrictKeyPressEvents,
+       kDescription + "keypress event shouldn't be fired when Meta + Shift + Enter key is pressed in strict keypress dispatching mode");
+    is(beforeinputFired, false,
+       kDescription + "beforeinput event shouldn't be fired when Meta + Shift + Enter key is pressed");
+    is(target.value, "",
+       kDescription + "Meta + Shift + Enter key should not cause inputting a line break");
+
+    target.removeEventListener("keydown", onEvent);
+    target.removeEventListener("keypress", onEvent);
+    target.removeEventListener("beforeinput", onEvent);
+  }
+}
+
 function runTests()
 {
   testInitializingUntrustedEvent();
   testSynthesizedKeyLocation();
+  testEnterKeyPressEvent();
   SimpleTest.finish();
 }
 
 </script>
 </pre>
 </body>
 </html>
--- a/widget/TextEventDispatcher.cpp
+++ b/widget/TextEventDispatcher.cpp
@@ -691,17 +691,17 @@ TextEventDispatcher::DispatchKeyboardEve
                    static_cast<WidgetKeyboardEvent&>(original).mKeyValue);
       MOZ_ASSERT(keyEvent.mCodeValue ==
                    static_cast<WidgetKeyboardEvent&>(original).mCodeValue);
     }
   }
 
   if (sDispatchKeyPressEventsOnlySystemGroupInContent &&
       keyEvent.mMessage == eKeyPress &&
-      !keyEvent.IsInputtingText() && !keyEvent.IsInputtingLineBreak()) {
+      !keyEvent.ShouldKeyPressEventBeFiredOnContent()) {
     keyEvent.mFlags.mOnlySystemGroupDispatchInContent = true;
   }
 
   DispatchInputEvent(mWidget, keyEvent, aStatus);
   return true;
 }
 
 bool
--- a/widget/TextEvents.h
+++ b/widget/TextEvents.h
@@ -246,16 +246,39 @@ public:
     return mMessage == eKeyPress &&
            mKeyNameIndex == KEY_NAME_INDEX_Enter &&
            !(mModifiers & (MODIFIER_ALT |
                            MODIFIER_CONTROL |
                            MODIFIER_META |
                            MODIFIER_OS));
   }
 
+  /**
+   * ShouldKeyPressEventBeFiredOnContent() should be called only when the
+   * instance is eKeyPress event.  This returns true when the eKeyPress
+   * event should be fired even on content in the default event group.
+   */
+  bool ShouldKeyPressEventBeFiredOnContent() const
+  {
+    MOZ_DIAGNOSTIC_ASSERT(mMessage == eKeyPress);
+    if (IsInputtingText() || IsInputtingLineBreak()) {
+      return true;
+    }
+    // Ctrl + Enter won't cause actual input in our editor.
+    // However, the other browsers fire keypress event in any platforms.
+    // So, for compatibility with them, we should fire keypress event for
+    // Ctrl + Enter too.
+    return mMessage == eKeyPress &&
+           mKeyNameIndex == KEY_NAME_INDEX_Enter &&
+           !(mModifiers & (MODIFIER_ALT |
+                           MODIFIER_META |
+                           MODIFIER_OS |
+                           MODIFIER_SHIFT));
+  }
+
   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);