Bug 1651257 - iFLYTEC IME often commits composition string unfortunately. r=geckoview-reviewers,snorp

This is timing issue when using iFLYTEC keyboard in Xiaomi App Store.

Although replace text transaction often dispatch dummy text change, it may be
unnecessary when other text change is already in text change queue. This issue
occurs if dummy text change is dispatched in the queue and other text change
is also dispatched in the queue.

`AddIMETextChange` merges old text change with newer text change when its range
is overlapped, but it doesn't consider range is same. So if same range, we
should adjust old end simply.

GV-junit test case emulates this situation, but since this is timing issue,
we won't reproduce this by this test case. But it is useful to check future
regressions.

Differential Revision: https://phabricator.services.mozilla.com/D84140

mobile/android/geckoview/src/androidTest/java/org/mozilla/geckoview/test/TextInputDelegateTest.kt

@@ -605,6 +605,23 @@ class TextInputDelegateTest : BaseSessionTest() {
         assertText("Can type using event", ic, "frabat")
     }
 
+    // Test for Multiple setComposingText with same string length.
+    @WithDisplay(width = 512, height = 512) // Child process updates require having a display.
+    @Test fun inputConnection_multiple_setComposingText() {
+
+        setupContent("")
+        val ic = mainSession.textInput.onCreateInputConnection(EditorInfo())!!
+
+        // Don't wait composition event for this test.
+        ic.setComposingText("aaa", 1)
+        ic.setComposingText("aaa", 1)
+        ic.setComposingText("aab", 1)
+
+        finishComposingText(ic)
+        assertTextAndSelectionAt("Multiple setComposingText don't commit composition string",
+                                 ic, "aab", 3)
+    }
+
     // Bug 1133802, duplication when setting the same composing text more than once.
     @Ignore // Disable for frequent failures.
     @WithDisplay(width = 512, height = 512) // Child process updates require having a display.

widget/android/GeckoEditableSupport.cpp

@@ -595,24 +595,30 @@ void GeckoEditableSupport::AddIMETextChange(const IMETextChange& aChange) {
       // No overlap between ranges
       continue;
     }
-    // When merging two ranges, there are generally four posibilities:
-    // [----(----]----), (----[----]----),
-    // [----(----)----], (----[----)----]
-    // where [----] is the first range and (----) is the second range
-    // As seen above, the start of the merged range is always the lesser
-    // of the two start offsets. OldEnd and NewEnd then need to be
-    // adjusted separately depending on the case. In any case, the change
-    // in text length of the merged range should be the sum of text length
-    // changes of the two original ranges, i.e.,
-    // newNewEnd - newOldEnd == newEnd1 - oldEnd1 + newEnd2 - oldEnd2
-    dst.mStart = std::min(dst.mStart, src.mStart);
-    if (src.mOldEnd < dst.mNewEnd) {
-      // New range overlaps or is within previous range; merge
-      dst.mNewEnd += src.mNewEnd - src.mOldEnd;
-    } else {  // src.mOldEnd >= dst.mNewEnd
-      // New range overlaps previous range; merge
-      dst.mOldEnd += src.mOldEnd - dst.mNewEnd;
-      dst.mNewEnd = src.mNewEnd;
+
+    if (src.mStart == dst.mStart && src.mNewEnd == dst.mNewEnd) {
+      // Same range. Adjust old end offset.
+      dst.mOldEnd = std::min(src.mOldEnd, dst.mOldEnd);
+    } else {
+      // When merging two ranges, there are generally four posibilities:
+      // [----(----]----), (----[----]----),
+      // [----(----)----], (----[----)----]
+      // where [----] is the first range and (----) is the second range
+      // As seen above, the start of the merged range is always the lesser
+      // of the two start offsets. OldEnd and NewEnd then need to be
+      // adjusted separately depending on the case. In any case, the change
+      // in text length of the merged range should be the sum of text length
+      // changes of the two original ranges, i.e.,
+      // newNewEnd - newOldEnd == newEnd1 - oldEnd1 + newEnd2 - oldEnd2
+      dst.mStart = std::min(dst.mStart, src.mStart);
+      if (src.mOldEnd < dst.mNewEnd) {
+        // New range overlaps or is within previous range; merge
+        dst.mNewEnd += src.mNewEnd - src.mOldEnd;
+      } else {  // src.mOldEnd >= dst.mNewEnd
+        // New range overlaps previous range; merge
+        dst.mOldEnd += src.mOldEnd - dst.mNewEnd;
+        dst.mNewEnd = src.mNewEnd;
+      }
     }
     // src merged to dst; delete src.
     mIMETextChanges.RemoveElementAt(srcIndex);