diff --git a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/TextInputChannel.java b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/TextInputChannel.java index 59fd7e1b914..d69b0766de9 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/TextInputChannel.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/embedding/engine/systemchannels/TextInputChannel.java @@ -741,5 +741,9 @@ public class TextInputChannel { // be -1. return selectionStart >= 0; } + + public boolean hasComposing() { + return composingStart >= 0 && composingEnd > composingStart; + } } } diff --git a/engine/src/flutter/shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java b/engine/src/flutter/shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java index c4d5ad39255..9e782624ded 100644 --- a/engine/src/flutter/shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java +++ b/engine/src/flutter/shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java @@ -9,7 +9,6 @@ import android.content.Context; import android.graphics.Rect; import android.os.Build; import android.os.Bundle; -import android.provider.Settings; import android.text.Editable; import android.text.InputType; import android.util.SparseArray; @@ -22,7 +21,6 @@ import android.view.autofill.AutofillValue; import android.view.inputmethod.EditorInfo; import android.view.inputmethod.InputConnection; import android.view.inputmethod.InputMethodManager; -import android.view.inputmethod.InputMethodSubtype; import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; @@ -49,7 +47,6 @@ public class TextInputPlugin implements ListenableEditingState.EditingStateWatch @Nullable private InputConnection lastInputConnection; @NonNull private PlatformViewsController platformViewsController; @Nullable private Rect lastClientRect; - private final boolean restartAlwaysRequired; private ImeSyncDeferringInsetsCallback imeSyncCallback; private AndroidKeyProcessor keyProcessor; @@ -161,7 +158,6 @@ public class TextInputPlugin implements ListenableEditingState.EditingStateWatch this.platformViewsController = platformViewsController; this.platformViewsController.attachTextInputPlugin(this); - restartAlwaysRequired = isRestartAlwaysRequired(); } @NonNull @@ -417,16 +413,45 @@ public class TextInputPlugin implements ListenableEditingState.EditingStateWatch mRestartInputPending = false; } + private static boolean composingChanged( + TextInputChannel.TextEditState before, TextInputChannel.TextEditState after) { + final int composingRegionLength = before.composingEnd - before.composingStart; + if (composingRegionLength != after.composingEnd - after.composingStart) { + return true; + } + for (int index = 0; index < composingRegionLength; index++) { + if (before.text.charAt(index + before.composingStart) + != after.text.charAt(index + after.composingStart)) { + return true; + } + } + return false; + } + // Called by the text input channel to update the text input plugin with the // latest TextEditState from the framework. @VisibleForTesting void setTextInputEditingState(View view, TextInputChannel.TextEditState state) { + if (!mRestartInputPending + && mLastKnownFrameworkTextEditingState != null + && mLastKnownFrameworkTextEditingState.hasComposing()) { + // Also restart input if the framework (or the developer) decides to + // change the composing region by itself (which is discouraged). Many IMEs + // don't expect editors to commit composing text, so a restart is needed + // to reset their internal states. + mRestartInputPending = composingChanged(mLastKnownFrameworkTextEditingState, state); + if (mRestartInputPending) { + Log.w( + TAG, + "Changing the content within the the composing region may cause the input method to behave strangely, and is therefore discouraged. See https://github.com/flutter/flutter/issues/78827 for more details"); + } + } + mLastKnownFrameworkTextEditingState = state; mEditable.setEditingState(state); - // Restart if there is a pending restart or the device requires a force restart - // (see isRestartAlwaysRequired). Restarting will also update the selection. - if (restartAlwaysRequired || mRestartInputPending) { + // Restart if needed. Restarting will also update the selection. + if (mRestartInputPending) { mImm.restartInput(view); mRestartInputPending = false; } @@ -476,32 +501,6 @@ public class TextInputPlugin implements ListenableEditingState.EditingStateWatch (int) Math.ceil(minMax[3] * density)); } - // Samsung's Korean keyboard has a bug where it always attempts to combine characters based on - // its internal state, ignoring if and when the cursor is moved programmatically. The same bug - // also causes non-korean keyboards to occasionally duplicate text when tapping in the middle - // of existing text to edit it. - // - // Fully restarting the IMM works around this because it flushes the keyboard's internal state - // and stops it from trying to incorrectly combine characters. However this also has some - // negative performance implications, so we don't want to apply this workaround in every case. - @SuppressLint("NewApi") // New API guard is inline, the linter can't see it. - @SuppressWarnings("deprecation") - private boolean isRestartAlwaysRequired() { - InputMethodSubtype subtype = mImm.getCurrentInputMethodSubtype(); - // Impacted devices all shipped with Android Lollipop or newer. - if (subtype == null - || Build.VERSION.SDK_INT < Build.VERSION_CODES.LOLLIPOP - || !Build.MANUFACTURER.equals("samsung")) { - return false; - } - String keyboardName = - Settings.Secure.getString( - mView.getContext().getContentResolver(), Settings.Secure.DEFAULT_INPUT_METHOD); - // The Samsung keyboard is called "com.sec.android.inputmethod/.SamsungKeypad" but look - // for "Samsung" just in case Samsung changes the name of the keyboard. - return keyboardName.contains("Samsung"); - } - @VisibleForTesting void clearTextInputClient() { if (inputTarget.type == InputTarget.Type.PLATFORM_VIEW) { @@ -572,15 +571,14 @@ public class TextInputPlugin implements ListenableEditingState.EditingStateWatch final int selectionEnd = mEditable.getSelectionEnd(); final int composingStart = mEditable.getComposingStart(); final int composingEnd = mEditable.getComposingEnd(); - // The framework needs to send value first. final boolean skipFrameworkUpdate = + // The framework needs to send its editing state first. mLastKnownFrameworkTextEditingState == null || (mEditable.toString().equals(mLastKnownFrameworkTextEditingState.text) && selectionStart == mLastKnownFrameworkTextEditingState.selectionStart && selectionEnd == mLastKnownFrameworkTextEditingState.selectionEnd && composingStart == mLastKnownFrameworkTextEditingState.composingStart && composingEnd == mLastKnownFrameworkTextEditingState.composingEnd); - // Skip if we're currently setting if (!skipFrameworkUpdate) { Log.v(TAG, "send EditingState to flutter: " + mEditable.toString()); textInputChannel.updateEditingState( diff --git a/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java b/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java index 19a8b1b5be5..7e8436418ae 100644 --- a/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java +++ b/engine/src/flutter/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java @@ -345,16 +345,13 @@ public class TextInputPluginTest { // https://github.com/flutter/flutter/issues/31512 // All modern Samsung keybords are affected including non-korean languages and thus // need the restart. + // Update: many other keyboards need this too: + // https://github.com/flutter/flutter/issues/78827 @Test - public void setTextInputEditingState_alwaysRestartsOnAffectedDevices2() { + public void setTextInputEditingState_restartsIMEOnlyWhenFrameworkChangesComposingRegion() { // Initialize a TextInputPlugin that needs to be always restarted. - ShadowBuild.setManufacturer("samsung"); InputMethodSubtype inputMethodSubtype = new InputMethodSubtype(0, 0, /*locale=*/ "en", "", "", false, false); - Settings.Secure.putString( - RuntimeEnvironment.application.getContentResolver(), - Settings.Secure.DEFAULT_INPUT_METHOD, - "com.sec.android.inputmethod/.SamsungKeypad"); TestImm testImm = Shadow.extract( RuntimeEnvironment.application.getSystemService(Context.INPUT_METHOD_SERVICE)); @@ -370,7 +367,7 @@ public class TextInputPluginTest { false, true, TextInputChannel.TextCapitalization.NONE, - null, + new TextInputChannel.InputType(TextInputChannel.TextInputType.TEXT, false, false), null, null, null, @@ -378,59 +375,31 @@ public class TextInputPluginTest { // There's a pending restart since we initialized the text input client. Flush that now. textInputPlugin.setTextInputEditingState( testView, new TextInputChannel.TextEditState("", 0, 0, -1, -1)); - - // Move the cursor. assertEquals(1, testImm.getRestartCount(testView)); + InputConnection connection = textInputPlugin.createInputConnection(testView, new EditorInfo()); + connection.setComposingText("POWERRRRR", 1); + textInputPlugin.setTextInputEditingState( - testView, new TextInputChannel.TextEditState("", 0, 0, -1, -1)); + testView, new TextInputChannel.TextEditState("UNLIMITED POWERRRRR", 0, 0, 10, 19)); + // Does not restart since the composing text is not changed. + assertEquals(1, testImm.getRestartCount(testView)); + + connection.finishComposingText(); + // Does not restart since the composing text is committed by the IME. + assertEquals(1, testImm.getRestartCount(testView)); + + // Does not restart since the composing text is changed by the IME. + connection.setComposingText("POWERRRRR", 1); + assertEquals(1, testImm.getRestartCount(testView)); + + // The framework tries to commit the composing region. + textInputPlugin.setTextInputEditingState( + testView, new TextInputChannel.TextEditState("POWERRRRR", 0, 0, -1, -1)); // Verify that we've restarted the input. assertEquals(2, testImm.getRestartCount(testView)); } - @Test - public void setTextInputEditingState_doesNotRestartOnUnaffectedDevices() { - // Initialize a TextInputPlugin that needs to be always restarted. - ShadowBuild.setManufacturer("samsung"); - InputMethodSubtype inputMethodSubtype = - new InputMethodSubtype(0, 0, /*locale=*/ "en", "", "", false, false); - Settings.Secure.putString( - RuntimeEnvironment.application.getContentResolver(), - Settings.Secure.DEFAULT_INPUT_METHOD, - "com.fake.test.blah/.NotTheRightKeyboard"); - TestImm testImm = - Shadow.extract( - RuntimeEnvironment.application.getSystemService(Context.INPUT_METHOD_SERVICE)); - testImm.setCurrentInputMethodSubtype(inputMethodSubtype); - View testView = new View(RuntimeEnvironment.application); - TextInputChannel textInputChannel = new TextInputChannel(mock(DartExecutor.class)); - TextInputPlugin textInputPlugin = - new TextInputPlugin(testView, textInputChannel, mock(PlatformViewsController.class)); - textInputPlugin.setTextInputClient( - 0, - new TextInputChannel.Configuration( - false, - false, - true, - TextInputChannel.TextCapitalization.NONE, - null, - null, - null, - null, - null)); - // There's a pending restart since we initialized the text input client. Flush that now. - textInputPlugin.setTextInputEditingState( - testView, new TextInputChannel.TextEditState("", 0, 0, -1, -1)); - - // Move the cursor. - assertEquals(1, testImm.getRestartCount(testView)); - textInputPlugin.setTextInputEditingState( - testView, new TextInputChannel.TextEditState("", 0, 0, -1, -1)); - - // Verify that we've restarted the input. - assertEquals(1, testImm.getRestartCount(testView)); - } - @Test public void TextEditState_throwsOnInvalidStatesReceived() { // Index OOB: