Bug 1613804 - InputConnection.finishComposingText should commit composition text. r=geckoview-reviewers,snorp

Gecko don't commit composition when software keyboard calls
InputConnection.finishComposingText. It is incompatible with Blink's behaviour.

BaseInputConnection.finishComposingText() implementation is the following.

1. Begin batch edit.
2. Remove all composition span flag.
3. End batch edit.

So if no composition after batch edit is finished, we should commit it on Gecko
to synchronize composition state.

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

--HG--
extra : moz-landing-system : lando

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

@@ -93,6 +93,16 @@ class TextInputDelegateTest : BaseSessionTest() {
         promise.value
     }
 
+    private fun finishComposingText(ic: InputConnection) {
+        val promise = mainSession.evaluatePromiseJS(
+                when (id) {
+                    "#designmode" -> "new Promise(r => document.querySelector('$id').contentDocument.addEventListener('compositionend', r, { once: true }))"
+                    else -> "new Promise(r => document.querySelector('$id').addEventListener('compositionend', r, { once: true }))"
+                })
+        ic.finishComposingText()
+        promise.value
+    }
+
     private fun pressKey(keyCode: Int) {
         // Create a Promise to listen to the key event, and wait on it below.
         val promise = mainSession.evaluatePromiseJS(
@@ -526,4 +536,27 @@ class TextInputDelegateTest : BaseSessionTest() {
         promise.value
         assertText("empty text", ic, "")
     }
+
+    @WithDisplay(width = 512, height = 512) // Child process updates require having a display.
+    @Test fun bug1613804_finishComposingText() {
+        mainSession.textInput.view = View(InstrumentationRegistry.getInstrumentation().targetContext)
+
+        mainSession.loadTestPath(INPUTS_PATH)
+        mainSession.waitForPageStop()
+
+        textContent = ""
+        mainSession.evaluateJS("document.querySelector('$id').focus()")
+        mainSession.waitUntilCalled(GeckoSession.TextInputDelegate::class, "restartInput")
+
+        val ic = mainSession.textInput.onCreateInputConnection(EditorInfo())!!
+
+        ic.beginBatchEdit();
+        ic.setComposingText("abc", 1)
+        ic.endBatchEdit()
+
+        // finishComposingText has to dispatch compositionend event.
+        finishComposingText(ic)
+
+        assertText("commit abc", ic, "abc")
+    }
 }

mobile/android/geckoview/src/main/aidl/org/mozilla/gecko/IGeckoEditableChild.aidl

@@ -35,4 +35,7 @@ interface IGeckoEditableChild {
 
     // Request cursor updates from the child.
     void onImeRequestCursorUpdates(int requestMode);
+
+    // Commit current composition.
+    void onImeRequestCommit();
 }

mobile/android/geckoview/src/main/java/org/mozilla/gecko/GeckoEditableChild.java

@@ -75,6 +75,11 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl
         public void onImeRequestCursorUpdates(final int requestMode) {
             GeckoEditableChild.this.onImeRequestCursorUpdates(requestMode);
         }
+
+        @Override // IGeckoEditableChild
+        public void onImeRequestCommit() {
+            GeckoEditableChild.this.onImeRequestCommit();
+        }
     }
 
     private final IGeckoEditableChild mEditableChild;
@@ -148,6 +153,9 @@ public final class GeckoEditableChild extends JNIObject implements IGeckoEditabl
     @WrapForJNI(dispatchTo = "proxy") @Override // IGeckoEditableChild
     public native void onImeRequestCursorUpdates(int requestMode);
 
+    @WrapForJNI(dispatchTo = "proxy") @Override // IGeckoEditableChild
+    public native void onImeRequestCommit();
+
     @Override // JNIObject
     protected void disposeNative() {
         // Disposal happens in native code.

mobile/android/geckoview/src/main/java/org/mozilla/geckoview/GeckoEditable.java

@@ -318,29 +318,11 @@ import android.view.inputmethod.EditorInfo;
          * @return true if discarding composition
          */
         private boolean isDiscardingComposition() {
-            boolean wasComposing = false;
-            Object[] spans = mShadowText.getSpans(0, mShadowText.length(), Object.class);
-            for (final Object span : spans) {
-                if ((mShadowText.getSpanFlags(span) & Spanned.SPAN_COMPOSING) != 0) {
-                    wasComposing = true;
-                    break;
-                }
-            }
-
-            if (!wasComposing) {
+            if (!isComposing(mShadowText)) {
                 return false;
             }
 
-            boolean isComposing = false;
-            spans = mCurrentText.getSpans(0, mCurrentText.length(), Object.class);
-            for (final Object span : spans) {
-                if ((mCurrentText.getSpanFlags(span) & Spanned.SPAN_COMPOSING) != 0) {
-                    isComposing = true;
-                    break;
-                }
-            }
-
-            return !isComposing;
+            return !isComposing(mCurrentText);
         }
 
         public synchronized void syncShadowText(
@@ -1071,6 +1053,24 @@ import android.view.inputmethod.EditorInfo;
 
         mInBatchMode = inBatchMode;
 
+        if (!inBatchMode && mFocusedChild != null) {
+            // We may not commit composition on Gecko even if Java side has
+            // no composition. So we have to sync composition state with Gecko
+            // when batch edit is done.
+            //
+            // i.e. Although finishComposingText removes composing span, we
+            // don't commit current composition yet.
+            final Editable editable = getEditable();
+            if (editable != null && !isComposing(editable)) {
+                try {
+                    mFocusedChild.onImeRequestCommit();
+                } catch (final RemoteException e) {
+                    Log.e(LOGTAG, "Remote call failed", e);
+                }
+            }
+            // Committing composition doesn't change text, so we can sync shadow text.
+        }
+
         if (!inBatchMode && mNeedSync) {
             icSyncShadowText();
         }
@@ -1380,13 +1380,9 @@ import android.view.inputmethod.EditorInfo;
                 //
                 // Nevertheless, if we somehow lost the composition, we must force the
                 // keyboard to reset.
-                final Spanned text = mText.getShadowText();
-                final Object[] spans = text.getSpans(0, text.length(), Object.class);
-                for (final Object span : spans) {
-                    if ((text.getSpanFlags(span) & Spanned.SPAN_COMPOSING) != 0) {
-                        // Still have composition; no need to reset.
-                        return; // Don't notify listener.
-                    }
+                if (isComposing(mText.getShadowText())) {
+                    // Still have composition; no need to reset.
+                    return; // Don't notify listener.
                 }
                 // No longer have composition; perform reset.
                 icRestartInput(GeckoSession.TextInputDelegate.RESTART_REASON_CONTENT_CHANGE,
@@ -2199,5 +2195,16 @@ import android.view.inputmethod.EditorInfo;
         }
         return true;
     }
+
+    private static boolean isComposing(final Spanned text) {
+        final Object[] spans = text.getSpans(0, text.length(), Object.class);
+        for (final Object span : spans) {
+            if ((text.getSpanFlags(span) & Spanned.SPAN_COMPOSING) != 0) {
+                return true;
+            }
+        }
+
+        return false;
+    }
 }
 

widget/android/GeckoEditableSupport.cpp

@@ -1317,6 +1317,15 @@ void GeckoEditableSupport::OnImeRequestCursorUpdates(int aRequestMode) {
   mIMEMonitorCursor = (aRequestMode == EditableClient::START_MONITOR);
 }
 
+void GeckoEditableSupport::OnImeRequestCommit() {
+  if (mIMEMaskEventsCount > 0) {
+    // Not focused.
+    return;
+  }
+
+  RemoveComposition(COMMIT_IME_COMPOSITION);
+}
+
 void GeckoEditableSupport::AsyncNotifyIME(int32_t aNotification) {
   RefPtr<GeckoEditableSupport> self(this);
 

widget/android/GeckoEditableSupport.h

@@ -240,6 +240,9 @@ class GeckoEditableSupport final
 
   // Set cursor mode whether IME requests
   void OnImeRequestCursorUpdates(int aRequestMode);
+
+  // Commit current composition to sync Gecko text state with Java.
+  void OnImeRequestCommit();
 };
 
 }  // namespace widget