[Android Text Input] restart when framework changes composing region (flutter/engine#25180)

This commit is contained in:
LongCatIsLooong 2021-03-29 16:29:01 -07:00 committed by GitHub
parent 0f9f16b58a
commit bc034502d7
3 changed files with 59 additions and 88 deletions

View File

@ -741,5 +741,9 @@ public class TextInputChannel {
// be -1.
return selectionStart >= 0;
}
public boolean hasComposing() {
return composingStart >= 0 && composingEnd > composingStart;
}
}
}

View File

@ -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(

View File

@ -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: