Bug 1583766 - Consider `IMEContentObserver` to send selection change notification when its sender sends a text change notification r=m_kato a=lizzard
authorMasayuki Nakano <masayuki@d-toybox.com>
Mon, 30 Sep 2019 03:59:09 +0000
changeset 552251 03a4de4097ad2d3c938f71c2d2c227d538fced61
parent 552250 6d321e762d12fe61f383e744cedfe7f19610b8c2
child 552252 384a31a31869a3896e0956ab41b9e37fa480b065
push id12108
push userncsoregi@mozilla.com
push dateFri, 04 Oct 2019 19:33:42 +0000
treeherdermozilla-beta@03a4de4097ad [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersm_kato, lizzard
bugs1583766
milestone70.0
Bug 1583766 - Consider `IMEContentObserver` to send selection change notification when its sender sends a text change notification r=m_kato a=lizzard The previous change is wrong because the sender shouldn't send selection change notification if `IMEContentObserver` does not receive text change notifications nor selection change notifications. This patch reverts the change in the if block of previous change and instead sets `IMEContentObserver::mNeedsToNotifyIMEOfSelectionChange` to `true` if the sender sends a text change notification. (Note that even if there is another text change with a mutation event listener, we need to send selection change notification later so that this must be the right approach.) Differential Revision: https://phabricator.services.mozilla.com/D47564
dom/events/IMEContentObserver.cpp
--- a/dom/events/IMEContentObserver.cpp
+++ b/dom/events/IMEContentObserver.cpp
@@ -1675,33 +1675,36 @@ IMEContentObserver::IMENotificationSende
     // anymore since IME starts to query content after it gets focus.
     observer->ClearPendingNotifications();
     return NS_OK;
   }
 
   if (observer->mNeedsToNotifyIMEOfTextChange) {
     observer->mNeedsToNotifyIMEOfTextChange = false;
     SendTextChange();
+    // Even if the observer hasn't received selection change, let's try to send
+    // selection change notification to IME because selection start offset may
+    // be changed if the previous contents of selection start are changed.  For
+    // example, when previous `<p>` element of another `<p>` element which
+    // contains caret is removed by a DOM mutation, selection change event
+    // won't be fired, but selection start offset should be decreased by the
+    // length of removed `<p>` element.
+    observer->mNeedsToNotifyIMEOfSelectionChange = true;
   }
 
   // If a text change notification causes another text change again, we should
   // notify IME of that before sending a selection change notification.
-  // Otherwise, even if the observer hasn't received selection change, let's
-  // try to send selection change notification to IME because selection
-  // start offset may be changed if the previous contents of selection start
-  // are changed.  For example, when previous `<p>` element of another `<p>`
-  // element which contains caret is removed by a DOM mutation, selection
-  // change event won't be fired, but selection start offset should be
-  // decreased by the length of removed `<p>` element.
   if (!observer->mNeedsToNotifyIMEOfTextChange) {
     // Be aware, PuppetWidget depends on the order of this. A selection change
     // notification should not be sent before a text change notification because
     // PuppetWidget shouldn't query new text content every selection change.
-    observer->mNeedsToNotifyIMEOfSelectionChange = false;
-    SendSelectionChange();
+    if (observer->mNeedsToNotifyIMEOfSelectionChange) {
+      observer->mNeedsToNotifyIMEOfSelectionChange = false;
+      SendSelectionChange();
+    }
   }
 
   // If a text change notification causes another text change again or a
   // selection change notification causes either a text change or another
   // selection change, we should notify IME of those before sending a position
   // change notification.
   if (!observer->mNeedsToNotifyIMEOfTextChange &&
       !observer->mNeedsToNotifyIMEOfSelectionChange) {