Work around Samsung keyboard issue (#12432)

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. EG typing "ㄴㅇ" and then moving the cursor
back to the front of the text and typing "ㄴ" again would result in
"ㄴㅇㄴ", not "ㄴㄴㅇ".

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 only apply the workaround on Samsung devices set
to use Korean input.

This also effectively disables the feature on Samsung keyboards that
allowed users to re-open a composing region for previously typed
characters. See https://github.com/flutter/flutter/issues/29341#issuecomment-531283508.

Fixes flutter/flutter#29341.
This commit is contained in:
Michael Klimushyn 2019-09-25 11:16:32 -07:00 committed by GitHub
parent 0bfca375b3
commit efb7bf434f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 126 additions and 3 deletions

View File

@ -419,6 +419,7 @@ action("robolectric_tests") {
"test/io/flutter/embedding/engine/renderer/FlutterRendererTest.java",
"test/io/flutter/embedding/engine/systemchannels/PlatformChannelTest.java",
"test/io/flutter/plugin/common/StandardMessageCodecTest.java",
"test/io/flutter/plugin/editing/TextInputPluginTest.java",
"test/io/flutter/plugin/platform/SingleViewPresentationTest.java",
"test/io/flutter/util/PreconditionsTest.java",
]
@ -437,6 +438,7 @@ action("robolectric_tests") {
"//third_party/robolectric/lib/robolectric-3.8.jar",
"//third_party/robolectric/lib/shadows-framework-3.8.jar",
"//third_party/robolectric/lib/annotations-3.8.jar",
"//third_party/robolectric/lib/shadowapi-3.8.jar",
"//third_party/robolectric/lib/runtime-1.1.1.jar",
"//third_party/robolectric/lib/common-1.1.1.jar",
"//third_party/robolectric/lib/common-java8-1.1.1.jar",

View File

@ -4,9 +4,12 @@
package io.flutter.plugin.editing;
import android.annotation.SuppressLint;
import android.content.Context;
import android.os.Build;
import android.support.annotation.NonNull;
import android.support.annotation.Nullable;
import android.support.annotation.VisibleForTesting;
import android.text.Editable;
import android.text.InputType;
import android.text.Selection;
@ -41,6 +44,7 @@ public class TextInputPlugin {
private InputConnection lastInputConnection;
@NonNull
private PlatformViewsController platformViewsController;
private final boolean restartAlwaysRequired;
// When true following calls to createInputConnection will return the cached lastInputConnection if the input
// target is a platform view. See the comments on lockPlatformViewInputConnection for more details.
@ -86,6 +90,7 @@ public class TextInputPlugin {
this.platformViewsController = platformViewsController;
this.platformViewsController.attachTextInputPlugin(this);
restartAlwaysRequired = isRestartAlwaysRequired();
}
@NonNull
@ -261,7 +266,7 @@ public class TextInputPlugin {
mImm.hideSoftInputFromWindow(view.getApplicationWindowToken(), 0);
}
private void setTextInputClient(int client, TextInputChannel.Configuration configuration) {
@VisibleForTesting void setTextInputClient(int client, TextInputChannel.Configuration configuration) {
inputTarget = new InputTarget(InputTarget.Type.FRAMEWORK_CLIENT, client);
this.configuration = configuration;
mEditable = Editable.Factory.getInstance().newEditable("");
@ -293,8 +298,8 @@ public class TextInputPlugin {
}
}
private void setTextInputEditingState(View view, TextInputChannel.TextEditState state) {
if (!mRestartInputPending && state.text.equals(mEditable.toString())) {
@VisibleForTesting void setTextInputEditingState(View view, TextInputChannel.TextEditState state) {
if (!restartAlwaysRequired && !mRestartInputPending && state.text.equals(mEditable.toString())) {
applyStateToSelection(state);
mImm.updateSelection(mView, Math.max(Selection.getSelectionStart(mEditable), 0),
Math.max(Selection.getSelectionEnd(mEditable), 0),
@ -308,6 +313,20 @@ public class TextInputPlugin {
}
}
// 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.
//
// 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.
private boolean isRestartAlwaysRequired() {
String language = (Build.VERSION.SDK_INT >= Build.VERSION_CODES.N)
? mImm.getCurrentInputMethodSubtype().getLanguageTag()
: mImm.getCurrentInputMethodSubtype().getLocale();
return Build.MANUFACTURER.equals("samsung") && language.equals("ko");
}
private void clearTextInputClient() {
if (inputTarget.type == InputTarget.Type.PLATFORM_VIEW) {
// Focus changes in the framework tree have no guarantees on the order focus nodes are notified. A node

View File

@ -16,6 +16,7 @@ import io.flutter.embedding.engine.RenderingComponentTest;
import io.flutter.embedding.engine.renderer.FlutterRendererTest;
import io.flutter.embedding.engine.systemchannels.PlatformChannelTest;
import io.flutter.plugin.common.StandardMessageCodecTest;
import io.flutter.plugin.editing.TextInputPluginTest;
import io.flutter.plugin.platform.SingleViewPresentationTest;
import io.flutter.util.PreconditionsTest;
@ -33,6 +34,7 @@ import io.flutter.util.PreconditionsTest;
StandardMessageCodecTest.class,
SingleViewPresentationTest.class,
SmokeTest.class,
TextInputPluginTest.class,
})
/** Runs all of the unit tests listed in the {@code @SuiteClasses} annotation. */
public class FlutterTestSuite { }

View File

@ -0,0 +1,100 @@
package io.flutter.plugin.editing;
import android.content.Context;
import android.util.SparseIntArray;
import android.view.View;
import android.view.inputmethod.InputMethodManager;
import android.view.inputmethod.InputMethodSubtype;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.RuntimeEnvironment;
import org.robolectric.annotation.Config;
import org.robolectric.annotation.Implementation;
import org.robolectric.annotation.Implements;
import org.robolectric.shadow.api.Shadow;
import org.robolectric.shadows.ShadowBuild;
import org.robolectric.shadows.ShadowInputMethodManager;
import io.flutter.embedding.engine.dart.DartExecutor;
import io.flutter.embedding.engine.systemchannels.TextInputChannel;
import io.flutter.plugin.platform.PlatformViewsController;
import static org.junit.Assert.assertEquals;
import static org.mockito.Mockito.mock;
@Config(manifest = Config.NONE, shadows = TextInputPluginTest.TestImm.class)
@RunWith(RobolectricTestRunner.class)
public class TextInputPluginTest {
@Test
public void setTextInputEditingState_doesNotRestartWhenTextIsIdentical() {
// Initialize a general TextInputPlugin.
InputMethodSubtype inputMethodSubtype = mock(InputMethodSubtype.class);
TestImm testImm = Shadow.extract(RuntimeEnvironment.application.getSystemService(Context.INPUT_METHOD_SERVICE));
testImm.setCurrentInputMethodSubtype(inputMethodSubtype);
View testView = new View(RuntimeEnvironment.application);
TextInputPlugin textInputPlugin = new TextInputPlugin(testView, mock(DartExecutor.class), mock(PlatformViewsController.class));
textInputPlugin.setTextInputClient(0, new TextInputChannel.Configuration(false, false, TextInputChannel.TextCapitalization.NONE, 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));
// Move the cursor.
assertEquals(1, testImm.getRestartCount(testView));
textInputPlugin.setTextInputEditingState(testView, new TextInputChannel.TextEditState("", 0, 0));
// Verify that we haven't restarted the input.
assertEquals(1, testImm.getRestartCount(testView));
}
// See https://github.com/flutter/flutter/issues/29341
@Test
public void setTextInputEditingState_alwaysRestartsOnAffectedDevices() {
// Initialize a TextInputPlugin that needs to be always restarted.
ShadowBuild.setManufacturer("samsung");
InputMethodSubtype inputMethodSubtype = new InputMethodSubtype(0, 0, /*locale=*/"ko", "", "", false, false);
TestImm testImm = Shadow.extract(RuntimeEnvironment.application.getSystemService(Context.INPUT_METHOD_SERVICE));
testImm.setCurrentInputMethodSubtype(inputMethodSubtype);
View testView = new View(RuntimeEnvironment.application);
TextInputPlugin textInputPlugin = new TextInputPlugin(testView, mock(DartExecutor.class), mock(PlatformViewsController.class));
textInputPlugin.setTextInputClient(0, new TextInputChannel.Configuration(false, false, TextInputChannel.TextCapitalization.NONE, 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));
// Move the cursor.
assertEquals(1, testImm.getRestartCount(testView));
textInputPlugin.setTextInputEditingState(testView, new TextInputChannel.TextEditState("", 0, 0));
// Verify that we've restarted the input.
assertEquals(2, testImm.getRestartCount(testView));
}
@Implements(InputMethodManager.class)
public static class TestImm extends ShadowInputMethodManager {
private InputMethodSubtype currentInputMethodSubtype;
private SparseIntArray restartCounter = new SparseIntArray();
public TestImm() {
}
@Implementation
public InputMethodSubtype getCurrentInputMethodSubtype() {
return currentInputMethodSubtype;
}
@Implementation
public void restartInput(View view) {
int count = restartCounter.get(view.hashCode(), /*defaultValue=*/0) + 1;
restartCounter.put(view.hashCode(), count);
}
public void setCurrentInputMethodSubtype(InputMethodSubtype inputMethodSubtype) {
this.currentInputMethodSubtype = inputMethodSubtype;
}
public int getRestartCount(View view) {
return restartCounter.get(view.hashCode(), /*defaultValue=*/0);
}
}
}