From e1d8dafcd98f73b173a90a63eb8c255485ab5b5f Mon Sep 17 00:00:00 2001 From: Masayuki Nakano Date: Tue, 28 Jun 2016 15:23:12 +0900 Subject: [PATCH] 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 --HG-- extra : rebase_source : 3e16d5023835f99f82934e754d2e7db70474f9ee --- dom/events/IMEContentObserver.cpp | 20 ++++++++++++++++---- dom/events/IMEContentObserver.h | 4 ++-- widget/IMEData.h | 8 +------- widget/PuppetWidget.cpp | 6 ++---- widget/android/nsWindow.cpp | 4 +--- widget/cocoa/nsChildView.mm | 9 ++++----- widget/gtk/IMContextWrapper.cpp | 2 +- widget/windows/IMMHandler.cpp | 1 - widget/windows/TSFTextStore.cpp | 1 - 9 files changed, 27 insertions(+), 28 deletions(-) diff --git a/dom/events/IMEContentObserver.cpp b/dom/events/IMEContentObserver.cpp index 83b76163ac08..2363d1781ea8 100644 --- a/dom/events/IMEContentObserver.cpp +++ b/dom/events/IMEContentObserver.cpp @@ -351,6 +351,8 @@ IMEContentObserver::InitWithEditor(nsPresContext* aPresContext, return false; } + MOZ_ASSERT(!WasInitializedWithPlugin()); + return true; } @@ -386,9 +388,17 @@ IMEContentObserver::InitWithPlugin(nsPresContext* aPresContext, return false; } + MOZ_ASSERT(WasInitializedWithPlugin()); + return true; } +bool +IMEContentObserver::WasInitializedWithPlugin() const +{ + return mDocShell && !mEditor; +} + void IMEContentObserver::Clear() { @@ -424,8 +434,10 @@ IMEContentObserver::ObserveEditableNode() } 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 selPrivate(do_QueryInterface(mSelection)); NS_ENSURE_TRUE_VOID(selPrivate); nsresult rv = selPrivate->AddSelectionListener(this); @@ -494,7 +506,7 @@ IMEContentObserver::UnregisterObservers() mEditor->RemoveEditorObserver(this); } - if (mUpdatePreference.WantSelectionChange() && mSelection) { + if (mSelection) { nsCOMPtr selPrivate(do_QueryInterface(mSelection)); if (selPrivate) { selPrivate->RemoveSelectionListener(this); @@ -1285,7 +1297,7 @@ IMEContentObserver::UpdateSelectionCache() { MOZ_ASSERT(IsSafeToNotifyIME()); - if (!mUpdatePreference.WantSelectionChange()) { + if (WasInitializedWithPlugin()) { return false; } diff --git a/dom/events/IMEContentObserver.h b/dom/events/IMEContentObserver.h index 4c95270e1ee9..1f120fc46d9a 100644 --- a/dom/events/IMEContentObserver.h +++ b/dom/events/IMEContentObserver.h @@ -94,6 +94,7 @@ public: nsIEditor* aEditor); bool IsManaging(nsPresContext* aPresContext, nsIContent* aContent) const; bool IsManaging(const TextComposition* aTextComposition) const; + bool WasInitializedWithPlugin() const; bool IsEditorHandlingEventForComposition() const; bool KeepAliveDuringDeactive() const { @@ -186,8 +187,7 @@ private: * 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(); diff --git a/widget/IMEData.h b/widget/IMEData.h index 3a88716afedf..413b2dab57ce 100644 --- a/widget/IMEData.h +++ b/widget/IMEData.h @@ -39,7 +39,6 @@ struct nsIMEUpdatePreference final 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 @@ -81,11 +80,6 @@ struct nsIMEUpdatePreference final mWantUpdates &= ~DEFAULT_CONDITIONS_OF_NOTIFYING_CHANGES; } - bool WantSelectionChange() const - { - return !!(mWantUpdates & NOTIFY_SELECTION_CHANGE); - } - bool WantTextChange() const { return !!(mWantUpdates & NOTIFY_TEXT_CHANGE); @@ -98,7 +92,7 @@ struct nsIMEUpdatePreference final bool WantChanges() const { - return WantSelectionChange() || WantTextChange(); + return WantTextChange(); } bool WantMouseButtonEventOnChar() const diff --git a/widget/PuppetWidget.cpp b/widget/PuppetWidget.cpp index da91181a8548..d13582b09eba 100644 --- a/widget/PuppetWidget.cpp +++ b/widget/PuppetWidget.cpp @@ -821,7 +821,6 @@ PuppetWidget::GetIMEUpdatePreference() nsIMEUpdatePreference::NOTIFY_POSITION_CHANGE); } return nsIMEUpdatePreference(mIMEPreferenceOfParent.mWantUpdates | - nsIMEUpdatePreference::NOTIFY_SELECTION_CHANGE | nsIMEUpdatePreference::NOTIFY_TEXT_CHANGE | nsIMEUpdatePreference::NOTIFY_POSITION_CHANGE ); #else @@ -897,9 +896,8 @@ PuppetWidget::NotifyIMEOfSelectionChange( 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); diff --git a/widget/android/nsWindow.cpp b/widget/android/nsWindow.cpp index da618a4b9ad4..442237e00f27 100644 --- a/widget/android/nsWindow.cpp +++ b/widget/android/nsWindow.cpp @@ -3369,9 +3369,7 @@ nsWindow::GetIMEUpdatePreference() 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 diff --git a/widget/cocoa/nsChildView.mm b/widget/cocoa/nsChildView.mm index bc696a00d8c7..e95ad926f120 100644 --- a/widget/cocoa/nsChildView.mm +++ b/widget/cocoa/nsChildView.mm @@ -1934,11 +1934,10 @@ nsChildView::ExecuteNativeKeyBinding(NativeKeyBindingsType aType, 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* nsChildView::GetEditorView() diff --git a/widget/gtk/IMContextWrapper.cpp b/widget/gtk/IMContextWrapper.cpp index b9fa926cbf2f..89e775620c52 100644 --- a/widget/gtk/IMContextWrapper.cpp +++ b/widget/gtk/IMContextWrapper.cpp @@ -333,7 +333,7 @@ IMContextWrapper::GetIMEUpdatePreference() const } 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; diff --git a/widget/windows/IMMHandler.cpp b/widget/windows/IMMHandler.cpp index 4b8183964c80..5b3242770105 100644 --- a/widget/windows/IMMHandler.cpp +++ b/widget/windows/IMMHandler.cpp @@ -395,7 +395,6 @@ IMMHandler::GetIMEUpdatePreference() { return nsIMEUpdatePreference( nsIMEUpdatePreference::NOTIFY_POSITION_CHANGE | - nsIMEUpdatePreference::NOTIFY_SELECTION_CHANGE | nsIMEUpdatePreference::NOTIFY_MOUSE_BUTTON_EVENT_ON_CHAR); } diff --git a/widget/windows/TSFTextStore.cpp b/widget/windows/TSFTextStore.cpp index f6feb7581987..4b287863c30b 100644 --- a/widget/windows/TSFTextStore.cpp +++ b/widget/windows/TSFTextStore.cpp @@ -4546,7 +4546,6 @@ TSFTextStore::GetIMEUpdatePreference() 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 |