From d60b6a18cc68492511a9b903434c199104f609dd Mon Sep 17 00:00:00 2001 From: Makoto Kato Date: Mon, 6 Sep 2021 12:38:26 +0000 Subject: [PATCH] Bug 1724811 - Use selection data from notification instead of get current selection. r=geckoview-reviewers,agi Although root cause isn't bug 1713334, this occurs after landing it. When inputting "." by Swiftkey, it generates key event then dispatches it to Gecko. After text is changed by this key event, we will update this change to GeckoView. But when this occurs, we post invalid selection change to GeckoView, then `GeckoEditableChild` throws `IllegalArgumentException`. Normal ====== 1. Dispatch "." text change. 2. Receive text and selection change notification by 1. 3. Posted text/selection change notification to GV's parent This situation ============== 1. Dispatch "." text change. 2. Receive text and selection change notification by 1. 3. Dispatch next "." text change. 4. Posted 2.'s text/selection change notification to GV's parent. 5. `GeckoEditableChild` throws `IllegalArgumentException`. So when step 4.., we should use 2.'s change data. But since we query current Gecko's selection data, we may use invalid selection data. So we should cache selection change, then use it. Differential Revision: https://phabricator.services.mozilla.com/D124435 --- widget/android/GeckoEditableSupport.cpp | 40 ++++++++++++++++++------- widget/android/GeckoEditableSupport.h | 15 ++++++++++ 2 files changed, 45 insertions(+), 10 deletions(-) diff --git a/widget/android/GeckoEditableSupport.cpp b/widget/android/GeckoEditableSupport.cpp index 35bc41bc6e6e..90c719e1aace 100644 --- a/widget/android/GeckoEditableSupport.cpp +++ b/widget/android/GeckoEditableSupport.cpp @@ -687,19 +687,31 @@ void GeckoEditableSupport::FlushIMEChanges(FlushChangesFlag aFlags) { int32_t selEnd = -1; if (mIMESelectionChanged) { - WidgetQueryContentEvent querySelectedTextEvent(true, eQuerySelectedText, - widget); - widget->DispatchEvent(&querySelectedTextEvent, status); + if (mCachedSelection.IsValid()) { + selStart = mCachedSelection.mStartOffset; + selEnd = mCachedSelection.mEndOffset; + } else { + // XXX Unfortunately we don't know current selection via selection + // change notification. + // eQuerySelectedText might be newer data than text change data. + // It means that GeckoEditableChild.onSelectionChange may throw + // IllegalArgumentException since we don't merge with newer text + // change. + WidgetQueryContentEvent querySelectedTextEvent(true, eQuerySelectedText, + widget); + widget->DispatchEvent(&querySelectedTextEvent, status); - if (shouldAbort(NS_WARN_IF(querySelectedTextEvent.DidNotFindSelection()))) { - return; + if (shouldAbort( + NS_WARN_IF(querySelectedTextEvent.DidNotFindSelection()))) { + return; + } + + selStart = static_cast( + querySelectedTextEvent.mReply->SelectionStartOffset()); + selEnd = static_cast( + querySelectedTextEvent.mReply->SelectionEndOffset()); } - selStart = static_cast( - querySelectedTextEvent.mReply->SelectionStartOffset()); - selEnd = static_cast( - querySelectedTextEvent.mReply->SelectionEndOffset()); - if (aFlags == FLUSH_FLAG_RECOVER && textTransaction.IsValid()) { // Sometimes we get out-of-bounds selection during recovery. // Limit the offsets so we don't crash. @@ -1293,6 +1305,8 @@ nsresult GeckoEditableSupport::NotifyIME( mDispatcher = dispatcher; mIMEKeyEvents.Clear(); + mCachedSelection.Reset(); + mIMEDelaySynchronizeReply = false; mIMEActiveCompositionCount = 0; FlushIMEText(); @@ -1333,6 +1347,12 @@ nsresult GeckoEditableSupport::NotifyIME( ALOGIME("IME: NOTIFY_IME_OF_SELECTION_CHANGE: SelectionChangeData=%s", ToString(aNotification.mSelectionChangeData).c_str()); + mCachedSelection.mStartOffset = + aNotification.mSelectionChangeData.mOffset; + mCachedSelection.mEndOffset = + aNotification.mSelectionChangeData.mString->Length() + + aNotification.mSelectionChangeData.mOffset; + PostFlushIMEChanges(); mIMESelectionChanged = true; break; diff --git a/widget/android/GeckoEditableSupport.h b/widget/android/GeckoEditableSupport.h index dd9f5a5f2f10..abc746efd962 100644 --- a/widget/android/GeckoEditableSupport.h +++ b/widget/android/GeckoEditableSupport.h @@ -77,6 +77,21 @@ class GeckoEditableSupport final bool mIMETextChangedDuringFlush; bool mIMEMonitorCursor; + // The cached selection data + struct Selection { + Selection() : mStartOffset(-1), mEndOffset(-1) {} + + void Reset() { + mStartOffset = -1; + mEndOffset = -1; + } + + bool IsValid() const { return mStartOffset >= 0 && mEndOffset >= 0; } + + int32_t mStartOffset; + int32_t mEndOffset; + } mCachedSelection; + nsIWidget* GetWidget() const; nsWindow* GetNsWindow() const;