Bug 1440189 - Stop dispatching keypress event to the default event group in web content (only Nightly and early Beta) unless web page isn't in blacklist r?smaug draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Fri, 23 Mar 2018 12:06:55 +0900
changeset 782018 c11d3968273ba7fbd5cc9536c48ad31f688c4ca2
parent 782003 6547c27303bc4d8961b11e656751e839807d65c7
push id106464
push usermasayuki@d-toybox.com
push dateSat, 14 Apr 2018 00:16:58 +0000
reviewerssmaug
bugs1440189
milestone61.0a1
Bug 1440189 - Stop dispatching keypress event to the default event group in web content (only Nightly and early Beta) unless web page isn't in blacklist r?smaug UI Events declares that keypress event should be fired only when the keydown sequence produces some characters. For conforming to UI Events and compatibility with the other browsers, we should stop dispatching keypress events for non-printable keys. For getting regression reports, we should enable this new behavior only on Nightly. However, some web apps actually broken with the standardized behavior. For protecting testers from known broken web apps, this patch introduces a blacklist to take the traditional behavior under specific domain (and path in it, optionally). Currently, docs.google.com and mail.google.com are set by default. MozReview-Commit-ID: HSrYX8LUB0p
layout/base/PresShell.cpp
layout/base/PresShell.h
modules/libpref/init/all.js
widget/TextEventDispatcher.cpp
--- a/layout/base/PresShell.cpp
+++ b/layout/base/PresShell.cpp
@@ -824,16 +824,20 @@ PresShell::PresShell()
   , mShouldUnsuppressPainting(false)
   , mApproximateFrameVisibilityVisited(false)
   , mNextPaintCompressed(false)
   , mHasCSSBackgroundColor(false)
   , mScaleToResolution(false)
   , mIsLastChromeOnlyEscapeKeyConsumed(false)
   , mHasReceivedPaintMessage(false)
   , mHasHandledUserInput(false)
+#ifdef NIGHTLY_BUILD
+  , mForceDispatchKeyPressEventsForNonPrintableKeys(false)
+  , mInitializedForceDispatchKeyPressEventsForNonPrintableKeys(false)
+#endif // #ifdef NIGHTLY_BUILD
 {
   MOZ_LOG(gLog, LogLevel::Debug, ("PresShell::PresShell this=%p", this));
 
 #ifdef MOZ_REFLOW_PERF
   mReflowCountMgr = new ReflowCountMgr();
   mReflowCountMgr->SetPresContext(mPresContext);
   mReflowCountMgr->SetPresShell(this);
 #endif
@@ -7807,17 +7811,113 @@ PresShell::HandleEventInternal(WidgetEve
       sLastInputCreated = aEvent->mTimeStamp;
     }
     sLastInputProcessed = now;
   }
 
   return rv;
 }
 
-
+#ifdef NIGHTLY_BUILD
+static already_AddRefed<nsIURI>
+GetDocumentURIToCompareWithBlacklist(PresShell& aPresShell)
+{
+  nsPresContext* presContext = aPresShell.GetPresContext();
+  if (NS_WARN_IF(!presContext)) {
+    return nullptr;
+  }
+  // If the document is sandboxed document or data: document, we should
+  // get URI of the parent document.
+  for (nsIDocument* document = presContext->Document();
+       document && document->IsContentDocument();
+       document = document->GetParentDocument()) {
+    // The document URI may be about:blank even if it comes from actual web
+    // site.  Therefore, we need to check the URI of its principal.
+    nsIPrincipal* principal = document->NodePrincipal();
+    if (principal->GetIsNullPrincipal()) {
+      continue;
+    }
+    nsCOMPtr<nsIURI> uri;
+    principal->GetURI(getter_AddRefs(uri));
+    return uri.forget();
+  }
+  return nullptr;
+}
+
+static bool
+DispatchKeyPressEventsEvenForNonPrintableKeys(nsIURI* aURI)
+{
+  if (!aURI) {
+    return false;
+  }
+
+  nsAutoCString scheme;
+  aURI->GetScheme(scheme);
+  if (!scheme.EqualsLiteral("http") &&
+      !scheme.EqualsLiteral("https")) {
+    return false;
+  }
+
+  nsAutoCString host;
+  aURI->GetHost(host);
+  if (host.IsEmpty()) {
+    return false;
+  }
+
+  // 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);
+  if (blackList.IsEmpty()) {
+    return false;
+  }
+
+  for (;;) {
+    int32_t index = blackList.Find(host, false);
+    if (index >= 0 &&
+        static_cast<uint32_t>(index) + host.Length() <= blackList.Length() &&
+        // If start of the black list or next to ","?
+        (!index || blackList[index - 1] == ',')) {
+      // If end of the black list or immediately before ","?
+      size_t indexAfterHost = index + host.Length();
+      if (indexAfterHost == blackList.Length() ||
+          blackList[indexAfterHost] == ',') {
+        return true;
+      }
+      // If next character is '/', we need to check the path too.
+      // We assume the path in blacklist means "/foo" + "*".
+      if (blackList[indexAfterHost] == '/') {
+        int32_t endOfPath = blackList.Find(",", false, indexAfterHost);
+        nsDependentCSubstring::size_type length =
+          endOfPath < 0 ? static_cast<nsDependentCSubstring::size_type>(-1) :
+                          endOfPath - indexAfterHost;
+        nsDependentCSubstring pathInBlackList(blackList,
+                                              indexAfterHost, length);
+        nsAutoCString filePath;
+        aURI->GetFilePath(filePath);
+        if (StringBeginsWith(filePath, pathInBlackList)) {
+          return true;
+        }
+      }
+    }
+    int32_t startIndexOfCurrentLevel = host[0] == '*' ? 1 : 0;
+    int32_t startIndexOfNextLevel =
+      host.Find(".", false, startIndexOfCurrentLevel + 1);
+    if (startIndexOfNextLevel <= 0) {
+      return false;
+    }
+    host = NS_LITERAL_CSTRING("*") +
+             nsDependentCSubstring(host, startIndexOfNextLevel);
+  }
+  return false;
+}
+#endif // #ifdef NIGHTLY_BUILD
 
 nsresult
 PresShell::DispatchEventToDOM(WidgetEvent* aEvent,
                               nsEventStatus* aStatus,
                               nsPresShellEventCB* aEventCB)
 {
   nsresult rv = NS_OK;
   nsCOMPtr<nsINode> eventTarget = mCurrentEventContent.get();
@@ -7835,16 +7935,36 @@ PresShell::DispatchEventToDOM(WidgetEven
       // If we don't have any content, the callback wouldn't probably
       // do nothing.
       eventCBPtr = nullptr;
     }
   }
   if (eventTarget) {
     if (aEvent->IsBlockedForFingerprintingResistance()) {
       aEvent->mFlags.mOnlySystemGroupDispatchInContent = true;
+#ifdef NIGHTLY_BUILD
+    } else if (aEvent->mMessage == eKeyPress &&
+               aEvent->mFlags.mOnlySystemGroupDispatchInContent) {
+      // 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
+      // shouldn't dispatch keypress event.  However, some web apps may be
+      // 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;
+        nsCOMPtr<nsIURI> uri = GetDocumentURIToCompareWithBlacklist(*this);
+        mForceDispatchKeyPressEventsForNonPrintableKeys =
+          DispatchKeyPressEventsEvenForNonPrintableKeys(uri);
+      }
+      if (mForceDispatchKeyPressEventsForNonPrintableKeys) {
+        aEvent->mFlags.mOnlySystemGroupDispatchInContent = false;
+      }
+#endif // #ifdef NIGHTLY_BUILD
     }
 
     if (aEvent->mClass == eCompositionEventClass) {
       IMEStateManager::DispatchCompositionEvent(eventTarget, mPresContext,
                                                 aEvent->AsCompositionEvent(),
                                                 aStatus, eventCBPtr);
     } else {
       EventDispatcher::Dispatch(eventTarget, mPresContext,
--- a/layout/base/PresShell.h
+++ b/layout/base/PresShell.h
@@ -867,16 +867,24 @@ private:
   // Whether the widget has received a paint message yet.
   bool mHasReceivedPaintMessage : 1;
 
   bool mIsLastKeyDownCanceled : 1;
 
   // Whether we have ever handled a user input event
   bool mHasHandledUserInput : 1;
 
+#ifdef NIGHTLY_BUILD
+  // 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;
+#endif // #ifdef NIGHTLY_BUILD
+
   static bool sDisableNonTestMouseEvents;
 
   TimeStamp mLastOSWake;
 
   static TimeStamp sLastInputCreated;
   static TimeStamp sLastInputProcessed;
 
   static bool sProcessInteractable;
--- a/modules/libpref/init/all.js
+++ b/modules/libpref/init/all.js
@@ -222,17 +222,29 @@ pref("dom.gamepad.haptic_feedback.enable
 pref("dom.keyboardevent.dispatch_during_composition", true);
 #else
 pref("dom.keyboardevent.dispatch_during_composition", false);
 #endif
 
 // If this is true, TextEventDispatcher dispatches keypress event with setting
 // WidgetEvent::mFlags::mOnlySystemGroupDispatchInContent to true if it won't
 // cause inputting printable character.
+#ifdef NIGHTLY_BUILD
+pref("dom.keyboardevent.keypress.dispatch_non_printable_keys_only_system_group_in_content", true);
+// Blacklist of domains of web apps which are not aware of strict keypress
+// dispatching behavior.  This is comma separated list.  If you need to match
+// all sub-domains, you can specify it as "*.example.com".  Additionally, you
+// can limit the path.  E.g., "example.com/foo" means "example.com/foo*".  So,
+// if you need to limit under a directory, the path should end with "/" like
+// "example.com/foo/".  Note that this cannot limit port number for now.
+pref("dom.keyboardevent.keypress.hack.dispatch_non_printable_keys",
+     "docs.google.com,mail.google.com");
+#else
 pref("dom.keyboardevent.keypress.dispatch_non_printable_keys_only_system_group_in_content", false);
+#endif
 
 // Whether the WebMIDI API is enabled
 pref("dom.webmidi.enabled", false);
 
 // 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
 // bytecode would be loaded from the cache instead of being generated once more.
--- a/widget/TextEventDispatcher.cpp
+++ b/widget/TextEventDispatcher.cpp
@@ -40,17 +40,17 @@ TextEventDispatcher::TextEventDispatcher
     Preferences::AddBoolVarCache(
       &sDispatchKeyEventsDuringComposition,
       "dom.keyboardevent.dispatch_during_composition",
       true);
     Preferences::AddBoolVarCache(
       &sDispatchKeyPressEventsOnlySystemGroupInContent,
       "dom.keyboardevent.keypress."
         "dispatch_non_printable_keys_only_system_group_in_content",
-      false);
+      true);
     sInitialized = true;
   }
 
   ClearNotificationRequests();
 }
 
 nsresult
 TextEventDispatcher::BeginInputTransaction(
@@ -692,16 +692,18 @@ TextEventDispatcher::DispatchKeyboardEve
       MOZ_ASSERT(keyEvent.mCodeValue ==
                    static_cast<WidgetKeyboardEvent&>(original).mCodeValue);
     }
   }
 
   if (sDispatchKeyPressEventsOnlySystemGroupInContent &&
       keyEvent.mMessage == eKeyPress &&
       !keyEvent.ShouldKeyPressEventBeFiredOnContent()) {
+    // Note that even if we set it to true, this may be overwritten by
+    // PresShell::DispatchEventToDOM().
     keyEvent.mFlags.mOnlySystemGroupDispatchInContent = true;
   }
 
   DispatchInputEvent(mWidget, keyEvent, aStatus);
   return true;
 }
 
 bool