From 403d32d659ffa763e1ce86d772c55c8bc84c346a Mon Sep 17 00:00:00 2001 From: GaryQian Date: Tue, 11 Feb 2020 13:31:39 -0800 Subject: [PATCH 01/10] Samsung duplication bug hack-fix --- .../editing/InputConnectionAdaptor.java | 59 ++++++++++++++++--- 1 file changed, 52 insertions(+), 7 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java index 64e41df5f0047..cb4aedb4883cc 100644 --- a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java +++ b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java @@ -4,8 +4,10 @@ package io.flutter.plugin.editing; +import android.annotation.SuppressLint; import android.content.Context; import android.os.Build; +import android.provider.Settings; import android.text.DynamicLayout; import android.text.Editable; import android.text.Layout; @@ -17,6 +19,7 @@ import android.view.inputmethod.CursorAnchorInfo; import android.view.inputmethod.EditorInfo; import android.view.inputmethod.InputMethodManager; +import android.view.inputmethod.InputMethodSubtype; import io.flutter.Log; import io.flutter.embedding.engine.systemchannels.TextInputChannel; @@ -30,6 +33,9 @@ class InputConnectionAdaptor extends BaseInputConnection { private InputMethodManager mImm; private final Layout mLayout; + // Used to determine if Samsung-specific hacks should be applied. + private final boolean mIsSamsung; + @SuppressWarnings("deprecation") public InputConnectionAdaptor( View view, @@ -56,6 +62,8 @@ public InputConnectionAdaptor( 0.0f, false); mImm = (InputMethodManager) view.getContext().getSystemService(Context.INPUT_METHOD_SERVICE); + + mIsSamsung = isSamsung(); } // Send the current state of the editable to Flutter. @@ -132,19 +140,56 @@ public boolean setComposingText(CharSequence text, int newCursorPosition) { public boolean finishComposingText() { boolean result = super.finishComposingText(); - if (Build.VERSION.SDK_INT >= 21) { - // Update the keyboard with a reset/empty composing region. Critical on - // Samsung keyboards to prevent punctuation duplication. - CursorAnchorInfo.Builder builder = new CursorAnchorInfo.Builder(); - builder.setComposingText(-1, ""); - CursorAnchorInfo anchorInfo = builder.build(); - mImm.updateCursorAnchorInfo(mFlutterView, anchorInfo); + // Apply Samsung hacks. Samsung caches composing region data strangely, causing text duplication. + if (mIsSamsung) { + if (Build.VERSION.SDK_INT >= 21) { + // Update the keyboard with a reset/empty composing region. Critical on + // Samsung keyboards to prevent punctuation duplication. + CursorAnchorInfo.Builder builder = new CursorAnchorInfo.Builder(); + builder.setComposingText(-1, ""); + CursorAnchorInfo anchorInfo = builder.build(); + mImm.updateCursorAnchorInfo(mFlutterView, anchorInfo); + } + // TODO(garyq): There is still a duplication case that comes from hiding+showing the keyboard. + // The exact behavior to cause it has so far been hard to pinpoint and it happens far more + // rarely than the original bug. + + // Temporarily indicate to the IME that the composing region selection should be reset. + // The correct selection is then immediately set properly in the updateEditingState() call + // in this method. This is a hack to trigger Samsung keyboard's internal cache to clear. + // This prevents duplication on keyboard hide+show. See https://github.com/flutter/flutter/issues/31512 + // + // We only do this if the proper selection will be restored later, eg, when mBatchCount is 0. + if (mBatchCount == 0) { + mImm.updateSelection(mFlutterView, -1, -1, -1, -1); + } } updateEditingState(); return result; } + // Detect if the keyboard is a Samsung keyboard, where we apply Samsung-specific hacks to + // fix critical bugs that make the keyboard otherwise unusable. See finishComposingText() for + // more details. + @SuppressLint("NewApi") // New API guard is inline, the linter can't see it. + @SuppressWarnings("deprecation") + private boolean isSamsung() { + 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( + mFlutterView.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"); + } + @Override public boolean setSelection(int start, int end) { boolean result = super.setSelection(start, end); From 014ff7bf1506bdcc8d7f02848ae926765d72f375 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Tue, 11 Feb 2020 16:52:14 -0800 Subject: [PATCH 02/10] Fix formatting --- .../io/flutter/plugin/editing/InputConnectionAdaptor.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java index cb4aedb4883cc..db147be955b70 100644 --- a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java +++ b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java @@ -140,7 +140,8 @@ public boolean setComposingText(CharSequence text, int newCursorPosition) { public boolean finishComposingText() { boolean result = super.finishComposingText(); - // Apply Samsung hacks. Samsung caches composing region data strangely, causing text duplication. + // Apply Samsung hacks. Samsung caches composing region data strangely, causing text + // duplication. if (mIsSamsung) { if (Build.VERSION.SDK_INT >= 21) { // Update the keyboard with a reset/empty composing region. Critical on @@ -157,7 +158,8 @@ public boolean finishComposingText() { // Temporarily indicate to the IME that the composing region selection should be reset. // The correct selection is then immediately set properly in the updateEditingState() call // in this method. This is a hack to trigger Samsung keyboard's internal cache to clear. - // This prevents duplication on keyboard hide+show. See https://github.com/flutter/flutter/issues/31512 + // This prevents duplication on keyboard hide+show. See + // https://github.com/flutter/flutter/issues/31512 // // We only do this if the proper selection will be restored later, eg, when mBatchCount is 0. if (mBatchCount == 0) { From 465a8a257e271dfcd5384689ce06cc794f249d39 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Tue, 11 Feb 2020 18:23:16 -0800 Subject: [PATCH 03/10] Add test --- .../plugin/editing/TextInputPluginTest.java | 84 ++++++++++++++++++- 1 file changed, 83 insertions(+), 1 deletion(-) diff --git a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java index bb0a8ff9fa7ea..ca2f9800779b9 100644 --- a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java +++ b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java @@ -28,6 +28,9 @@ import io.flutter.plugin.common.MethodCall; import io.flutter.plugin.platform.PlatformViewsController; import java.nio.ByteBuffer; +import java.util.List; +import java.util.ArrayList; +import java.util.Arrays; import org.json.JSONArray; import org.json.JSONException; import org.junit.Test; @@ -305,9 +308,17 @@ public void inputConnection_createsActionFromEnter() throws JSONException { @Test public void inputConnection_finishComposingTextUpdatesIMM() throws JSONException { + 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)); + testImm.setCurrentInputMethodSubtype(inputMethodSubtype); FlutterJNI mockFlutterJni = mock(FlutterJNI.class); View testView = new View(RuntimeEnvironment.application); DartExecutor dartExecutor = spy(new DartExecutor(mockFlutterJni, mock(AssetManager.class))); @@ -338,13 +349,58 @@ public void inputConnection_finishComposingTextUpdatesIMM() throws JSONException } } + @Test + public void inputConnection_samsungFinishComposingTextSetsSelection() throws JSONException { + 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)); + testImm.setCurrentInputMethodSubtype(inputMethodSubtype); + FlutterJNI mockFlutterJni = mock(FlutterJNI.class); + View testView = new View(RuntimeEnvironment.application); + DartExecutor dartExecutor = spy(new DartExecutor(mockFlutterJni, mock(AssetManager.class))); + TextInputPlugin textInputPlugin = + new TextInputPlugin(testView, dartExecutor, mock(PlatformViewsController.class)); + textInputPlugin.setTextInputClient( + 0, + new TextInputChannel.Configuration( + false, + false, + true, + TextInputChannel.TextCapitalization.NONE, + new TextInputChannel.InputType(TextInputChannel.TextInputType.TEXT, false, false), + 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)); + InputConnection connection = textInputPlugin.createInputConnection(testView, new EditorInfo()); + + testImm.setTrackSelection(true); + connection.finishComposingText(); + testImm.setTrackSelection(false); + + List expectedSelectionValues = Arrays.asList(0, 0, -1, -1, -1, -1, -1, -1, 0, 0, -1, -1); + assertEquals(testImm.getSelectionUpdateValues(), expectedSelectionValues); + } + @Implements(InputMethodManager.class) public static class TestImm extends ShadowInputMethodManager { private InputMethodSubtype currentInputMethodSubtype; private SparseIntArray restartCounter = new SparseIntArray(); private CursorAnchorInfo cursorAnchorInfo; + private ArrayList selectionUpdateValues; + private boolean trackSelection = false; - public TestImm() {} + public TestImm() { + selectionUpdateValues = new ArrayList(); + } @Implementation public InputMethodSubtype getCurrentInputMethodSubtype() { @@ -370,6 +426,32 @@ public void updateCursorAnchorInfo(View view, CursorAnchorInfo cursorAnchorInfo) this.cursorAnchorInfo = cursorAnchorInfo; } + // We simply store the values to verify later. + @Implementation + public void updateSelection( + View view, + int selStart, + int selEnd, + int candidatesStart, + int candidatesEnd) { + if (trackSelection) { + this.selectionUpdateValues.add(selStart); + this.selectionUpdateValues.add(selEnd); + this.selectionUpdateValues.add(candidatesStart); + this.selectionUpdateValues.add(candidatesEnd); + } + } + + // only track values when enabled via this. + public void setTrackSelection(boolean val) { + trackSelection = val; + } + + // Returns true if the last updateSelection call passed the following values. + public ArrayList getSelectionUpdateValues() { + return selectionUpdateValues; + } + public CursorAnchorInfo getLastCursorAnchorInfo() { return cursorAnchorInfo; } From b0ad38f08a4596d75348ff857af6420a985c19c9 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 12 Feb 2020 11:23:59 -0800 Subject: [PATCH 04/10] COmments --- .../io/flutter/plugin/editing/InputConnectionAdaptor.java | 1 + 1 file changed, 1 insertion(+) diff --git a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java index db147be955b70..46176b5be184b 100644 --- a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java +++ b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java @@ -144,6 +144,7 @@ public boolean finishComposingText() { // duplication. if (mIsSamsung) { if (Build.VERSION.SDK_INT >= 21) { + // Samsung keyboards don't clear the composing region on finishComposingText. // Update the keyboard with a reset/empty composing region. Critical on // Samsung keyboards to prevent punctuation duplication. CursorAnchorInfo.Builder builder = new CursorAnchorInfo.Builder(); From 11d88fddba3c0eb819c4104fdc2a737a05ce76f0 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 12 Feb 2020 12:34:12 -0800 Subject: [PATCH 05/10] Formatting --- .../plugin/editing/TextInputPluginTest.java | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java index ca2f9800779b9..149403b7e968e 100644 --- a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java +++ b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java @@ -386,7 +386,8 @@ public void inputConnection_samsungFinishComposingTextSetsSelection() throws JSO connection.finishComposingText(); testImm.setTrackSelection(false); - List expectedSelectionValues = Arrays.asList(0, 0, -1, -1, -1, -1, -1, -1, 0, 0, -1, -1); + List expectedSelectionValues = + Arrays.asList(0, 0, -1, -1, -1, -1, -1, -1, 0, 0, -1, -1); assertEquals(testImm.getSelectionUpdateValues(), expectedSelectionValues); } @@ -399,7 +400,7 @@ public static class TestImm extends ShadowInputMethodManager { private boolean trackSelection = false; public TestImm() { - selectionUpdateValues = new ArrayList(); + selectionUpdateValues = new ArrayList(); } @Implementation @@ -429,11 +430,7 @@ public void updateCursorAnchorInfo(View view, CursorAnchorInfo cursorAnchorInfo) // We simply store the values to verify later. @Implementation public void updateSelection( - View view, - int selStart, - int selEnd, - int candidatesStart, - int candidatesEnd) { + View view, int selStart, int selEnd, int candidatesStart, int candidatesEnd) { if (trackSelection) { this.selectionUpdateValues.add(selStart); this.selectionUpdateValues.add(selEnd); @@ -444,12 +441,12 @@ public void updateSelection( // only track values when enabled via this. public void setTrackSelection(boolean val) { - trackSelection = val; + trackSelection = val; } // Returns true if the last updateSelection call passed the following values. public ArrayList getSelectionUpdateValues() { - return selectionUpdateValues; + return selectionUpdateValues; } public CursorAnchorInfo getLastCursorAnchorInfo() { From 5dce04978d62a547130060fc3c026df2269e5b2c Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 12 Feb 2020 12:40:04 -0800 Subject: [PATCH 06/10] Formatting --- .../test/io/flutter/plugin/editing/TextInputPluginTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java index 149403b7e968e..51dbc603b9793 100644 --- a/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java +++ b/shell/platform/android/test/io/flutter/plugin/editing/TextInputPluginTest.java @@ -28,9 +28,9 @@ import io.flutter.plugin.common.MethodCall; import io.flutter.plugin.platform.PlatformViewsController; import java.nio.ByteBuffer; -import java.util.List; import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import org.json.JSONArray; import org.json.JSONException; import org.junit.Test; From cbfed0e8a960a6495387c6054e3d8fce34c42e1f Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 12 Feb 2020 12:53:57 -0800 Subject: [PATCH 07/10] Nits --- .../plugin/editing/InputConnectionAdaptor.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java index 46176b5be184b..a33cc45b55921 100644 --- a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java +++ b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java @@ -34,7 +34,7 @@ class InputConnectionAdaptor extends BaseInputConnection { private final Layout mLayout; // Used to determine if Samsung-specific hacks should be applied. - private final boolean mIsSamsung; + private final boolean isSamsung; @SuppressWarnings("deprecation") public InputConnectionAdaptor( @@ -63,7 +63,7 @@ public InputConnectionAdaptor( false); mImm = (InputMethodManager) view.getContext().getSystemService(Context.INPUT_METHOD_SERVICE); - mIsSamsung = isSamsung(); + isSamsung = isSamsung(); } // Send the current state of the editable to Flutter. @@ -142,13 +142,13 @@ public boolean finishComposingText() { // Apply Samsung hacks. Samsung caches composing region data strangely, causing text // duplication. - if (mIsSamsung) { + if (isSamsung) { if (Build.VERSION.SDK_INT >= 21) { // Samsung keyboards don't clear the composing region on finishComposingText. // Update the keyboard with a reset/empty composing region. Critical on // Samsung keyboards to prevent punctuation duplication. CursorAnchorInfo.Builder builder = new CursorAnchorInfo.Builder(); - builder.setComposingText(-1, ""); + builder.setComposingText(/*composingTextStart*/-1, /*composingText*/""); CursorAnchorInfo anchorInfo = builder.build(); mImm.updateCursorAnchorInfo(mFlutterView, anchorInfo); } @@ -164,7 +164,8 @@ public boolean finishComposingText() { // // We only do this if the proper selection will be restored later, eg, when mBatchCount is 0. if (mBatchCount == 0) { - mImm.updateSelection(mFlutterView, -1, -1, -1, -1); + mImm.updateSelection( + mFlutterView, /*selStart*/-1, /*selEnd*/-1, /*candidatesStart*/-1, /*candidatesEnd*/-1); } } From f01c9855e26032c7c1d579fcbd125b260abe2e6b Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 12 Feb 2020 13:50:14 -0800 Subject: [PATCH 08/10] Formatting --- .../flutter/plugin/editing/InputConnectionAdaptor.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java index a33cc45b55921..05c0a25cffef1 100644 --- a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java +++ b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java @@ -148,7 +148,7 @@ public boolean finishComposingText() { // Update the keyboard with a reset/empty composing region. Critical on // Samsung keyboards to prevent punctuation duplication. CursorAnchorInfo.Builder builder = new CursorAnchorInfo.Builder(); - builder.setComposingText(/*composingTextStart*/-1, /*composingText*/""); + builder.setComposingText(/*composingTextStart*/ -1, /*composingText*/ ""); CursorAnchorInfo anchorInfo = builder.build(); mImm.updateCursorAnchorInfo(mFlutterView, anchorInfo); } @@ -165,7 +165,12 @@ public boolean finishComposingText() { // We only do this if the proper selection will be restored later, eg, when mBatchCount is 0. if (mBatchCount == 0) { mImm.updateSelection( - mFlutterView, /*selStart*/-1, /*selEnd*/-1, /*candidatesStart*/-1, /*candidatesEnd*/-1); + mFlutterView, + -1, /*selStart*/ + -1, /*selEnd*/ + -1, /*candidatesStart*/ + -1, /*candidatesEnd*/ + ); } } From 541ebeefa5bf08d1368e71dd77c5dea973a567ca Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 12 Feb 2020 14:07:28 -0800 Subject: [PATCH 09/10] Typo --- .../io/flutter/plugin/editing/InputConnectionAdaptor.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java index 05c0a25cffef1..243dd544b6699 100644 --- a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java +++ b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java @@ -169,7 +169,7 @@ public boolean finishComposingText() { -1, /*selStart*/ -1, /*selEnd*/ -1, /*candidatesStart*/ - -1, /*candidatesEnd*/ + -1 /*candidatesEnd*/ ); } } From c447211622cfcdccbd1120dca92f9bed27c87c04 Mon Sep 17 00:00:00 2001 From: GaryQian Date: Wed, 12 Feb 2020 15:39:08 -0800 Subject: [PATCH 10/10] FOrmatting again --- .../io/flutter/plugin/editing/InputConnectionAdaptor.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java index 243dd544b6699..20b11c68024f4 100644 --- a/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java +++ b/shell/platform/android/io/flutter/plugin/editing/InputConnectionAdaptor.java @@ -169,8 +169,7 @@ public boolean finishComposingText() { -1, /*selStart*/ -1, /*selEnd*/ -1, /*candidatesStart*/ - -1 /*candidatesEnd*/ - ); + -1 /*candidatesEnd*/); } }