Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: We actually don't want variables prefixed with m, it goes against Google Java style. The other members in the class already have it but I'd avoid matching it going forward. Ideally somebody should come back here later and clean the other names up so they fit the style guide.


@SuppressWarnings("deprecation")
public InputConnectionAdaptor(
View view,
Expand All @@ -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.
Expand Down Expand Up @@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe lead with "Samsung keyboards don't clear the composing region on finishComposingText. [... current text ...]

CursorAnchorInfo.Builder builder = new CursorAnchorInfo.Builder();
builder.setComposingText(-1, "");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional/subjective nit here and throughout: It makes API calls like this easier to read if the primitives are annotated so you can tell what they're supposed to be controlling.

Suggested change
builder.setComposingText(-1, "");
builder.setComposingText(/*composingTextStart=*/-1, /*composingText=*/"");

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea, much clearer what this is doing!

CursorAnchorInfo anchorInfo = builder.build();
mImm.updateCursorAnchorInfo(mFlutterView, anchorInfo);
}
// TODO(garyq): There is still a duplication case that comes from hiding+showing the keyboard.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hiding and showing the keyboard can restart the IMM. May be related?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't have a solid answer here, but in my experimentation, restarting IMM did not seem to have any noticeable effect on this behavior.

// 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;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume if they fix this bug in a future release, we'll need to change this logic. Is there no way we can narrow this down further?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this hack should be fully compatible with other keyboards. Anything we do here is not 1: telling the keyboard something that it should already know, or 2: performing an action that we are not guaranteed to immediately revert.

It seems unlikely to me that Samsung will release a fix for all versions as well as distribute to the hundreds of millions of devices in the wild.

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);
Expand Down