Bug 1282668 Get rid of nsIMEUpdatePreference::NOTIFY_SELECTION_CHANGE r?m_kato draft
authorMasayuki Nakano <masayuki@d-toybox.com>
Tue, 28 Jun 2016 15:23:12 +0900
changeset 381820 7257e3e91f76ca140e9944fd9c872a21a18c8b16
parent 381817 dedac63b419f7973c3ca26148d47e28fcf627e50
child 524042 58322447d292afa78c63ad6c956dee59d2880f9f
push id21563
push usermasayuki@d-toybox.com
push dateTue, 28 Jun 2016 09:57:04 +0000
reviewersm_kato
bugs1282668
milestone50.0a1
Bug 1282668 Get rid of nsIMEUpdatePreference::NOTIFY_SELECTION_CHANGE r?m_kato Currently, all widgets request selection change notifications to IMEContentObserver. Additionally, IMEContentObserver needs to listen selection changes for caching latest selection for eQuerySelectedText. Therefore, it doesn't make sense to keep defining nsIMEUpdatePreference::NOTIFY_SELECTION_CHANGE. If widgets didn't need selection change notifications, they could just ignore the unnecessary notifications. Note that all widgets don't need selection change notifications if a plugin has focus and IMEContentObserver cannot observe selection changes in the plugin. Therefore, if IMEContentObserver is initialized with a plugin, it shouldn't listen selection changes (and doesn't need to notify widgets of selection changes). MozReview-Commit-ID: FOVFFgA2nOz
dom/events/IMEContentObserver.cpp
dom/events/IMEContentObserver.h
widget/IMEData.h
widget/PuppetWidget.cpp
widget/android/nsWindow.cpp
widget/cocoa/nsChildView.mm
widget/gtk/IMContextWrapper.cpp
widget/windows/IMMHandler.cpp
widget/windows/TSFTextStore.cpp
--- a/dom/events/IMEContentObserver.cpp
+++ b/dom/events/IMEContentObserver.cpp
@@ -346,16 +346,18 @@ IMEContentObserver::InitWithEditor(nsPre
     return false;
   }
 
   mDocShell = aPresContext->GetDocShell();
   if (NS_WARN_IF(!mDocShell)) {
     return false;
   }
 
+  MOZ_ASSERT(!WasInitializedWithPlugin());
+
   return true;
 }
 
 bool
 IMEContentObserver::InitWithPlugin(nsPresContext* aPresContext,
                                    nsIContent* aContent)
 {
   if (NS_WARN_IF(!aContent) ||
@@ -381,19 +383,27 @@ IMEContentObserver::InitWithPlugin(nsPre
   mEditableNode = aContent;
   mRootContent = aContent;
 
   mDocShell = aPresContext->GetDocShell();
   if (NS_WARN_IF(!mDocShell)) {
     return false;
   }
 
+  MOZ_ASSERT(WasInitializedWithPlugin());
+
   return true;
 }
 
+bool
+IMEContentObserver::WasInitializedWithPlugin() const
+{
+  return mDocShell && !mEditor;
+}
+
 void
 IMEContentObserver::Clear()
 {
   mEditor = nullptr;
   mSelection = nullptr;
   mEditableNode = nullptr;
   mRootContent = nullptr;
   mDocShell = nullptr;
@@ -419,18 +429,20 @@ IMEContentObserver::ObserveEditableNode(
   }
 
   mIsObserving = true;
   if (mEditor) {
     mEditor->AddEditorObserver(this);
   }
 
   mUpdatePreference = mWidget->GetIMEUpdatePreference();
-  if (mUpdatePreference.WantSelectionChange()) {
-    // add selection change listener
+  if (!WasInitializedWithPlugin()) {
+    // Add selection change listener only when this starts to observe
+    // non-plugin content since we cannot detect selection changes in
+    // plugins.
     nsCOMPtr<nsISelectionPrivate> selPrivate(do_QueryInterface(mSelection));
     NS_ENSURE_TRUE_VOID(selPrivate);
     nsresult rv = selPrivate->AddSelectionListener(this);
     NS_ENSURE_SUCCESS_VOID(rv);
   }
 
   if (mUpdatePreference.WantTextChange()) {
     // add text change observer
@@ -489,17 +501,17 @@ IMEContentObserver::UnregisterObservers(
     return;
   }
   mIsObserving = false;
 
   if (mEditor) {
     mEditor->RemoveEditorObserver(this);
   }
 
-  if (mUpdatePreference.WantSelectionChange() && mSelection) {
+  if (mSelection) {
     nsCOMPtr<nsISelectionPrivate> selPrivate(do_QueryInterface(mSelection));
     if (selPrivate) {
       selPrivate->RemoveSelectionListener(this);
     }
     mSelectionData.Clear();
     mFocusedWidget = nullptr;
   }
 
@@ -1280,17 +1292,17 @@ IMEContentObserver::MaybeNotifyCompositi
   FlushMergeableNotifications();
 }
 
 bool
 IMEContentObserver::UpdateSelectionCache()
 {
   MOZ_ASSERT(IsSafeToNotifyIME());
 
-  if (!mUpdatePreference.WantSelectionChange()) {
+  if (WasInitializedWithPlugin()) {
     return false;
   }
 
   mSelectionData.ClearSelectionData();
 
   // XXX Cannot we cache some information for reducing the cost to compute
   //     selection offset and writing mode?
   WidgetQueryContentEvent selection(true, eQuerySelectedText, mWidget);
--- a/dom/events/IMEContentObserver.h
+++ b/dom/events/IMEContentObserver.h
@@ -89,16 +89,17 @@ public:
    *                    Otherwise, false.
    */
   bool MaybeReinitialize(nsIWidget* aWidget,
                          nsPresContext* aPresContext,
                          nsIContent* aContent,
                          nsIEditor* aEditor);
   bool IsManaging(nsPresContext* aPresContext, nsIContent* aContent) const;
   bool IsManaging(const TextComposition* aTextComposition) const;
+  bool WasInitializedWithPlugin() const;
   bool IsEditorHandlingEventForComposition() const;
   bool KeepAliveDuringDeactive() const
   {
     return mUpdatePreference.WantDuringDeactive();
   }
   nsIWidget* GetWidget() const { return mWidget; }
   nsIEditor* GetEditor() const { return mEditor; }
   void SuppressNotifyingIME();
@@ -181,18 +182,17 @@ private:
            mNeedsToNotifyIMEOfPositionChange ||
            mNeedsToNotifyIMEOfCompositionEventHandled;
   }
 
   /**
    * UpdateSelectionCache() updates mSelectionData with the latest selection.
    * This should be called only when IsSafeToNotifyIME() returns true.
    *
-   * Note that this does nothing if mUpdatePreference.WantSelectionChange()
-   * returns false.
+   * Note that this does nothing if WasInitializedWithPlugin() returns true.
    */
   bool UpdateSelectionCache();
 
   nsCOMPtr<nsIWidget> mWidget;
   // mFocusedWidget has the editor observed by the instance.  E.g., if the
   // focused editor is in XUL panel, this should be the widget of the panel.
   // On the other hand, mWidget is its parent which handles IME.
   nsCOMPtr<nsIWidget> mFocusedWidget;
--- a/widget/IMEData.h
+++ b/widget/IMEData.h
@@ -34,17 +34,16 @@ class WritingMode;
  */
 struct nsIMEUpdatePreference final
 {
   typedef uint8_t Notifications;
 
   enum : Notifications
   {
     NOTIFY_NOTHING                       = 0,
-    NOTIFY_SELECTION_CHANGE              = 1 << 0,
     NOTIFY_TEXT_CHANGE                   = 1 << 1,
     NOTIFY_POSITION_CHANGE               = 1 << 2,
     // NOTIFY_MOUSE_BUTTON_EVENT_ON_CHAR is used when mouse button is pressed
     // or released on a character in the focused editor.  The notification is
     // notified to IME as a mouse event.  If it's consumed by IME, NotifyIME()
     // returns NS_SUCCESS_EVENT_CONSUMED.  Otherwise, it returns NS_OK if it's
     // handled without any error.
     NOTIFY_MOUSE_BUTTON_EVENT_ON_CHAR    = 1 << 3,
@@ -76,34 +75,29 @@ struct nsIMEUpdatePreference final
     return nsIMEUpdatePreference(aOther.mWantUpdates | mWantUpdates);
   }
 
   void DontNotifyChangesCausedByComposition()
   {
     mWantUpdates &= ~DEFAULT_CONDITIONS_OF_NOTIFYING_CHANGES;
   }
 
-  bool WantSelectionChange() const
-  {
-    return !!(mWantUpdates & NOTIFY_SELECTION_CHANGE);
-  }
-
   bool WantTextChange() const
   {
     return !!(mWantUpdates & NOTIFY_TEXT_CHANGE);
   }
 
   bool WantPositionChanged() const
   {
     return !!(mWantUpdates & NOTIFY_POSITION_CHANGE);
   }
 
   bool WantChanges() const
   {
-    return WantSelectionChange() || WantTextChange();
+    return WantTextChange();
   }
 
   bool WantMouseButtonEventOnChar() const
   {
     return !!(mWantUpdates & NOTIFY_MOUSE_BUTTON_EVENT_ON_CHAR);
   }
 
   bool WantChangesCausedByComposition() const
--- a/widget/PuppetWidget.cpp
+++ b/widget/PuppetWidget.cpp
@@ -816,17 +816,16 @@ PuppetWidget::GetIMEUpdatePreference()
   if (mInputContext.mIMEState.mEnabled == IMEState::PLUGIN) {
     // But if a plugin has focus, we cannot receive text nor selection change
     // in the plugin.  Therefore, PuppetWidget needs to receive only position
     // change event for updating the editor rect cache.
     return nsIMEUpdatePreference(mIMEPreferenceOfParent.mWantUpdates |
                                  nsIMEUpdatePreference::NOTIFY_POSITION_CHANGE);
   }
   return nsIMEUpdatePreference(mIMEPreferenceOfParent.mWantUpdates |
-                               nsIMEUpdatePreference::NOTIFY_SELECTION_CHANGE |
                                nsIMEUpdatePreference::NOTIFY_TEXT_CHANGE |
                                nsIMEUpdatePreference::NOTIFY_POSITION_CHANGE );
 #else
   // B2G doesn't handle IME as widget-level.
   return nsIMEUpdatePreference();
 #endif
 }
 
@@ -892,19 +891,18 @@ PuppetWidget::NotifyIMEOfSelectionChange
   // Therefore, we don't need to query text content again here.
   mContentCache.SetSelection(
     this, 
     aIMENotification.mSelectionChangeData.mOffset,
     aIMENotification.mSelectionChangeData.Length(),
     aIMENotification.mSelectionChangeData.mReversed,
     aIMENotification.mSelectionChangeData.GetWritingMode());
 
-  if (mIMEPreferenceOfParent.WantSelectionChange() &&
-      (mIMEPreferenceOfParent.WantChangesCausedByComposition() ||
-       !aIMENotification.mSelectionChangeData.mCausedByComposition)) {
+  if (mIMEPreferenceOfParent.WantChangesCausedByComposition() ||
+      !aIMENotification.mSelectionChangeData.mCausedByComposition) {
     mTabChild->SendNotifyIMESelection(mContentCache, aIMENotification);
   } else {
     mTabChild->SendUpdateContentCache(mContentCache);
   }
   return NS_OK;
 }
 
 nsresult
--- a/widget/android/nsWindow.cpp
+++ b/widget/android/nsWindow.cpp
@@ -3364,19 +3364,17 @@ nsWindow::GetInputContext()
 nsIMEUpdatePreference
 nsWindow::GetIMEUpdatePreference()
 {
     // While a plugin has focus, nsWindow for Android doesn't need any
     // notifications.
     if (GetInputContext().mIMEState.mEnabled == IMEState::PLUGIN) {
       return nsIMEUpdatePreference();
     }
-    return nsIMEUpdatePreference(
-        nsIMEUpdatePreference::NOTIFY_SELECTION_CHANGE |
-        nsIMEUpdatePreference::NOTIFY_TEXT_CHANGE);
+    return nsIMEUpdatePreference(nsIMEUpdatePreference::NOTIFY_TEXT_CHANGE);
 }
 
 nsresult
 nsWindow::SynthesizeNativeTouchPoint(uint32_t aPointerId,
                                      TouchPointerState aPointerState,
                                      LayoutDeviceIntPoint aPoint,
                                      double aPointerPressure,
                                      uint32_t aPointerOrientation,
--- a/widget/cocoa/nsChildView.mm
+++ b/widget/cocoa/nsChildView.mm
@@ -1929,21 +1929,20 @@ nsChildView::ExecuteNativeKeyBinding(Nat
 
   NativeKeyBindings* keyBindings = NativeKeyBindings::GetInstance(aType);
   return keyBindings->Execute(aEvent, aCallback, aCallbackData);
 }
 
 nsIMEUpdatePreference
 nsChildView::GetIMEUpdatePreference()
 {
-  // While a plugin has focus, IMEInputHandler doesn't need any notifications.
-  if (mInputContext.mIMEState.mEnabled == IMEState::PLUGIN) {
-    return nsIMEUpdatePreference();
-  }
-  return nsIMEUpdatePreference(nsIMEUpdatePreference::NOTIFY_SELECTION_CHANGE);
+  // XXX Shouldn't we move floating window which shows composition string
+  //     when plugin has focus and its parent is scrolled or the window is
+  //     moved?
+  return nsIMEUpdatePreference();
 }
 
 NSView<mozView>* nsChildView::GetEditorView()
 {
   NSView<mozView>* editorView = mView;
   // We need to get editor's view. E.g., when the focus is in the bookmark
   // dialog, the view is <panel> element of the dialog.  At this time, the key
   // events are processed the parent window's view that has native focus.
--- a/widget/gtk/IMContextWrapper.cpp
+++ b/widget/gtk/IMContextWrapper.cpp
@@ -328,17 +328,17 @@ IMContextWrapper::GetIMEUpdatePreference
 {
     // While a plugin has focus, IMContextWrapper doesn't need any
     // notifications.
     if (mInputContext.mIMEState.mEnabled == IMEState::PLUGIN) {
       return nsIMEUpdatePreference();
     }
 
     nsIMEUpdatePreference::Notifications notifications =
-        nsIMEUpdatePreference::NOTIFY_SELECTION_CHANGE;
+        nsIMEUpdatePreference::DEFAULT_CONDITIONS_OF_NOTIFYING_CHANGES;
     // If it's not enabled, we don't need position change notification.
     if (IsEnabled()) {
         notifications |= nsIMEUpdatePreference::NOTIFY_POSITION_CHANGE;
     }
     nsIMEUpdatePreference updatePreference(notifications);
     return updatePreference;
 }
 
--- a/widget/windows/IMMHandler.cpp
+++ b/widget/windows/IMMHandler.cpp
@@ -390,17 +390,16 @@ IMMHandler::GetKeyboardCodePage()
 }
 
 // static
 nsIMEUpdatePreference
 IMMHandler::GetIMEUpdatePreference()
 {
   return nsIMEUpdatePreference(
     nsIMEUpdatePreference::NOTIFY_POSITION_CHANGE |
-    nsIMEUpdatePreference::NOTIFY_SELECTION_CHANGE |
     nsIMEUpdatePreference::NOTIFY_MOUSE_BUTTON_EVENT_ON_CHAR);
 }
 
 // used for checking the lParam of WM_IME_COMPOSITION
 #define IS_COMPOSING_LPARAM(lParam) \
   ((lParam) & (GCS_COMPSTR | GCS_COMPATTR | GCS_COMPCLAUSE | GCS_CURSORPOS))
 #define IS_COMMITTING_LPARAM(lParam) ((lParam) & GCS_RESULTSTR)
 // Some IMEs (e.g., the standard IME for Korean) don't have caret position,
--- a/widget/windows/TSFTextStore.cpp
+++ b/widget/windows/TSFTextStore.cpp
@@ -4541,17 +4541,16 @@ TSFTextStore::CreateAndSetFocus(nsWindow
 nsIMEUpdatePreference
 TSFTextStore::GetIMEUpdatePreference()
 {
   if (sThreadMgr && sEnabledTextStore && sEnabledTextStore->mDocumentMgr) {
     RefPtr<ITfDocumentMgr> docMgr;
     sThreadMgr->GetFocus(getter_AddRefs(docMgr));
     if (docMgr == sEnabledTextStore->mDocumentMgr) {
       nsIMEUpdatePreference updatePreference(
-        nsIMEUpdatePreference::NOTIFY_SELECTION_CHANGE |
         nsIMEUpdatePreference::NOTIFY_TEXT_CHANGE |
         nsIMEUpdatePreference::NOTIFY_POSITION_CHANGE |
         nsIMEUpdatePreference::NOTIFY_MOUSE_BUTTON_EVENT_ON_CHAR |
         nsIMEUpdatePreference::NOTIFY_DURING_DEACTIVE);
       // TSFTextStore shouldn't notify TSF of selection change and text change
       // which are caused by composition.
       updatePreference.DontNotifyChangesCausedByComposition();
       return updatePreference;