[Android Text Input] Remove Samsung restart input workaround for newer Samsung keyboards #24288

LongCatIsLooong · 2021-02-08T20:31:17Z

Fixes flutter/flutter#73433, where holding the backspace on a Samsung keyboard only deletes 1 character at a time. To delete more characters you'd have to press the backspace key again.

The Samsung "always-restart-input" workaround doesn't seem to be needed anymore on newer Samsung keyboards (tested on a Galaxy Tab A SMT510, Samsung Keyboard 3.3.31.83, the duplication bug didn't happen with this patch applied).

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.
The reviewer has submitted any presubmit flakes in this PR using the engine presubmit flakes form before re-triggering the failure.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

justinmc · 2021-02-08T22:42:31Z

I tried this on my S6 and I think the duplication bug is still happening. Can you try this? Using the Samsung Korean keyboard and a text field, type:

Type: ㄴㅇ
Tap once to move the cursor to the start of the field.
Type: ㄴ
Result: ㅇㄴㄴㅇ
Expected: ㄴㄴㅇ

LongCatIsLooong · 2021-02-08T23:20:36Z

@justinmc thanks for checking! On my SM-T510 it works fine:

example.mp4

justinmc · 2021-02-08T23:32:41Z

Hmm, do you have any other Samsung devices lying around? Here is a list of a bunch of devices we were able to reproduce the original bug on: flutter/flutter#31512 (comment)

LongCatIsLooong · 2021-02-08T23:50:55Z

unfortunately that's the only Samsung device I have. It's running Samsung Keyboard version 3.3.31.83, maybe an update changed how composing regions are handled?

justinmc · 2021-02-09T00:27:44Z

v2.0.01-44 for me.

LongCatIsLooong · 2021-02-09T03:24:22Z

I switched to 1.12.13+hotfix8 and still couldn't repro. Looks like when I connect the tablet to download the Korean language support the keyboard updated itself and fixed the composing region bug.

hamdikahloun · 2021-02-10T18:37:51Z

Remote samsung devices for debugging and testing : https://developer.samsung.com/remotetestlab/rtlDeviceList.action

justinmc

LGTM 👍

Hopefully old keyboards like mine are rare and getting rarer, and the vast majority of users get a fully-functioning keyboard.

justinmc · 2021-02-10T22:24:18Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

+    }
+
+    // 3.3.31.83 is a known version that's free of the aforementioned bug.
+    // TODO(LongCatIsLooong): Find the minimum version that has the fix.


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.

Unfortunately I am still not able to reset the IME version. I've tried adb uninstall --user 0. It did uninstall the keyboard but it still wouldn't let me install an earlier version (says downgrade version).

@hamdikahloun thanks for the pointer! It's pretty handy.

justinmc · 2021-02-10T22:25:02Z

shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java

+  public void setTextInputEditingState_alwaysRestartsOnAffectedDevices() {
+    // Initialize a TextInputPlugin with a Samsung keypad.
    ShadowBuild.setManufacturer("samsung");
+    // final ShadowPackageManager packageManager = RuntimeEnvironment.getPackageManager();


Commented line that maybe you meant to remove.

justinmc · 2021-02-10T22:26:05Z

shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java

+    final PackageInfo info = new PackageInfo();
+    info.packageName = "com.sec.android.inputmethod";
+    info.versionCode = 200000000;
+    packageManager.addPackage(info);


It's nice that you were able to fake this 👍

justinmc · 2021-02-10T22:26:59Z

shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java

        testView, new TextInputChannel.TextEditState("", 0, 0, -1, -1));

-    // Verify that we've restarted the input.
+    // Verify that we've NOT restarted the input.


I think this comment should have stayed as it was?

chinmaygarde · 2021-02-11T22:38:36Z

This looks good to go. Can we land this please?

…for newer Samsung keyboards (flutter/engine#24288)

* 1dc8695 Replace Flutter surface only after all platform views are destroyed (flutter/engine#24363) * 0d4accf Roll Skia from ec24154521f3 to 28698696f1b3 (4 revisions) (flutter/engine#24374) * 454a34f [canvaskit] support adding leaf layers w/o container layers (flutter/engine#24357) * 2edeb47 Roll Dart SDK from 1e0fec6e48cf to add1eac3622c (1 revision) (flutter/engine#24375) * 7376d18 Roll Skia from 28698696f1b3 to 3354f8c4f8cb (2 revisions) (flutter/engine#24378) * 157797e Fix vulkan surface leaks. (flutter/engine#24372) * 72ed416 Roll Skia from 3354f8c4f8cb to 2494942f0d8d (4 revisions) (flutter/engine#24380) * 6747b83 Roll Skia from 2494942f0d8d to 6e73404a782c (3 revisions) (flutter/engine#24383) * 48cf2b9 [canvaskit] fix Path.from (flutter/engine#24382) * 42019c8 Roll Fuchsia Linux SDK from YMY-cajLa... to AjjJVX_95... (flutter/engine#24381) * c3f0894 Roll Skia from 6e73404a782c to ba4c3086ba80 (3 revisions) (flutter/engine#24388) * 4f33430 Roll Dart SDK from add1eac3622c to 6eafd1856eb5 (1 revision) (flutter/engine#24390) * 7661388 Roll Fuchsia Mac SDK from WIqUEANCH... to yyQmi032P... (flutter/engine#24391) * 6b33d4e Roll Skia from ba4c3086ba80 to 4f065e286df6 (2 revisions) (flutter/engine#24393) * 623d9ce Windows: linker compatibility with AppContainer for winuwp target (flutter/engine#24318) * 89c8b6e Roll Dart SDK from 6eafd1856eb5 to e9693f1b2401 (1 revision) (flutter/engine#24394) * 447733c Roll Fuchsia Linux SDK from AjjJVX_95... to uJ8jPHroy... (flutter/engine#24395) * 7b1efcf Roll Fuchsia Mac SDK from yyQmi032P... to NQL9o1B8n... (flutter/engine#24396) * 08dcad7 Roll Dart SDK from e9693f1b2401 to 4c8147b7b41e (1 revision) (flutter/engine#24397) * 102cb3c Roll Skia from 4f065e286df6 to 554aabbaa8e8 (1 revision) (flutter/engine#24398) * 780ba15 Roll Dart SDK from 4c8147b7b41e to 56fa015ca3ec (1 revision) (flutter/engine#24402) * 9286135 Roll Skia from 554aabbaa8e8 to 7a2ff98ce68b (1 revision) (flutter/engine#24403) * 95cc4ce Roll buildroot to e7857d2 (flutter/engine#24399) * 9bc4bf1 Revert "Roll buildroot to e7857d2 (#24399)" * b8126b6 Roll Skia from 7a2ff98ce68b to c3e152b9f795 (2 revisions) (flutter/engine#24404) * cfdbd16 Roll Dart SDK from 56fa015ca3ec to c9b47adb178e (1 revision) (flutter/engine#24408) * 6ece8d1 Roll Fuchsia Mac SDK from NQL9o1B8n... to OHm_snJzB... (flutter/engine#24410) * e3d05a1 Roll Fuchsia Linux SDK from uJ8jPHroy... to JmB58d-3R... (flutter/engine#24411) * 491eeeb Roll Skia from c3e152b9f795 to b44fbb3392f8 (3 revisions) (flutter/engine#24414) * 2349ee9 Roll Skia from b44fbb3392f8 to 56a8fbd21ee2 (3 revisions) (flutter/engine#24415) * cce0869 Roll Dart SDK from c9b47adb178e to 648d5f951915 (1 revision) (flutter/engine#24416) * 138ec27 Roll Skia from 56a8fbd21ee2 to 7cb0f6e9702a (1 revision) (flutter/engine#24418) * dbded38 Roll Fuchsia Mac SDK from OHm_snJzB... to H0v5Ox4Vs... (flutter/engine#24419) * 05347d4 Roll Fuchsia Linux SDK from JmB58d-3R... to KKD5xVYLO... (flutter/engine#24420) * 5659b56 Roll Dart SDK from 648d5f951915 to 39dc63439396 (1 revision) (flutter/engine#24421) * 0561a98 Add missing header guard, namespace (flutter/engine#24423) * 3e91ee2 Adjust header guards for updated paths (flutter/engine#24424) * d71c030 Roll Skia from 7cb0f6e9702a to 330578e81158 (1 revision) (flutter/engine#24427) * dcb0c0f Roll Fuchsia Mac SDK from H0v5Ox4Vs... to eG4TqZBn9... (flutter/engine#24425) * d7705f9 Roll Fuchsia Linux SDK from KKD5xVYLO... to C2FaCuCLB... (flutter/engine#24430) * e1f03cd Roll Dart SDK from 39dc63439396 to 4db0f5599c85 (1 revision) (flutter/engine#24431) * 1687040 [Android Text Input] Remove Samsung restart input workaround for newer Samsung keyboards (flutter/engine#24288) * bee0d2a Roll Skia from 330578e81158 to 4a3ec173b31b (5 revisions) (flutter/engine#24434) * 8490ec2 [ci] Remove null safety experiment flag (flutter/engine#24392) * 6993cb2 Revert Dart SDK to 1e0fec6e48cf89f46973af38a5c2de4bc760a135 (flutter/engine#24436)

stuartmorgan-g · 2021-02-18T17:49:20Z

shell/platform/android/io/flutter/plugin/editing/TextInputPlugin.java

+              .getContext()
+              .getPackageManager()
+              .getPackageInfo("com.sec.android.inputmethod", 0)
+              .getLongVersionCode();


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.

justinmc · 2021-02-18T18:01:38Z

There is an issue open about a similar problem on a different keyboard: a Japanese keyboard called Simeji flutter/flutter#72980.

Edit: I missed the comment above about reverting this, my comment was unrelated.

…for newer Samsung keyboards (#24288)" This reverts commit 1687040.

…for newer Samsung keyboards (#24288)" (#24486)

newer Samsung keyboards" flutter#24288

…r Samsung keyboards (flutter#24288)

…for newer Samsung keyboards (flutter#24288)" (flutter#24486)

remove samsung restart input workaround

ee157f3

LongCatIsLooong requested review from GaryQian, jason-simmons and justinmc February 8, 2021 20:31

flutter-dashboard bot added the platform-android label Feb 8, 2021

google-cla bot added the cla: yes label Feb 8, 2021

LongCatIsLooong marked this pull request as draft February 9, 2021 03:59

Remove the Samsung compsoing region fix on newer Samsung keyboards

33fdb7f

LongCatIsLooong changed the title ~~[Android Text Input] Remove Samsung restart input workaround~~ [Android Text Input] Remove Samsung restart input workaround for newer Samsung keyboards Feb 10, 2021

LongCatIsLooong marked this pull request as ready for review February 10, 2021 08:28

justinmc approved these changes Feb 10, 2021

View reviewed changes

LongCatIsLooong added 2 commits February 10, 2021 15:43

review

569de83

lower version

4385f6b

LongCatIsLooong force-pushed the remove-samsung-input-restart branch from 1c026f4 to 4385f6b Compare February 11, 2021 02:17

LongCatIsLooong added the waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. label Feb 16, 2021

fluttergithubbot merged commit 1687040 into flutter:master Feb 16, 2021

LongCatIsLooong deleted the remove-samsung-input-restart branch February 16, 2021 17:30

engine-flutter-autoroll mentioned this pull request Feb 16, 2021

Roll Engine from 1d537824d6d2 to 16870408831e (42 revisions) flutter/flutter#76137

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 16, 2021

1687040 [Android Text Input] Remove Samsung restart input workaround …

9f064d4

…for newer Samsung keyboards (flutter/engine#24288)

engine-flutter-autoroll mentioned this pull request Feb 16, 2021

Roll Engine from 1d537824d6d2 to 8490ec2ac672 (44 revisions) flutter/flutter#76142

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 16, 2021

1687040 [Android Text Input] Remove Samsung restart input workaround …

92d77d1

…for newer Samsung keyboards (flutter/engine#24288)

engine-flutter-autoroll mentioned this pull request Feb 16, 2021

Roll Engine from 1d537824d6d2 to 8490ec2ac672 (44 revisions) flutter/flutter#76147

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 16, 2021

1687040 [Android Text Input] Remove Samsung restart input workaround …

8d0a6d8

…for newer Samsung keyboards (flutter/engine#24288)

engine-flutter-autoroll mentioned this pull request Feb 16, 2021

Roll Engine from 1d537824d6d2 to 6993cb229b99 (45 revisions) flutter/flutter#76151

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 16, 2021

1687040 [Android Text Input] Remove Samsung restart input workaround …

fe62340

…for newer Samsung keyboards (flutter/engine#24288)

stuartmorgan-g reviewed Feb 18, 2021

View reviewed changes

LongCatIsLooong added a commit that referenced this pull request Feb 18, 2021

Revert "[Android Text Input] Remove Samsung restart input workaround …

0ea829a

…for newer Samsung keyboards (#24288)" This reverts commit 1687040.

LongCatIsLooong mentioned this pull request Feb 18, 2021

Revert "[Android Text Input] Remove Samsung restart input workaround for newer Samsung keyboards" #24486

Merged

fluttergithubbot pushed a commit that referenced this pull request Feb 18, 2021

Revert "[Android Text Input] Remove Samsung restart input workaround …

4956ad9

…for newer Samsung keyboards (#24288)" (#24486)

LongCatIsLooong added a commit to LongCatIsLooong/engine that referenced this pull request Feb 19, 2021

reland "[Android Text Input] Remove Samsung restart input workaround for

a38935b

newer Samsung keyboards" flutter#24288

LongCatIsLooong mentioned this pull request Feb 19, 2021

Reland "[Android Text Input] Remove Samsung restart input workaround for newer Samsung keyboards" #24288 #24500

Closed

9 tasks

hjfreyer pushed a commit to hjfreyer/engine that referenced this pull request Mar 22, 2021

[Android Text Input] Remove Samsung restart input workaround for newe…

46b1ba0

…r Samsung keyboards (flutter#24288)

hjfreyer pushed a commit to hjfreyer/engine that referenced this pull request Mar 22, 2021

Revert "[Android Text Input] Remove Samsung restart input workaround …

0c08a88

…for newer Samsung keyboards (flutter#24288)" (flutter#24486)

[Android Text Input] Remove Samsung restart input workaround for newer Samsung keyboards #24288

[Android Text Input] Remove Samsung restart input workaround for newer Samsung keyboards #24288

Uh oh!

Conversation

LongCatIsLooong commented Feb 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

justinmc commented Feb 8, 2021

Uh oh!

LongCatIsLooong commented Feb 8, 2021

Uh oh!

justinmc commented Feb 8, 2021

Uh oh!

LongCatIsLooong commented Feb 8, 2021

Uh oh!

justinmc commented Feb 9, 2021

Uh oh!

LongCatIsLooong commented Feb 9, 2021

Uh oh!

hamdikahloun commented Feb 10, 2021

Uh oh!

justinmc left a comment

Choose a reason for hiding this comment

Uh oh!

justinmc Feb 10, 2021

Choose a reason for hiding this comment

Uh oh!

LongCatIsLooong Feb 11, 2021

Choose a reason for hiding this comment

Uh oh!

justinmc Feb 10, 2021

Choose a reason for hiding this comment

Uh oh!

justinmc Feb 10, 2021

Choose a reason for hiding this comment

Uh oh!

justinmc Feb 10, 2021

Choose a reason for hiding this comment

Uh oh!

chinmaygarde commented Feb 11, 2021

Uh oh!

stuartmorgan-g Feb 18, 2021

Choose a reason for hiding this comment

Uh oh!

justinmc commented Feb 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

LongCatIsLooong commented Feb 8, 2021 •

edited

Loading

justinmc commented Feb 18, 2021 •

edited

Loading