Bug 1502795 - Set keyCode or charCode of keypress event whose value is zero to the other's non-zero value by default again unless dispatched on known broken web apps r=smaug

This patch re-enables the new behavior of bug 1479964, to set keyCode or
charCode of keypress event whose value is zero to the other's non-zero value.

However, some web apps are still broken with the new behavior.  Therefore,
this patch adds a blacklist to keep using our legacy behavior in some specific
web apps.

Note that Google Docs, Gmail and Remember The Milk are reported as broken.
However, I don't see any broken shortcut with Gmail.  Therefore, this patch
adds only Google Docs and Remeber The Milk into the blacklist.

Differential Revision: https://phabricator.services.mozilla.com/D10322

--HG--
extra : moz-landing-system : lando

dom/events/KeyboardEvent.cpp

@@ -148,19 +148,23 @@ void KeyboardEvent::GetInitDict(KeyboardEventInit& aParam)
 
 bool
 KeyboardEvent::ShouldUseSameValueForCharCodeAndKeyCode(
+                 const WidgetKeyboardEvent& aWidgetKeyboardEvent,
                  CallerType aCallerType) const
 {
   // - If this event is initialized by JS, we don't need to return same value
   //   for keyCode and charCode since they can be initialized separately.
   // - If this is not a keypress event, we shouldn't return same value for
   //   keyCode and charCode.
+  // - If we need to return legacy keyCode and charCode values for the web
+  //   app due to in the blacklist.
   // - If this event is referred by default handler, i.e., the caller is
   //   system or this event is now in the system group, we don't need to use
   //   hack for web-compat.
   if (mInitializedByJS ||
-      mEvent->mMessage != eKeyPress ||
+      aWidgetKeyboardEvent.mMessage != eKeyPress ||
+      aWidgetKeyboardEvent.mUseLegacyKeyCodeAndCharCodeValues ||
       aCallerType == CallerType::System ||
-      mEvent->mFlags.mInSystemGroup) {
+      aWidgetKeyboardEvent.mFlags.mInSystemGroup) {
     return false;
   }
 
@@ -193,7 +197,8 @@ KeyboardEvent::CharCode(CallerType aCallerType)
   // value.
 
   if (widgetKeyboardEvent->mKeyNameIndex != KEY_NAME_INDEX_USE_STRING &&
-      ShouldUseSameValueForCharCodeAndKeyCode(aCallerType)) {
+      ShouldUseSameValueForCharCodeAndKeyCode(*widgetKeyboardEvent,
+                                              aCallerType)) {
     return ComputeTraditionalKeyCode(*widgetKeyboardEvent, aCallerType);
   }
 
@@ -226,7 +231,8 @@ KeyboardEvent::KeyCode(CallerType aCallerType)
   // for keyCode value if this is a "keypress" event.
 
   if (widgetKeyboardEvent->mKeyNameIndex == KEY_NAME_INDEX_USE_STRING &&
-      ShouldUseSameValueForCharCodeAndKeyCode(aCallerType)) {
+      ShouldUseSameValueForCharCodeAndKeyCode(*widgetKeyboardEvent,
+                                              aCallerType)) {
     return widgetKeyboardEvent->mCharCode;
   }
 

dom/events/KeyboardEvent.h

@@ -130,7 +130,9 @@ private:
    * ShouldUseSameValueForCharCodeAndKeyCode() returns true if KeyCode() and
    * CharCode() should return same value.
    */
-  bool ShouldUseSameValueForCharCodeAndKeyCode(CallerType aCallerType) const;
+  bool ShouldUseSameValueForCharCodeAndKeyCode(
+          const WidgetKeyboardEvent& aKeyboardEvent,
+          CallerType aCallerType) const;
 };
 
 } // namespace dom

layout/base/PresShell.cpp

@@ -826,7 +826,8 @@ PresShell::PresShell()
   , mHasHandledUserInput(false)
 #ifdef NIGHTLY_BUILD
   , mForceDispatchKeyPressEventsForNonPrintableKeys(false)
-  , mInitializedForceDispatchKeyPressEventsForNonPrintableKeys(false)
+  , mForceUseLegacyKeyCodeAndCharCodeValues(false)
+  , mInitializedWithKeyPressEventDispatchingBlacklist(false)
 #endif // #ifdef NIGHTLY_BUILD
 {
   MOZ_LOG(gLog, LogLevel::Debug, ("PresShell::PresShell this=%p", this));
@@ -7919,7 +7920,8 @@ GetDocumentURIToCompareWithBlacklist(PresShell& aPresShell)
 }
 
 static bool
-DispatchKeyPressEventsEvenForNonPrintableKeys(nsIURI* aURI)
+IsURIInBlacklistPref(nsIURI* aURI,
+                     const char* aBlacklistPrefName)
 {
   if (!aURI) {
     return false;
@@ -7940,11 +7942,8 @@ DispatchKeyPressEventsEvenForNonPrintableKeys(nsIURI* aURI)
 
   // The black list is comma separated domain list.  Each item may start with
   // "*.".  If starts with "*.", it matches any sub-domains.
-  static const char* kPrefNameOfBlackList =
-    "dom.keyboardevent.keypress.hack.dispatch_non_printable_keys";
-
   nsAutoCString blackList;
-  Preferences::GetCString(kPrefNameOfBlackList, blackList);
+  Preferences::GetCString(aBlacklistPrefName, blackList);
   if (blackList.IsEmpty()) {
     return false;
   }
@@ -8016,8 +8015,7 @@ PresShell::DispatchEventToDOM(WidgetEvent* aEvent,
     if (aEvent->IsBlockedForFingerprintingResistance()) {
       aEvent->mFlags.mOnlySystemGroupDispatchInContent = true;
 #ifdef NIGHTLY_BUILD
-    } else if (aEvent->mMessage == eKeyPress &&
-               aEvent->mFlags.mOnlySystemGroupDispatchInContent) {
+    } else if (aEvent->mMessage == eKeyPress) {
       // If eKeyPress event is marked as not dispatched in the default event
       // group in web content, it's caused by non-printable key or key
       // combination.  In this case, UI Events declares that browsers
@@ -8025,15 +8023,26 @@ PresShell::DispatchEventToDOM(WidgetEvent* aEvent,
       // broken with this strict behavior due to historical issue.
       // Therefore, we need to keep dispatching keypress event for such keys
       // even with breaking the standard.
-      if (!mInitializedForceDispatchKeyPressEventsForNonPrintableKeys) {
-        mInitializedForceDispatchKeyPressEventsForNonPrintableKeys = true;
+      // Similarly, the other browsers sets non-zero value of keyCode or
+      // charCode of keypress event to the other.  Therefore, we should
+      // behave so, however, some web apps may be broken.  On such web apps,
+      // we should keep using legacy our behavior.
+      if (!mInitializedWithKeyPressEventDispatchingBlacklist) {
+        mInitializedWithKeyPressEventDispatchingBlacklist = true;
         nsCOMPtr<nsIURI> uri = GetDocumentURIToCompareWithBlacklist(*this);
         mForceDispatchKeyPressEventsForNonPrintableKeys =
-          DispatchKeyPressEventsEvenForNonPrintableKeys(uri);
+          IsURIInBlacklistPref(uri,
+            "dom.keyboardevent.keypress.hack.dispatch_non_printable_keys");
+        mForceUseLegacyKeyCodeAndCharCodeValues =
+          IsURIInBlacklistPref(uri,
+            "dom.keyboardevent.keypress.hack.use_legacy_keycode_and_charcode");
       }
       if (mForceDispatchKeyPressEventsForNonPrintableKeys) {
         aEvent->mFlags.mOnlySystemGroupDispatchInContent = false;
       }
+      if (mForceUseLegacyKeyCodeAndCharCodeValues) {
+        aEvent->AsKeyboardEvent()->mUseLegacyKeyCodeAndCharCodeValues = true;
+      }
 #endif // #ifdef NIGHTLY_BUILD
     }
 

layout/base/PresShell.h

@@ -871,8 +871,14 @@ private:
   // Whether we should dispatch keypress events even for non-printable keys
   // for keeping backward compatibility.
   bool mForceDispatchKeyPressEventsForNonPrintableKeys : 1;
-  // Whether mForceDispatchKeyPressEventsForNonPrintableKeys is initialized.
-  bool mInitializedForceDispatchKeyPressEventsForNonPrintableKeys : 1;
+  // Whether we should set keyCode or charCode value of keypress events whose
+  // value is zero to the other value or not.  When this is set to true, we
+  // should keep using legacy keyCode and charCode values (i.e., one of them
+  // is always 0).
+  bool mForceUseLegacyKeyCodeAndCharCodeValues : 1;
+  // Whether mForceDispatchKeyPressEventsForNonPrintableKeys and
+  // mForceUseLegacyKeyCodeAndCharCodeValues are initialized.
+  bool mInitializedWithKeyPressEventDispatchingBlacklist : 1;
 #endif // #ifdef NIGHTLY_BUILD
 
   static bool sDisableNonTestMouseEvents;

modules/libpref/init/StaticPrefList.h

@@ -197,11 +197,17 @@ VARCACHE_PREF(
 
 // If this is true, "keypress" event's keyCode value and charCode value always
 // become same if the event is not created/initialized by JS.
+#ifdef NIGHTLY_BUILD
+# define PREF_VALUE  true
+#else
+# define PREF_VALUE  false
+#endif
 VARCACHE_PREF(
   "dom.keyboardevent.keypress.set_keycode_and_charcode_to_same_value",
    dom_keyboardevent_keypress_set_keycode_and_charcode_to_same_value,
-  bool, false
+  bool, PREF_VALUE
 )
+#undef PREF_VALUE
 
 // NOTE: This preference is used in unit tests. If it is removed or its default
 // value changes, please update test_sharedMap_var_caches.js accordingly.

modules/libpref/init/all.js

@@ -231,6 +231,16 @@ pref("dom.keyboardevent.keypress.hack.dispatch_non_printable_keys",
 pref("dom.keyboardevent.keypress.dispatch_non_printable_keys_only_system_group_in_content", false);
 #endif
 
+#ifdef NIGHTLY_BUILD
+// Blacklist of domains of web apps which handle keyCode and charCode of
+// keypress events with a path only for Firefox (i.e., broken if we set
+// non-zero keyCode or charCode value to the other).  The format is exactly
+// same as "dom.keyboardevent.keypress.hack.dispatch_non_printable_keys". So,
+// check its explanation for the detail.
+pref("dom.keyboardevent.keypress.hack.use_legacy_keycode_and_charcode",
+     "docs.google.com,www.rememberthemilk.com");
+#endif
+
 // Whether the WebMIDI API is enabled
 pref("dom.webmidi.enabled", false);
 

widget/TextEvents.h

@@ -166,6 +166,7 @@ protected:
     , mIsComposing(false)
     , mIsSynthesizedByTIP(false)
     , mMaybeSkippableInRemoteProcess(true)
+    , mUseLegacyKeyCodeAndCharCodeValues(false)
     , mEditCommandsForSingleLineEditorInitialized(false)
     , mEditCommandsForMultiLineEditorInitialized(false)
     , mEditCommandsForRichTextEditorInitialized(false)
@@ -195,6 +196,7 @@ public:
     , mIsComposing(false)
     , mIsSynthesizedByTIP(false)
     , mMaybeSkippableInRemoteProcess(true)
+    , mUseLegacyKeyCodeAndCharCodeValues(false)
     , mEditCommandsForSingleLineEditorInitialized(false)
     , mEditCommandsForMultiLineEditorInitialized(false)
     , mEditCommandsForRichTextEditorInitialized(false)
@@ -400,6 +402,10 @@ public:
   // Don't refer this member directly when you need to check this.
   // Use CanSkipInRemoteProcess() instead.
   bool mMaybeSkippableInRemoteProcess;
+  // Indicates whether the event should return legacy keyCode value and
+  // charCode value to web apps (one of them is always 0) or not, when it's
+  // an eKeyPress event.
+  bool mUseLegacyKeyCodeAndCharCodeValues;
 
   bool CanSkipInRemoteProcess() const
   {
@@ -681,6 +687,8 @@ public:
 #endif
     mIsSynthesizedByTIP = aEvent.mIsSynthesizedByTIP;
     mMaybeSkippableInRemoteProcess = aEvent.mMaybeSkippableInRemoteProcess;
+    mUseLegacyKeyCodeAndCharCodeValues =
+      aEvent.mUseLegacyKeyCodeAndCharCodeValues;
 
     // Don't copy mEditCommandsFor*Editor because it may require a lot of
     // memory space.  For example, if the event is dispatched but grabbed by