Restart all modern Samsung keyboard IMM #12780
Conversation
| ? subtype.getLanguageTag() | ||
| : subtype.getLocale(); | ||
| return Build.MANUFACTURER.equals("samsung") && language.equals("ko"); | ||
| String keyboardName = Settings.Secure.getString(mView.getContext().getContentResolver(), Settings.Secure.DEFAULT_INPUT_METHOD); |
There was a problem hiding this comment.
Does this API call read from disk? I thought this API was slow because it ended up doing I/O, but I could be mistaken here.
There was a problem hiding this comment.
There seems to be no mention of it storing it on disk, which I would imagine if it were stored on disk, would at least be mentioned a few times.
There was a problem hiding this comment.
It reads from something called sNameValueCache, which I am assuming reads from disk at some point but the values should be cached when we use it here.
There was a problem hiding this comment.
In any case, I added short circuiting to prevent non-samsung devices from performing the lookup.
There was a problem hiding this comment.
Thanks for looking into it, this LGTM.
| ? subtype.getLanguageTag() | ||
| : subtype.getLocale(); | ||
| return Build.MANUFACTURER.equals("samsung") && language.equals("ko"); | ||
| String keyboardName = Settings.Secure.getString(mView.getContext().getContentResolver(), Settings.Secure.DEFAULT_INPUT_METHOD); |
There was a problem hiding this comment.
Thanks for looking into it, this LGTM.
| // All modern Samsung keybords are affected including non-korean languages and thus | ||
| // need the restart. | ||
| @Test | ||
| public void setTextInputEditingState_alwaysRestartsOnAffectedDevices2() { |
There was a problem hiding this comment.
The tests are failing because they're running on sdk=16, so the LOLLIPOP check is blocking them. You can change them by adding sdk=27 in the @Config annotation at the top.
On a second look, I'd also just replace the existing setTextInputEditingState_alwaysRestartsOnAffectedDevices with this one since we don't care about special casing the ko locale anymore. I think leaving that test around is a little confusing now.
justinmc
left a comment
justinmc
left a comment
There was a problem hiding this comment.
LGTM, thanks for following up and fixing this one too! Should make people very happy.
|
Do you think this issue be fixed by this as well? flutter/flutter#30656 |
|
One more that might be fixed by this... 🤞 flutter/flutter#40153 |
|
Well, the common thread seems to be Samsung keyboard, so hopefully these problems all go away... |
[email protected]:flutter/engine.git/compare/7d90779bb66f...1d62160 git log 7d90779..1d62160 --no-merges --oneline 2019-10-04 [email protected] Prettify all CMX files (flutter/engine#12800) 2019-10-04 [email protected] Restart all modern Samsung keyboard IMM (flutter/engine#12780) 2019-10-04 [email protected] Reland fuchsia build improvements (flutter/engine#12795) 2019-10-04 [email protected] Fixing selection issues in Firefox (flutter/engine#12793) 2019-10-04 [email protected] Disable EmbedderTest::CanLaunchAndShutdownMultipleTimes. (flutter/engine#12799) 2019-10-04 [email protected] [flutter_runner] Update the cmx files to include TZ support (flutter/engine#12798) 2019-10-04 [email protected] Roll src/third_party/skia fbdf48ecb204..95edac1c9a4a (1 commits) (flutter/engine#12790) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
[email protected]:flutter/engine.git/compare/7d90779bb66f...1d62160 git log 7d90779..1d62160 --no-merges --oneline 2019-10-04 [email protected] Prettify all CMX files (flutter/engine#12800) 2019-10-04 [email protected] Restart all modern Samsung keyboard IMM (flutter/engine#12780) 2019-10-04 [email protected] Reland fuchsia build improvements (flutter/engine#12795) 2019-10-04 [email protected] Fixing selection issues in Firefox (flutter/engine#12793) 2019-10-04 [email protected] Disable EmbedderTest::CanLaunchAndShutdownMultipleTimes. (flutter/engine#12799) 2019-10-04 [email protected] [flutter_runner] Update the cmx files to include TZ support (flutter/engine#12798) 2019-10-04 [email protected] Roll src/third_party/skia fbdf48ecb204..95edac1c9a4a (1 commits) (flutter/engine#12790) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-engine-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To report a problem with the AutoRoller itself, please file a bug: https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+/master/autoroll/README.md
|
Awesome! Thanks so much for fixing this! |
|
Locking caps lock and then typing resets the locked Caps lock after 1 character is typed on "Samsung keyboard" , in Text field |
6 participants
Turns out the restart workaround introduced in #12432 is applicable to all languages in the Samsung keyboard.
This PR enables the workaround on all Samsung devices using Samsung keyboard with Android Lollipop and newer as the oldest affected device shipped with Lollipop.
Fixes flutter/flutter#31512 (workaround)