Commit 94e195a2 authored by aelias's avatar aelias Committed by Commit bot

Translate physical keyboard accents to IME compositions.

On other platforms, the OS IME system generates composition events for
combining characters like accents, but on Android it's the
responsibility of the textbox implementation to do so for physical
keyboards.  This patch adds support for that.

This differs from the previous accent implementation
https://codereview.chromium.org/759033002 in that it uses IME
compositions.  This is more similar to desktop platforms from the point
of view of Blink/Javascript (I've verified compositionUpdate events are
sent on Mac instead of keycodes), and it avoids complications arising from
artificial backspace characters.

To simplify the if/else blocks, this patch also changes to update
mEditable on ACTION_DOWN instead of up, which also better maps to when
Blink makes the change.

BUG=230921

Review URL: https://codereview.chromium.org/1162863007

Cr-Commit-Position: refs/heads/master@{#333150}
parent 100d9480
...@@ -10,6 +10,7 @@ import android.text.InputType; ...@@ -10,6 +10,7 @@ import android.text.InputType;
import android.text.Selection; import android.text.Selection;
import android.text.TextUtils; import android.text.TextUtils;
import android.util.Log; import android.util.Log;
import android.view.KeyCharacterMap;
import android.view.KeyEvent; import android.view.KeyEvent;
import android.view.View; import android.view.View;
import android.view.inputmethod.BaseInputConnection; import android.view.inputmethod.BaseInputConnection;
...@@ -42,6 +43,7 @@ public class AdapterInputConnection extends BaseInputConnection { ...@@ -42,6 +43,7 @@ public class AdapterInputConnection extends BaseInputConnection {
private boolean mSingleLine; private boolean mSingleLine;
private int mNumNestedBatchEdits = 0; private int mNumNestedBatchEdits = 0;
private int mPendingAccent;
private int mLastUpdateSelectionStart = INVALID_SELECTION; private int mLastUpdateSelectionStart = INVALID_SELECTION;
private int mLastUpdateSelectionEnd = INVALID_SELECTION; private int mLastUpdateSelectionEnd = INVALID_SELECTION;
...@@ -242,6 +244,7 @@ public class AdapterInputConnection extends BaseInputConnection { ...@@ -242,6 +244,7 @@ public class AdapterInputConnection extends BaseInputConnection {
public boolean setComposingText(CharSequence text, int newCursorPosition) { public boolean setComposingText(CharSequence text, int newCursorPosition) {
if (DEBUG) Log.w(TAG, "setComposingText [" + text + "] [" + newCursorPosition + "]"); if (DEBUG) Log.w(TAG, "setComposingText [" + text + "] [" + newCursorPosition + "]");
if (maybePerformEmptyCompositionWorkaround(text)) return true; if (maybePerformEmptyCompositionWorkaround(text)) return true;
mPendingAccent = 0;
super.setComposingText(text, newCursorPosition); super.setComposingText(text, newCursorPosition);
updateSelectionIfRequired(); updateSelectionIfRequired();
return mImeAdapter.checkCompositionQueueAndCallNative(text, newCursorPosition, false); return mImeAdapter.checkCompositionQueueAndCallNative(text, newCursorPosition, false);
...@@ -254,6 +257,7 @@ public class AdapterInputConnection extends BaseInputConnection { ...@@ -254,6 +257,7 @@ public class AdapterInputConnection extends BaseInputConnection {
public boolean commitText(CharSequence text, int newCursorPosition) { public boolean commitText(CharSequence text, int newCursorPosition) {
if (DEBUG) Log.w(TAG, "commitText [" + text + "] [" + newCursorPosition + "]"); if (DEBUG) Log.w(TAG, "commitText [" + text + "] [" + newCursorPosition + "]");
if (maybePerformEmptyCompositionWorkaround(text)) return true; if (maybePerformEmptyCompositionWorkaround(text)) return true;
mPendingAccent = 0;
super.commitText(text, newCursorPosition); super.commitText(text, newCursorPosition);
updateSelectionIfRequired(); updateSelectionIfRequired();
return mImeAdapter.checkCompositionQueueAndCallNative(text, newCursorPosition, return mImeAdapter.checkCompositionQueueAndCallNative(text, newCursorPosition,
...@@ -351,6 +355,11 @@ public class AdapterInputConnection extends BaseInputConnection { ...@@ -351,6 +355,11 @@ public class AdapterInputConnection extends BaseInputConnection {
if (DEBUG) { if (DEBUG) {
Log.w(TAG, "deleteSurroundingText [" + beforeLength + " " + afterLength + "]"); Log.w(TAG, "deleteSurroundingText [" + beforeLength + " " + afterLength + "]");
} }
if (mPendingAccent != 0) {
finishComposingText();
}
int originalBeforeLength = beforeLength; int originalBeforeLength = beforeLength;
int originalAfterLength = afterLength; int originalAfterLength = afterLength;
int availableBefore = Selection.getSelectionStart(mEditable); int availableBefore = Selection.getSelectionStart(mEditable);
...@@ -403,15 +412,22 @@ public class AdapterInputConnection extends BaseInputConnection { ...@@ -403,15 +412,22 @@ public class AdapterInputConnection extends BaseInputConnection {
int keycode = event.getKeyCode(); int keycode = event.getKeyCode();
int unicodeChar = event.getUnicodeChar(); int unicodeChar = event.getUnicodeChar();
// If this isn't a KeyDown event, no need to update composition state; just pass the key
// event through and return.
if (action != KeyEvent.ACTION_DOWN) {
mImeAdapter.translateAndSendNativeEvents(event);
return true;
}
// If this is backspace/del or if the key has a character representation, // If this is backspace/del or if the key has a character representation,
// need to update the underlying Editable (i.e. the local representation of the text // need to update the underlying Editable (i.e. the local representation of the text
// being edited). Some IMEs like Jellybean stock IME and Samsung IME mix in delete // being edited). Some IMEs like Jellybean stock IME and Samsung IME mix in delete
// KeyPress events instead of calling deleteSurroundingText. // KeyPress events instead of calling deleteSurroundingText.
if (action == KeyEvent.ACTION_DOWN && keycode == KeyEvent.KEYCODE_DEL) { if (keycode == KeyEvent.KEYCODE_DEL) {
deleteSurroundingTextImpl(1, 0, true); deleteSurroundingTextImpl(1, 0, true);
} else if (action == KeyEvent.ACTION_DOWN && keycode == KeyEvent.KEYCODE_FORWARD_DEL) { } else if (keycode == KeyEvent.KEYCODE_FORWARD_DEL) {
deleteSurroundingTextImpl(0, 1, true); deleteSurroundingTextImpl(0, 1, true);
} else if (action == KeyEvent.ACTION_DOWN && keycode == KeyEvent.KEYCODE_ENTER) { } else if (keycode == KeyEvent.KEYCODE_ENTER) {
// Finish text composition when pressing enter, as that may submit a form field. // Finish text composition when pressing enter, as that may submit a form field.
// TODO(aurimas): remove this workaround when crbug.com/278584 is fixed. // TODO(aurimas): remove this workaround when crbug.com/278584 is fixed.
beginBatchEdit(); beginBatchEdit();
...@@ -419,7 +435,32 @@ public class AdapterInputConnection extends BaseInputConnection { ...@@ -419,7 +435,32 @@ public class AdapterInputConnection extends BaseInputConnection {
mImeAdapter.translateAndSendNativeEvents(event); mImeAdapter.translateAndSendNativeEvents(event);
endBatchEdit(); endBatchEdit();
return true; return true;
} else if (action == KeyEvent.ACTION_UP && unicodeChar != 0) { } else if ((unicodeChar & KeyCharacterMap.COMBINING_ACCENT) != 0) {
// Store a pending accent character and make it the current composition.
int pendingAccent = unicodeChar & KeyCharacterMap.COMBINING_ACCENT_MASK;
StringBuilder builder = new StringBuilder();
builder.appendCodePoint(pendingAccent);
setComposingText(builder.toString(), 1);
mPendingAccent = pendingAccent;
return true;
}
if (unicodeChar != 0) {
if (mPendingAccent != 0) {
int combined = KeyEvent.getDeadChar(mPendingAccent, unicodeChar);
if (combined != 0) {
StringBuilder builder = new StringBuilder();
builder.appendCodePoint(combined);
commitText(builder.toString(), 1);
return true;
}
// Noncombinable character; commit the accent character and fall through to sending
// the key event for the character afterwards.
finishComposingText();
}
// Update the mEditable state to reflect what Blink will do in response to the KeyDown
// for a unicode-mapped key event.
int selectionStart = Selection.getSelectionStart(mEditable); int selectionStart = Selection.getSelectionStart(mEditable);
int selectionEnd = Selection.getSelectionEnd(mEditable); int selectionEnd = Selection.getSelectionEnd(mEditable);
if (selectionStart > selectionEnd) { if (selectionStart > selectionEnd) {
...@@ -430,6 +471,7 @@ public class AdapterInputConnection extends BaseInputConnection { ...@@ -430,6 +471,7 @@ public class AdapterInputConnection extends BaseInputConnection {
mEditable.replace(selectionStart, selectionEnd, mEditable.replace(selectionStart, selectionEnd,
Character.toString((char) unicodeChar)); Character.toString((char) unicodeChar));
} }
mImeAdapter.translateAndSendNativeEvents(event); mImeAdapter.translateAndSendNativeEvents(event);
return true; return true;
} }
...@@ -440,6 +482,9 @@ public class AdapterInputConnection extends BaseInputConnection { ...@@ -440,6 +482,9 @@ public class AdapterInputConnection extends BaseInputConnection {
@Override @Override
public boolean finishComposingText() { public boolean finishComposingText() {
if (DEBUG) Log.w(TAG, "finishComposingText"); if (DEBUG) Log.w(TAG, "finishComposingText");
mPendingAccent = 0;
if (getComposingSpanStart(mEditable) == getComposingSpanEnd(mEditable)) { if (getComposingSpanStart(mEditable) == getComposingSpanEnd(mEditable)) {
return true; return true;
} }
...@@ -472,6 +517,7 @@ public class AdapterInputConnection extends BaseInputConnection { ...@@ -472,6 +517,7 @@ public class AdapterInputConnection extends BaseInputConnection {
if (DEBUG) Log.w(TAG, "restartInput"); if (DEBUG) Log.w(TAG, "restartInput");
getInputMethodManagerWrapper().restartInput(mInternalView); getInputMethodManagerWrapper().restartInput(mInternalView);
mNumNestedBatchEdits = 0; mNumNestedBatchEdits = 0;
mPendingAccent = 0;
} }
/** /**
......
...@@ -700,6 +700,86 @@ public class ImeTest extends ContentShellTestBase { ...@@ -700,6 +700,86 @@ public class ImeTest extends ContentShellTestBase {
assertEquals("", mConnection.getTextBeforeCursor(9, 0)); assertEquals("", mConnection.getTextBeforeCursor(9, 0));
} }
@SmallTest
@Feature({"TextInput", "Main"})
public void testAccentKeyCodesFromPhysicalKeyboard() throws Throwable {
DOMUtils.focusNode(mWebContents, "textarea");
assertWaitForKeyboardStatus(true);
mConnection = (TestAdapterInputConnection) getAdapterInputConnection();
waitAndVerifyEditableCallback(mConnection.mImeUpdateQueue, 0, "", 0, 0, -1, -1);
// h
dispatchKeyEvent(mConnection, new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_H));
dispatchKeyEvent(mConnection, new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_H));
assertEquals("h", mConnection.getTextBeforeCursor(9, 0));
// ALT-i (circumflex accent key on virtual keyboard)
dispatchKeyEvent(
mConnection, new KeyEvent(
0, 0, KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_I, 0, KeyEvent.META_ALT_ON));
assertUpdateStateCall(mConnection, 1000);
assertEquals("hˆ", mConnection.getTextBeforeCursor(9, 0));
dispatchKeyEvent(
mConnection, new KeyEvent(
0, 0, KeyEvent.ACTION_UP, KeyEvent.KEYCODE_I, 0, KeyEvent.META_ALT_ON));
assertEquals("hˆ", mConnection.getTextBeforeCursor(9, 0));
// o
dispatchKeyEvent(mConnection, new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_O));
assertUpdateStateCall(mConnection, 1000);
assertEquals("hô", mConnection.getTextBeforeCursor(9, 0));
dispatchKeyEvent(mConnection, new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_O));
assertEquals("hô", mConnection.getTextBeforeCursor(9, 0));
assertEquals(-1, mConnection.getComposingSpanEnd(mConnection.getEditable()));
// ALT-i
dispatchKeyEvent(
mConnection, new KeyEvent(
0, 0, KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_I, 0, KeyEvent.META_ALT_ON));
assertUpdateStateCall(mConnection, 1000);
dispatchKeyEvent(
mConnection, new KeyEvent(
0, 0, KeyEvent.ACTION_UP, KeyEvent.KEYCODE_I, 0, KeyEvent.META_ALT_ON));
assertEquals("hôˆ", mConnection.getTextBeforeCursor(9, 0));
// ALT-i again should have no effect
dispatchKeyEvent(
mConnection, new KeyEvent(
0, 0, KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_I, 0, KeyEvent.META_ALT_ON));
assertUpdateStateCall(mConnection, 1000);
dispatchKeyEvent(
mConnection, new KeyEvent(
0, 0, KeyEvent.ACTION_UP, KeyEvent.KEYCODE_I, 0, KeyEvent.META_ALT_ON));
assertEquals("hôˆ", mConnection.getTextBeforeCursor(9, 0));
// b (cannot be accented, should just appear after)
dispatchKeyEvent(mConnection, new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_B));
assertUpdateStateCall(mConnection, 1000);
assertEquals("hôˆb", mConnection.getTextBeforeCursor(9, 0));
dispatchKeyEvent(mConnection, new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_B));
assertEquals("hôˆb", mConnection.getTextBeforeCursor(9, 0));
assertEquals(-1, mConnection.getComposingSpanEnd(mConnection.getEditable()));
// ALT-i
dispatchKeyEvent(
mConnection, new KeyEvent(
0, 0, KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_I, 0, KeyEvent.META_ALT_ON));
assertUpdateStateCall(mConnection, 1000);
dispatchKeyEvent(
mConnection, new KeyEvent(
0, 0, KeyEvent.ACTION_UP, KeyEvent.KEYCODE_I, 0, KeyEvent.META_ALT_ON));
assertEquals("hôˆbˆ", mConnection.getTextBeforeCursor(9, 0));
// Backspace
dispatchKeyEvent(mConnection, new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_DEL));
assertUpdateStateCall(mConnection, 1000);
assertEquals("hôˆb", mConnection.getTextBeforeCursor(9, 0));
dispatchKeyEvent(mConnection, new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_DEL));
assertEquals("hôˆb", mConnection.getTextBeforeCursor(9, 0));
assertEquals(-1, mConnection.getComposingSpanEnd(mConnection.getEditable()));
}
@SmallTest @SmallTest
@Feature({"TextInput", "Main"}) @Feature({"TextInput", "Main"})
public void testSetComposingRegionOutOfBounds() throws Throwable { public void testSetComposingRegionOutOfBounds() throws Throwable {
...@@ -1056,6 +1136,15 @@ public class ImeTest extends ContentShellTestBase { ...@@ -1056,6 +1136,15 @@ public class ImeTest extends ContentShellTestBase {
}); });
} }
private void dispatchKeyEvent(final AdapterInputConnection connection, final KeyEvent event) {
ThreadUtils.runOnUiThreadBlocking(new Runnable() {
@Override
public void run() {
mImeAdapter.dispatchKeyEvent(event);
}
});
}
private static class TestAdapterInputConnectionFactory extends private static class TestAdapterInputConnectionFactory extends
ImeAdapter.AdapterInputConnectionFactory { ImeAdapter.AdapterInputConnectionFactory {
@Override @Override
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment