From ae79cbc8c0c94eb4f33d2003edf427d42b56a4d5 Mon Sep 17 00:00:00 2001 From: Jim Chen Date: Wed, 6 Jan 2016 21:33:18 -0500 Subject: [PATCH] Bug 1236640 - Make selection change part of the IME change transaction; r=esawin We notify IME text changes in transactions. We should make selection change notification part of that transaction. --- widget/android/nsWindow.cpp | 68 ++++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 24 deletions(-) diff --git a/widget/android/nsWindow.cpp b/widget/android/nsWindow.cpp index 62b5f018bcd7..0faaf3f9bf44 100644 --- a/widget/android/nsWindow.cpp +++ b/widget/android/nsWindow.cpp @@ -331,7 +331,7 @@ private: FLUSH_FLAG_NONE, FLUSH_FLAG_RETRY }; - void PostFlushIMEChanges(FlushChangesFlag aFlags = FLUSH_FLAG_NONE); + void PostFlushIMEChanges(); void FlushIMEChanges(FlushChangesFlag aFlags = FLUSH_FLAG_NONE); public: @@ -2259,10 +2259,9 @@ nsWindow::GeckoViewSupport::AddIMETextChange(const IMETextChange& aChange) } void -nsWindow::GeckoViewSupport::PostFlushIMEChanges(FlushChangesFlag aFlags) +nsWindow::GeckoViewSupport::PostFlushIMEChanges() { - if (aFlags != FLUSH_FLAG_RETRY && - (!mIMETextChanges.IsEmpty() || mIMESelectionChanged)) { + if (!mIMETextChanges.IsEmpty() || mIMESelectionChanged) { // Already posted return; } @@ -2270,9 +2269,9 @@ nsWindow::GeckoViewSupport::PostFlushIMEChanges(FlushChangesFlag aFlags) // Keep a strong reference to the window to keep 'this' alive. RefPtr window(&this->window); - nsAppShell::PostEvent([this, window, aFlags] { + nsAppShell::PostEvent([this, window] { if (!window->Destroyed()) { - FlushIMEChanges(aFlags); + FlushIMEChanges(); } }); } @@ -2307,6 +2306,23 @@ nsWindow::GeckoViewSupport::FlushIMEChanges(FlushChangesFlag aFlags) mIMETextChangedDuringFlush = false; + auto shouldAbort = [=] () -> bool { + if (!mIMETextChangedDuringFlush) { + return false; + } + // A query event could have triggered more text changes to come in, as + // indicated by our flag. If that happens, try flushing IME changes + // again. + if (aFlags != FLUSH_FLAG_RETRY) { + FlushIMEChanges(FLUSH_FLAG_RETRY); + } else { + // Don't retry if already retrying, to avoid infinite loops. + __android_log_print(ANDROID_LOG_WARN, "GeckoViewSupport", + "Already retrying IME flush"); + } + return true; + }; + for (const IMETextChange &change : mIMETextChanges) { if (change.mStart == change.mOldEnd && change.mStart == change.mNewEnd) { @@ -2324,14 +2340,7 @@ nsWindow::GeckoViewSupport::FlushIMEChanges(FlushChangesFlag aFlags) NS_ENSURE_TRUE_VOID(event.mReply.mContentsRoot == imeRoot.get()); } - if (mIMETextChangedDuringFlush) { - // The query event above could have triggered more text changes to - // come in, as indicated by our flag. If that happens, try flushing - // IME changes again later. - if (!NS_WARN_IF(aFlags == FLUSH_FLAG_RETRY)) { - // Don't retry if already retrying, to avoid infinite loops. - PostFlushIMEChanges(FLUSH_FLAG_RETRY); - } + if (shouldAbort()) { return; } @@ -2340,6 +2349,26 @@ nsWindow::GeckoViewSupport::FlushIMEChanges(FlushChangesFlag aFlags) change.mOldEnd, change.mNewEnd}); } + int32_t selStart; + int32_t selEnd; + + if (mIMESelectionChanged) { + WidgetQueryContentEvent event(true, eQuerySelectedText, &window); + window.InitEvent(event, nullptr); + window.DispatchEvent(&event); + + NS_ENSURE_TRUE_VOID(event.mSucceeded); + NS_ENSURE_TRUE_VOID(event.mReply.mContentsRoot == imeRoot.get()); + + if (shouldAbort()) { + return; + } + + selStart = int32_t(event.GetSelectionStart()); + selEnd = int32_t(event.GetSelectionEnd()); + } + + // Commit the text change and selection change transaction. mIMETextChanges.Clear(); for (const TextRecord& record : textTransaction) { @@ -2349,16 +2378,7 @@ nsWindow::GeckoViewSupport::FlushIMEChanges(FlushChangesFlag aFlags) if (mIMESelectionChanged) { mIMESelectionChanged = false; - - WidgetQueryContentEvent event(true, eQuerySelectedText, &window); - window.InitEvent(event, nullptr); - window.DispatchEvent(&event); - - NS_ENSURE_TRUE_VOID(event.mSucceeded); - NS_ENSURE_TRUE_VOID(event.mReply.mContentsRoot == imeRoot.get()); - - mEditable->OnSelectionChange(int32_t(event.GetSelectionStart()), - int32_t(event.GetSelectionEnd())); + mEditable->OnSelectionChange(selStart, selEnd); } }