Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Closed
Show file tree
Hide file tree
Changes from all 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 @@ -287,6 +287,8 @@ public boolean handleKeyEvent(KeyEvent event) {
return handleVerticalMovement(false, event.isShiftPressed());
// When the enter key is pressed on a non-multiline field, consider it a
// submit instead of a newline.
} else if (event.getKeyCode() == KeyEvent.KEYCODE_DEL) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the delete key not working? I think it's currently handled by the framework.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes the delete key not working in this case flutter/flutter#98352, you can use the simple code to verify it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use this demo, the framework doesn't handle the case of use input delete event. And in the engine doesn't handle it also.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is meant to be handled by the framework code here: https://github.com/flutter/flutter/blob/ffe64c6336bfb0ad68436af904e6e1f28724dd5f/packages/flutter/lib/src/widgets/default_text_editing_shortcuts.dart#L165-L174

Could you elaborate a bit on why the framework isn't handling it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we use TextFiled widget that will make overridable in here https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/widgets/editable_text.dart#L3120 and when we input Delete that will handle in method handleKaypress: https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/widgets/shortcuts.dart#L717, and the action https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/widgets/shortcuts.dart#L723 will be a instance of _OverridableContextAction<Intent>

But in this case, customer doesn't use TextFiled, only use a TextInputConfigure, when you input delete, the handleKaypress action will be null.

Copy link
Contributor

@LongCatIsLooong LongCatIsLooong Mar 14, 2022

Choose a reason for hiding this comment

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

Ah I see what you're saying. The sample code provided doesn't make use of EditableText and implements TextInputClient instead. However this is not recommended as most text editing key bindings should be handled by the framework (text layout isn't available to the engine so text input plugin can't reliably handle deleting to the beginning of the line reliably, for example), and I'm hoping that we can remove/deprecate this handleKeyEvent method.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey, do you mean that the problem is caused by the way the example is used?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, ideally the delete action needs to be handled by dart code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If delete the handleKey of Delete the user can't delete character. And do you have some suggestion to hold the delete character behavior?

return handleDelete();
} else if ((event.getKeyCode() == KeyEvent.KEYCODE_ENTER
|| event.getKeyCode() == KeyEvent.KEYCODE_NUMPAD_ENTER)
&& (InputType.TYPE_TEXT_FLAG_MULTI_LINE & mEditorInfo.inputType) == 0) {
Expand All @@ -297,12 +299,12 @@ public boolean handleKeyEvent(KeyEvent event) {
final int selStart = Selection.getSelectionStart(mEditable);
final int selEnd = Selection.getSelectionEnd(mEditable);
final int character = event.getUnicodeChar();
if (selStart < 0 || selEnd < 0 || character == 0) {
Copy link

Choose a reason for hiding this comment

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

is this case valid now, or just not possible because of the KEYCODE_DEL case above?

if (character == 0) {
return false;
}

final int selMin = Math.min(selStart, selEnd);
final int selMax = Math.max(selStart, selEnd);
final int selMin = Math.max(Math.min(selStart, selEnd), 0);
Copy link

Choose a reason for hiding this comment

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

related to the question above, when is selStart or selEnd < 0 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this issue case, the user use an TextInputConfiguration to connect the keyboard, but not use TextFiled, when the keyboard is number it will go to the handleKeyEvent method and cause is input a number then in to the else branch in this case the selStart and selEnd will < 0 that cause the bug

final int selMax = Math.max(Math.max(selStart, selEnd), 0);
beginBatchEdit();
if (selMin != selMax) mEditable.delete(selMin, selMax);
Copy link

Choose a reason for hiding this comment

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

is this case still relevant?

mEditable.insert(selMin, String.valueOf((char) character));
Expand Down Expand Up @@ -375,6 +377,20 @@ private boolean handleVerticalMovement(boolean isUp, boolean isShiftPressed) {
return true;
}

private boolean handleDelete() {
final int selStart = Selection.getSelectionStart(mEditable);
final int selEnd = Selection.getSelectionEnd(mEditable);

if (selStart <= 0 || selEnd <= 0) {
return false;
}
beginBatchEdit();
mEditable.delete(selStart - 1, selEnd);
setSelection(selStart - 1, selEnd - 1);
endBatchEdit();
return true;
}

@Override
public boolean performContextMenuAction(int id) {
beginBatchEdit();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@
import io.flutter.plugin.common.MethodCall;
import io.flutter.util.FakeKeyEvent;
import java.nio.ByteBuffer;
import java.util.ArrayList;
import java.util.List;
import org.json.JSONArray;
import org.json.JSONException;
import org.junit.Before;
Expand Down Expand Up @@ -1104,16 +1106,25 @@ public void testSendKeyEvent_sendHardwareKeyEvents() {

@Test
public void testSendKeyEvent_delKeyNotConsumed() {
ListenableEditingState editable = sampleEditable(5, 5);
ListenableEditingState editable = sampleEditable(5, 5, "01234");
InputConnectionAdaptor adaptor = sampleInputConnectionAdaptor(editable);

KeyEvent downKeyDown = new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_DEL);
KeyEvent downKeyUP = new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_DEL);
Copy link

Choose a reason for hiding this comment

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

This test was added by @gspencergoog in #22626.
Is this change correct, Greg?


for (int i = 0; i < 4; i++) {
boolean didConsume = adaptor.handleKeyEvent(downKeyDown);
boolean didConsume = adaptor.handleKeyEvent(downKeyUP);
assertFalse(didConsume);
}
assertEquals(5, Selection.getSelectionStart(editable));

// delete character
KeyEvent downKeyDown = new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_DEL);
for (int i = 0; i < 5; i++) {
boolean didConsume = adaptor.handleKeyEvent(downKeyDown);
assertTrue(didConsume);
}
assertEquals(0, Selection.getSelectionStart(editable));
assertEquals("", editable.toString());
}

@Test
Expand All @@ -1127,6 +1138,22 @@ public void testDoesNotConsumeBackButton() {
assertFalse(didConsume);
}

@Test
public void testInputKeycode() {
ListenableEditingState editable = sampleEditable(0, 0, "");
InputConnectionAdaptor adaptor = sampleInputConnectionAdaptor(editable);

List<FakeKeyEvent> keyEvents = new ArrayList<>();
keyEvents.add(new FakeKeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_1));
keyEvents.add(new FakeKeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_2));
keyEvents.add(new FakeKeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_3));

for (FakeKeyEvent keyEvent : keyEvents) {
adaptor.handleKeyEvent(keyEvent);
}
assertEquals("123", editable.toString());
}

@Test
public void testCleanUpBatchEndsOnCloseConnection() {
final ListenableEditingState editable = sampleEditable(0, 0);
Expand Down
21 changes: 20 additions & 1 deletion shell/platform/android/test/io/flutter/util/FakeKeyEvent.java
Original file line number Diff line number Diff line change
@@ -1,18 +1,37 @@
package io.flutter.util;

import android.view.KeyEvent;
import java.util.HashMap;
import java.util.Map;

// In the test environment, keyEvent.getUnicodeChar throws an exception. This
// class works around the exception by hardcoding the returned value.
public class FakeKeyEvent extends KeyEvent {

private Map<Integer, Integer> keyCodeMap = new HashMap<>();

public FakeKeyEvent(int action, int keyCode) {
super(action, keyCode);
keyCodeMap.put(KEYCODE_1, (int) '1');
keyCodeMap.put(KEYCODE_2, (int) '2');
keyCodeMap.put(KEYCODE_3, (int) '3');
keyCodeMap.put(KEYCODE_4, (int) '4');
keyCodeMap.put(KEYCODE_5, (int) '5');
keyCodeMap.put(KEYCODE_6, (int) '6');
keyCodeMap.put(KEYCODE_7, (int) '7');
keyCodeMap.put(KEYCODE_8, (int) '8');
keyCodeMap.put(KEYCODE_9, (int) '9');
keyCodeMap.put(KEYCODE_0, (int) '0');
}

public final int getUnicodeChar() {
if (getKeyCode() == KeyEvent.KEYCODE_BACK) {
int code = getKeyCode();
if (code == KeyEvent.KEYCODE_BACK) {
return 0;
}
if (keyCodeMap.containsKey(code)) {
return keyCodeMap.get(code);
}
return 1;
}
}