Bug 1434837 - Make autocomplete and satchel listen to keypress event at the system event group r=mak
authorMasayuki Nakano <masayuki@d-toybox.com>
Thu, 08 Feb 2018 22:42:29 +0900
changeset 404464 c0b4ca69376c1b8cd1c22a202100363631b5b9a6
parent 404463 8fa7c8455f707d4fa36a01469ae24af6ad0cde02
child 404465 5adb9a9328d7654b396f4d055855c2c8d22cd968
push id33479
push usershindli@mozilla.com
push dateTue, 20 Feb 2018 19:08:39 +0000
treeherdermozilla-central@ae61245586f7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmak
bugs1434837
milestone60.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 1434837 - Make autocomplete and satchel listen to keypress event at the system event group r=mak The autocomplete module listens to keypress event for both printable keys and non-printable keys a lot. However, we'll stop dispatching keypress event for non-printable keys in the default event group of web content. So, autocomplete should listen to keypress events at the system event group. Note that it's difficult to change keypress event listeners to keydown event listeners because if we stop keypress events at preceding keydown event in autocomplete or satchel module, some other modules fail to handle keydown or keypress event before autocomplete and it's not easy to investigate which module's which keypress event listener should be changed to keydown event listener. Therefore, this patch doesn't do it at least for now. MozReview-Commit-ID: 7e3aklmKrXu
toolkit/components/satchel/nsFormFillController.cpp
toolkit/components/satchel/test/test_popup_enter_event.html
toolkit/content/tests/chrome/test_autocomplete_mac_caret.xul
toolkit/content/tests/mochitest/test_autocomplete_change_after_focus.html
toolkit/content/widgets/autocomplete.xml
--- a/toolkit/components/satchel/nsFormFillController.cpp
+++ b/toolkit/components/satchel/nsFormFillController.cpp
@@ -3,16 +3,17 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "nsFormFillController.h"
 
 #include "mozilla/ClearOnShutdown.h"
 #include "mozilla/ErrorResult.h"
+#include "mozilla/EventListenerManager.h"
 #include "mozilla/dom/Element.h"
 #include "mozilla/dom/Event.h" // for nsIDOMEvent::InternalDOMEvent()
 #include "mozilla/dom/HTMLInputElement.h"
 #include "mozilla/dom/KeyboardEvent.h"
 #include "mozilla/dom/KeyboardEventBinding.h"
 #include "mozilla/dom/PageTransitionEvent.h"
 #include "mozilla/Logging.h"
 #include "nsIFormAutoComplete.h"
@@ -1263,33 +1264,39 @@ nsFormFillController::AddWindowListeners
   }
 
   EventTarget* target = aWindow->GetChromeEventHandler();
 
   if (!target) {
     return;
   }
 
-  target->AddEventListener(NS_LITERAL_STRING("focus"), this,
-                           true, false);
-  target->AddEventListener(NS_LITERAL_STRING("blur"), this,
-                           true, false);
-  target->AddEventListener(NS_LITERAL_STRING("pagehide"), this,
-                           true, false);
-  target->AddEventListener(NS_LITERAL_STRING("mousedown"), this,
-                           true, false);
-  target->AddEventListener(NS_LITERAL_STRING("input"), this,
-                           true, false);
-  target->AddEventListener(NS_LITERAL_STRING("keypress"), this, true, false);
-  target->AddEventListener(NS_LITERAL_STRING("compositionstart"), this,
-                           true, false);
-  target->AddEventListener(NS_LITERAL_STRING("compositionend"), this,
-                           true, false);
-  target->AddEventListener(NS_LITERAL_STRING("contextmenu"), this,
-                           true, false);
+  EventListenerManager* elm = target->GetOrCreateListenerManager();
+  if (NS_WARN_IF(!elm)) {
+    return;
+  }
+
+  elm->AddEventListenerByType(this, NS_LITERAL_STRING("focus"),
+                              TrustedEventsAtCapture());
+  elm->AddEventListenerByType(this, NS_LITERAL_STRING("blur"),
+                              TrustedEventsAtCapture());
+  elm->AddEventListenerByType(this, NS_LITERAL_STRING("pagehide"),
+                              TrustedEventsAtCapture());
+  elm->AddEventListenerByType(this, NS_LITERAL_STRING("mousedown"),
+                              TrustedEventsAtCapture());
+  elm->AddEventListenerByType(this, NS_LITERAL_STRING("input"),
+                              TrustedEventsAtCapture());
+  elm->AddEventListenerByType(this, NS_LITERAL_STRING("keypress"),
+                              TrustedEventsAtSystemGroupCapture());
+  elm->AddEventListenerByType(this, NS_LITERAL_STRING("compositionstart"),
+                              TrustedEventsAtCapture());
+  elm->AddEventListenerByType(this, NS_LITERAL_STRING("compositionend"),
+                              TrustedEventsAtCapture());
+  elm->AddEventListenerByType(this, NS_LITERAL_STRING("contextmenu"),
+                              TrustedEventsAtCapture());
 
   // Note that any additional listeners added should ensure that they ignore
   // untrusted events, which might be sent by content that's up to no good.
 }
 
 void
 nsFormFillController::RemoveWindowListeners(nsPIDOMWindowOuter* aWindow)
 {
@@ -1304,27 +1311,39 @@ nsFormFillController::RemoveWindowListen
   RemoveForDocument(doc);
 
   EventTarget* target = aWindow->GetChromeEventHandler();
 
   if (!target) {
     return;
   }
 
-  target->RemoveEventListener(NS_LITERAL_STRING("focus"), this, true);
-  target->RemoveEventListener(NS_LITERAL_STRING("blur"), this, true);
-  target->RemoveEventListener(NS_LITERAL_STRING("pagehide"), this, true);
-  target->RemoveEventListener(NS_LITERAL_STRING("mousedown"), this, true);
-  target->RemoveEventListener(NS_LITERAL_STRING("input"), this, true);
-  target->RemoveEventListener(NS_LITERAL_STRING("keypress"), this, true);
-  target->RemoveEventListener(NS_LITERAL_STRING("compositionstart"), this,
-                              true);
-  target->RemoveEventListener(NS_LITERAL_STRING("compositionend"), this,
-                              true);
-  target->RemoveEventListener(NS_LITERAL_STRING("contextmenu"), this, true);
+  EventListenerManager* elm = target->GetOrCreateListenerManager();
+  if (NS_WARN_IF(!elm)) {
+    return;
+  }
+
+  elm->RemoveEventListenerByType(this, NS_LITERAL_STRING("focus"),
+                                 TrustedEventsAtCapture());
+  elm->RemoveEventListenerByType(this, NS_LITERAL_STRING("blur"),
+                                 TrustedEventsAtCapture());
+  elm->RemoveEventListenerByType(this, NS_LITERAL_STRING("pagehide"),
+                                 TrustedEventsAtCapture());
+  elm->RemoveEventListenerByType(this, NS_LITERAL_STRING("mousedown"),
+                                 TrustedEventsAtCapture());
+  elm->RemoveEventListenerByType(this, NS_LITERAL_STRING("input"),
+                                 TrustedEventsAtCapture());
+  elm->RemoveEventListenerByType(this, NS_LITERAL_STRING("keypress"),
+                                 TrustedEventsAtSystemGroupCapture());
+  elm->RemoveEventListenerByType(this, NS_LITERAL_STRING("compositionstart"),
+                                 TrustedEventsAtCapture());
+  elm->RemoveEventListenerByType(this, NS_LITERAL_STRING("compositionend"),
+                                 TrustedEventsAtCapture());
+  elm->RemoveEventListenerByType(this, NS_LITERAL_STRING("contextmenu"),
+                                 TrustedEventsAtCapture());
 }
 
 void
 nsFormFillController::StartControllingInput(HTMLInputElement *aInput)
 {
   MOZ_LOG(sLogger, LogLevel::Verbose, ("StartControllingInput for %p", aInput));
   // Make sure we're not still attached to an input
   StopControllingInput();
--- a/toolkit/components/satchel/test/test_popup_enter_event.html
+++ b/toolkit/components/satchel/test/test_popup_enter_event.html
@@ -53,17 +53,17 @@ function handleEnter(evt) {
 
 function popupShownListener(evt) {
   synthesizeKey("KEY_ArrowDown");
   synthesizeKey("KEY_Enter"); // select the first entry in the popup
   synthesizeKey("KEY_Enter"); // try to submit the form with the filled value
 }
 
 function runTest() {
-  input.addEventListener("keypress", handleEnter, true);
+  SpecialPowers.addSystemEventListener(input, "keypress", handleEnter, true);
   form.addEventListener("submit", function submitCallback(evt) {
     is(input.value, expectedValue, "Check input value in the submit handler");
     evt.preventDefault();
 
     input.removeEventListener("keypress", handleEnter, true);
     form.removeEventListener("submit", submitCallback);
 
     SimpleTest.finish();
--- a/toolkit/content/tests/chrome/test_autocomplete_mac_caret.xul
+++ b/toolkit/content/tests/chrome/test_autocomplete_mac_caret.xul
@@ -18,48 +18,51 @@
 
 SimpleTest.waitForExplicitFinish();
 
 function keyCaretTest()
 {
   var autocomplete = $("autocomplete");
 
   autocomplete.focus();
-  checkKeyCaretTest("VK_UP", 0, 0, false, "no value up");
-  checkKeyCaretTest("VK_DOWN", 0, 0, false, "no value down");
+  checkKeyCaretTest("KEY_ArrowUp", 0, 0, false, "no value up");
+  checkKeyCaretTest("KEY_ArrowDown", 0, 0, false, "no value down");
 
   autocomplete.value = "Sample";
 
   autocomplete.selectionStart = 3;
   autocomplete.selectionEnd = 3;
-  checkKeyCaretTest("VK_UP", 0, 0, true, "value up with caret in middle");
-  checkKeyCaretTest("VK_UP", 0, 0, false, "value up with caret in middle again");
+  checkKeyCaretTest("KEY_ArrowUp", 0, 0, true, "value up with caret in middle");
+  checkKeyCaretTest("KEY_ArrowUp", 0, 0, false, "value up with caret in middle again");
 
   autocomplete.selectionStart = 2;
   autocomplete.selectionEnd = 2;
-  checkKeyCaretTest("VK_DOWN", 6, 6, true, "value down with caret in middle");
-  checkKeyCaretTest("VK_DOWN", 6, 6, false, "value down with caret in middle again");
+  checkKeyCaretTest("KEY_ArrowDown", 6, 6, true, "value down with caret in middle");
+  checkKeyCaretTest("KEY_ArrowDown", 6, 6, false, "value down with caret in middle again");
 
   autocomplete.selectionStart = 1;
   autocomplete.selectionEnd = 4;
-  checkKeyCaretTest("VK_UP", 0, 0, true, "value up with selection");
+  checkKeyCaretTest("KEY_ArrowUp", 0, 0, true, "value up with selection");
 
   autocomplete.selectionStart = 1;
   autocomplete.selectionEnd = 4;
-  checkKeyCaretTest("VK_DOWN", 6, 6, true, "value down with selection");
+  checkKeyCaretTest("KEY_ArrowDown", 6, 6, true, "value down with selection");
 
   SimpleTest.finish();
 }
 
 function checkKeyCaretTest(key, expectedStart, expectedEnd, result, testid)
 {
   var autocomplete = $("autocomplete");
-
-  var event = result ? "keypress" : "!keypress";
-  synthesizeKeyExpectEvent(key, { }, autocomplete.inputField, event, testid);
+  var keypressFired = false;
+  SpecialPowers.addSystemEventListener(autocomplete.inputField, "keypress", () => {
+    keypressFired = true;
+  }, {once: true});
+  synthesizeKey(key, {});
+  is(keypressFired, result, `${testid} keypress event should${result ? "" : " not"} be fired`);
   is(autocomplete.selectionStart, expectedStart, testid + " selectionStart");
   is(autocomplete.selectionEnd, expectedEnd, testid + " selectionEnd");
 }
 
 ]]>
 </script>
 
 <body xmlns="http://www.w3.org/1999/xhtml">
--- a/toolkit/content/tests/mochitest/test_autocomplete_change_after_focus.html
+++ b/toolkit/content/tests/mochitest/test_autocomplete_change_after_focus.html
@@ -74,17 +74,17 @@ https://bugzilla.mozilla.org/show_bug.cg
         if (evt.keyCode != KeyEvent.DOM_VK_RETURN) {
           return;
         }
         info("RETURN received for phase: " + evt.eventPhase);
         is(evt.target.value, "New value option", "Check that the correct autocomplete entry was used");
         resolve();
       }
 
-      field.addEventListener("keypress", handleEnter, true);
+      SpecialPowers.addSystemEventListener(field, "keypress", handleEnter, true);
     });
 
     field.focus();
 
     await promiseFieldFocus;
 
     await promisePopupShown;
 
--- a/toolkit/content/widgets/autocomplete.xml
+++ b/toolkit/content/widgets/autocomplete.xml
@@ -608,17 +608,17 @@
       </method>
     </implementation>
 
     <handlers>
       <handler event="input"><![CDATA[
         this.onInput(event);
       ]]></handler>
 
-      <handler event="keypress" phase="capturing"
+      <handler event="keypress" phase="capturing" group="system"
                action="return this.onKeyPress(event);"/>
 
       <handler event="compositionstart" phase="capturing"
                action="if (this.mController.input == this) this.mController.handleStartComposition();"/>
 
       <handler event="compositionend" phase="capturing"
                action="if (this.mController.input == this) this.mController.handleEndComposition();"/>