Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 4 commits
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 @@ -43,8 +43,7 @@ class InputConnectionAdaptor extends BaseInputConnection {
// Used to determine if Samsung-specific hacks should be applied.
private final boolean isSamsung;

private boolean mRepeatCheckNeeded = false;
private TextEditingValue mLastSentTextEditngValue;
private TextEditingValue mLastKnownTextEditingValue;
// Data class used to get and store the last-sent values via updateEditingState to
// the framework. These are then compared against to prevent redundant messages
// with the same data before any valid operations were made to the contents.
Expand Down Expand Up @@ -144,7 +143,7 @@ private void updateEditingState() {
// occurred to mark this as dirty. This prevents duplicate remote updates of
// the same data, which can break formatters that change the length of the
// contents.
if (mRepeatCheckNeeded && currentValue.equals(mLastSentTextEditngValue)) {
if (currentValue.equals(mLastKnownTextEditingValue)) {
return;
}

Expand All @@ -163,17 +162,13 @@ private void updateEditingState() {
currentValue.composingStart,
currentValue.composingEnd);

mRepeatCheckNeeded = true;
mLastSentTextEditngValue = currentValue;
mLastKnownTextEditingValue = currentValue;
}

// This should be called whenever a change could have been made to
// the value of mEditable, which will make any call of updateEditingState()
// ineligible for repeat checking as we do not want to skip sending real changes
// to the framework.
public void markDirty() {
// Disable updateEditngState's repeat-update check
mRepeatCheckNeeded = false;
// Called when the current text editing state held by the text input plugin is overwritten by a
// newly received value from the framework.
public void didUpdateEditingValue() {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The android text input plugin doesn't seem to completely overwrite the input state with the newly received value, the composing range is ignored. If the text is composing and the user replaces the entire string with something else, would that put the input method in a broken state?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Have you tried it out? Ignoring the composing region definitely seems incorrect. If there's no visible bug, maybe it's fixing itself when subsequent edits happen.

@LongCatIsLooong LongCatIsLooong Oct 13, 2020

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I tried a 3rd party japanese IME that uses composing region, it's not working in flutter apps (not composing at all). I'll see if there's an easy fix.

update it seems the Japanese input bug fixed itself.

mLastKnownTextEditingValue = new TextEditingValue(mEditable);
}

@Override
Expand All @@ -198,7 +193,6 @@ public boolean endBatchEdit() {
@Override
public boolean commitText(CharSequence text, int newCursorPosition) {
boolean result = super.commitText(text, newCursorPosition);
markDirty();
return result;
}

Expand All @@ -207,21 +201,18 @@ public boolean deleteSurroundingText(int beforeLength, int afterLength) {
if (Selection.getSelectionStart(mEditable) == -1) return true;

boolean result = super.deleteSurroundingText(beforeLength, afterLength);
markDirty();
return result;
}

@Override
public boolean deleteSurroundingTextInCodePoints(int beforeLength, int afterLength) {
boolean result = super.deleteSurroundingTextInCodePoints(beforeLength, afterLength);
markDirty();
return result;
}

@Override
public boolean setComposingRegion(int start, int end) {
boolean result = super.setComposingRegion(start, end);
markDirty();
return result;
}

Expand All @@ -233,7 +224,6 @@ public boolean setComposingText(CharSequence text, int newCursorPosition) {
} else {
result = super.setComposingText(text, newCursorPosition);
}
markDirty();
return result;
}

Expand All @@ -255,7 +245,6 @@ public boolean finishComposingText() {
}
}

markDirty();
return result;
}

Expand All @@ -272,7 +261,6 @@ public ExtractedText getExtractedText(ExtractedTextRequest request, int flags) {
@Override
public boolean clearMetaKeyStates(int states) {
boolean result = super.clearMetaKeyStates(states);
markDirty();
return result;
}

Expand Down Expand Up @@ -300,7 +288,6 @@ private boolean isSamsung() {
@Override
public boolean setSelection(int start, int end) {
boolean result = super.setSelection(start, end);
markDirty();
updateEditingState();
return result;
}
Expand All @@ -323,7 +310,6 @@ private static int clampIndexToEditable(int index, Editable editable) {

@Override
public boolean sendKeyEvent(KeyEvent event) {
markDirty();
if (event.getAction() == KeyEvent.ACTION_DOWN) {
if (event.getKeyCode() == KeyEvent.KEYCODE_DEL) {
int selStart = clampIndexToEditable(Selection.getSelectionStart(mEditable), mEditable);
Expand Down Expand Up @@ -427,7 +413,6 @@ public boolean sendKeyEvent(KeyEvent event) {

@Override
public boolean performContextMenuAction(int id) {
markDirty();
if (id == android.R.id.selectAll) {
setSelection(0, mEditable.length());
return true;
Expand Down Expand Up @@ -487,7 +472,6 @@ public boolean performPrivateCommand(String action, Bundle data) {

@Override
public boolean performEditorAction(int actionCode) {
markDirty();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It's great seeing all these markDirty's going away. This seems so much simpler.

switch (actionCode) {
case EditorInfo.IME_ACTION_NONE:
textInputChannel.newline(mClient);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -595,14 +595,13 @@ void setTextInputEditingState(View view, TextInputChannel.TextEditState state) {
if (!state.text.equals(mEditable.toString())) {
mEditable.replace(0, mEditable.length(), state.text);
}

// Notify the autofill manager of the value change.
notifyValueChanged(mEditable.toString());
// Always apply state to selection which handles updating the selection if needed.
applyStateToSelection(state);
InputConnection connection = getLastInputConnection();
if (connection != null && connection instanceof InputConnectionAdaptor) {
((InputConnectionAdaptor) connection).markDirty();
}
// Use updateSelection to update imm on selection if it is not neccessary to restart.

// Use updateSelection to update imm on selection if it is not necessary to restart.
if (!restartAlwaysRequired && !mRestartInputPending) {
mImm.updateSelection(
mView,
Expand All @@ -616,6 +615,12 @@ void setTextInputEditingState(View view, TextInputChannel.TextEditState state) {
mImm.restartInput(view);
mRestartInputPending = false;
}

// Notify the connection adaptor that the last known remote editing state has been updated.
InputConnection connection = getLastInputConnection();
if (connection != null && connection instanceof InputConnectionAdaptor) {
((InputConnectionAdaptor) connection).didUpdateEditingValue();
}
}

private interface MinMax {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -943,6 +943,46 @@ public void inputConnectionAdaptor_RepeatFilter() throws NullPointerException {
assertEquals(textInputChannel.selectionEnd, 4);
}

@Test
public void inputConnectionAdaptor_skipCallsAfterEditingValueUpdateFromFramework() {
View testView = new View(RuntimeEnvironment.application);
FlutterJNI mockFlutterJni = mock(FlutterJNI.class);
DartExecutor dartExecutor = spy(new DartExecutor(mockFlutterJni, mock(AssetManager.class)));
int inputTargetId = 0;
TestTextInputChannel textInputChannel = new TestTextInputChannel(dartExecutor);
AndroidKeyProcessor mockKeyProcessor = mock(AndroidKeyProcessor.class);
Editable mEditable = Editable.Factory.getInstance().newEditable("");
EditorInfo outAttrs = new EditorInfo();
outAttrs.inputType = InputType.TYPE_CLASS_TEXT | InputType.TYPE_TEXT_FLAG_MULTI_LINE;

InputConnectionAdaptor inputConnectionAdaptor =
new InputConnectionAdaptor(
testView, inputTargetId, textInputChannel, mockKeyProcessor, mEditable, outAttrs);

inputConnectionAdaptor.setComposingText("initial text", 1);
assertEquals(textInputChannel.text, "initial text");
// Calls updateEditingState.
inputConnectionAdaptor.beginBatchEdit();
inputConnectionAdaptor.endBatchEdit();
assertEquals(textInputChannel.text, "initial text");
// Do send update to the framework.
assertEquals(textInputChannel.updateEditingStateInvocations, 1);

// Change the internal state and pretend this is from the framework.
mEditable.replace(0, mEditable.length(), "updated text from the framework");
inputConnectionAdaptor.didUpdateEditingValue();
// Does not send update because it is the same as the state we just received.
assertEquals(textInputChannel.updateEditingStateInvocations, 1);

mEditable.replace(0, mEditable.length(), "yet another updated text from the framework");
// Calls updateEditingState.
inputConnectionAdaptor.beginBatchEdit();
inputConnectionAdaptor.endBatchEdit();
// Does send update because it is the same as the state we just received.
assertEquals(textInputChannel.text, "yet another updated text from the framework");
assertEquals(textInputChannel.updateEditingStateInvocations, 2);
}

@Test
public void testSendKeyEvent_delKeyDeletesBackward() {
int selStart = 29;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,46 @@ public void textInputPlugin_RequestsReattachOnCreation() throws JSONException {
verifyMethodCall(bufferCaptor.getValue(), "TextInputClient.requestExistingInputState", null);
}

@Test
public void setTextInputEditingState_doesNotInvokeUpdateEditingState() {
// Initialize a general TextInputPlugin.
InputMethodSubtype inputMethodSubtype = mock(InputMethodSubtype.class);
TestImm testImm =
Shadow.extract(
RuntimeEnvironment.application.getSystemService(Context.INPUT_METHOD_SERVICE));
testImm.setCurrentInputMethodSubtype(inputMethodSubtype);
View testView = new View(RuntimeEnvironment.application);
TextInputChannel textInputChannel = spy(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));

textInputPlugin.setTextInputEditingState(
testView, new TextInputChannel.TextEditState("initial input from framework", 0, 0));
assertTrue(textInputPlugin.getEditable().toString().equals("initial input from framework"));

verify(textInputChannel, times(0))
.updateEditingState(anyInt(), any(), anyInt(), anyInt(), anyInt(), anyInt());

textInputPlugin.setTextInputEditingState(
testView, new TextInputChannel.TextEditState("more update from the framwork", 1, 1));

assertTrue(textInputPlugin.getEditable().toString().equals("more update from the framwork"));
verify(textInputChannel, times(0))
.updateEditingState(anyInt(), any(), anyInt(), anyInt(), anyInt(), anyInt());
}

@Test
public void setTextInputEditingState_doesNotRestartWhenTextIsIdentical() {
// Initialize a general TextInputPlugin.
Expand Down