-
Notifications
You must be signed in to change notification settings - Fork 6k
[Android Text Input] Remove Samsung restart input workaround for newer Samsung keyboards #24288
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
|
|
||
| import android.annotation.SuppressLint; | ||
| import android.content.Context; | ||
| import android.content.pm.PackageManager; | ||
| import android.graphics.Rect; | ||
| import android.os.Build; | ||
| import android.os.Bundle; | ||
|
|
@@ -497,7 +498,27 @@ private boolean isRestartAlwaysRequired() { | |
| 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"); | ||
| if (!keyboardName.contains("Samsung")) { | ||
| return false; | ||
| } | ||
|
|
||
| final long versionCode; | ||
| try { | ||
| versionCode = | ||
| mView | ||
| .getContext() | ||
| .getPackageManager() | ||
| .getPackageInfo("com.sec.android.inputmethod", 0) | ||
| .getLongVersionCode(); | ||
| } catch (PackageManager.NameNotFoundException e) { | ||
| Log.w(TAG, "com.sec.android.inputmethod is not installed."); | ||
| return false; | ||
| } | ||
|
|
||
| // 3.3.23.33 is a known version that's free of the aforementioned bug. | ||
| // 3.0.24.96 still has this bug. | ||
| // TODO(LongCatIsLooong): Find the minimum version that has the fix. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you plan to do this in this PR or later? Definitely my keyboard v2.0.01-44 has the bug, so it's between these two.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately I am still not able to reset the IME version. I've tried @hamdikahloun thanks for the pointer! It's pretty handy. |
||
| return versionCode < 332333999; | ||
| } | ||
|
|
||
| @VisibleForTesting | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ | |
|
|
||
| import android.annotation.TargetApi; | ||
| import android.content.Context; | ||
| import android.content.pm.PackageInfo; | ||
| import android.content.res.AssetManager; | ||
| import android.graphics.Insets; | ||
| import android.graphics.Rect; | ||
|
|
@@ -64,13 +65,15 @@ | |
| import org.mockito.Mock; | ||
| import org.robolectric.RobolectricTestRunner; | ||
| import org.robolectric.RuntimeEnvironment; | ||
| import org.robolectric.Shadows; | ||
| 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.ShadowAutofillManager; | ||
| import org.robolectric.shadows.ShadowBuild; | ||
| import org.robolectric.shadows.ShadowInputMethodManager; | ||
| import org.robolectric.shadows.ShadowPackageManager; | ||
|
|
||
| @Config( | ||
| manifest = Config.NONE, | ||
|
|
@@ -339,14 +342,21 @@ public void setTextInputEditingState_alwaysSetEditableWhenDifferent() { | |
| assertTrue(textInputPlugin.getEditable().toString().equals("Shibuyawoo")); | ||
| } | ||
|
|
||
| // See https://github.com/flutter/flutter/issues/29341 and | ||
| // https://github.com/flutter/flutter/issues/31512 | ||
| // All modern Samsung keybords are affected including non-korean languages and thus | ||
| // need the restart. | ||
| // See also: https://github.com/flutter/flutter/issues/29341 and | ||
| // https://github.com/flutter/flutter/issues/31512. | ||
| // Some recent versions of Samsung keybords are affected including non-korean | ||
| // languages and thus needed the restart. | ||
| @Test | ||
| public void setTextInputEditingState_alwaysRestartsOnAffectedDevices2() { | ||
| // Initialize a TextInputPlugin that needs to be always restarted. | ||
| public void setTextInputEditingState_alwaysRestartsOnAffectedDevices() { | ||
| // Initialize a TextInputPlugin with a Samsung keypad. | ||
| ShadowBuild.setManufacturer("samsung"); | ||
| final ShadowPackageManager packageManager = | ||
| Shadows.shadowOf( | ||
| RuntimeEnvironment.application.getApplicationContext().getPackageManager()); | ||
| final PackageInfo info = new PackageInfo(); | ||
| info.packageName = "com.sec.android.inputmethod"; | ||
| info.versionCode = 200000000; | ||
| packageManager.addPackage(info); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's nice that you were able to fake this 👍 |
||
| InputMethodSubtype inputMethodSubtype = | ||
| new InputMethodSubtype(0, 0, /*locale=*/ "en", "", "", false, false); | ||
| Settings.Secure.putString( | ||
|
|
@@ -386,6 +396,59 @@ public void setTextInputEditingState_alwaysRestartsOnAffectedDevices2() { | |
| assertEquals(2, testImm.getRestartCount(testView)); | ||
| } | ||
|
|
||
| // Regression test for https://github.com/flutter/flutter/issues/73433. | ||
| // The restart workaround seems to have caused #73433 and it's no longer | ||
| // needed on newer versions of Samsung keyboard. | ||
| @Test | ||
| public void setTextInputEditingState_DontForceRestartOnNewSamsungKeyboard() { | ||
| // Initialize a TextInputPlugin with a Samsung keypad. | ||
| ShadowBuild.setManufacturer("samsung"); | ||
| final ShadowPackageManager packageManager = | ||
| Shadows.shadowOf( | ||
| RuntimeEnvironment.application.getApplicationContext().getPackageManager()); | ||
| final PackageInfo info = new PackageInfo(); | ||
| info.packageName = "com.sec.android.inputmethod"; | ||
| info.versionCode = 333183070; | ||
| packageManager.addPackage(info); | ||
| 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)); | ||
| 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 NOT restarted the input. | ||
| assertEquals(1, testImm.getRestartCount(testView)); | ||
| } | ||
|
|
||
| @Test | ||
| public void setTextInputEditingState_doesNotRestartOnUnaffectedDevices() { | ||
| // Initialize a TextInputPlugin that needs to be always restarted. | ||
|
|
@@ -425,7 +488,7 @@ public void setTextInputEditingState_doesNotRestartOnUnaffectedDevices() { | |
| textInputPlugin.setTextInputEditingState( | ||
| testView, new TextInputChannel.TextEditState("", 0, 0, -1, -1)); | ||
|
|
||
| // Verify that we've restarted the input. | ||
| // Verify that we've NOT restarted the input. | ||
| assertEquals(1, testImm.getRestartCount(testView)); | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This API was added in API level 28 (Pie) but the check above only requires Lollipop to get to this point. This is crashing on old devices, and blocking everything in the flutter/plugins tree. Please fix or revert this ASAP.