Commit 5f6e0363 authored by changwan's avatar changwan Committed by Commit bot

Make sure multi chracter codepoints are deleted correctly

deleteSurroundingText() only deletes one character even for multi-character
codepoint.

On the blink side, we have InputMethodController::extendSelectionAndDelete()
to make sure selection and deletion respect Unicode boundaries. However,
AdapterInputConnection keeps track of selection region separately, and this
value is incorrectly updated.

On top of adding a new test for this case, it extends
waitAndVerifyEditorCallback to also check the outbound calls to
InputMethodManager.

The above extension found that testEnterKeyEventWhileComposingText fails
because there is hidden discrepancy between blink implementation and
what we report to InputMethodManager. So I've added a TODO for that.

BUG=497091

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

Cr-Commit-Position: refs/heads/master@{#333704}
parent 45c0c979
...@@ -9,7 +9,6 @@ import android.text.Editable; ...@@ -9,7 +9,6 @@ import android.text.Editable;
import android.text.InputType; 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.view.KeyCharacterMap; import android.view.KeyCharacterMap;
import android.view.KeyEvent; import android.view.KeyEvent;
import android.view.View; import android.view.View;
...@@ -18,6 +17,7 @@ import android.view.inputmethod.EditorInfo; ...@@ -18,6 +17,7 @@ import android.view.inputmethod.EditorInfo;
import android.view.inputmethod.ExtractedText; import android.view.inputmethod.ExtractedText;
import android.view.inputmethod.ExtractedTextRequest; import android.view.inputmethod.ExtractedTextRequest;
import org.chromium.base.Log;
import org.chromium.base.VisibleForTesting; import org.chromium.base.VisibleForTesting;
import org.chromium.blink_public.web.WebInputEventType; import org.chromium.blink_public.web.WebInputEventType;
import org.chromium.blink_public.web.WebTextInputFlags; import org.chromium.blink_public.web.WebTextInputFlags;
...@@ -229,8 +229,8 @@ public class AdapterInputConnection extends BaseInputConnection { ...@@ -229,8 +229,8 @@ public class AdapterInputConnection extends BaseInputConnection {
} }
// updateSelection should be called every time the selection or composition changes // updateSelection should be called every time the selection or composition changes
// if it happens not within a batch edit, or at the end of each top level batch edit. // if it happens not within a batch edit, or at the end of each top level batch edit.
getInputMethodManagerWrapper().updateSelection(mInternalView, getInputMethodManagerWrapper().updateSelection(
selectionStart, selectionEnd, compositionStart, compositionEnd); mInternalView, selectionStart, selectionEnd, compositionStart, compositionEnd);
mLastUpdateSelectionStart = selectionStart; mLastUpdateSelectionStart = selectionStart;
mLastUpdateSelectionEnd = selectionEnd; mLastUpdateSelectionEnd = selectionEnd;
mLastUpdateCompositionStart = compositionStart; mLastUpdateCompositionStart = compositionStart;
...@@ -350,10 +350,23 @@ public class AdapterInputConnection extends BaseInputConnection { ...@@ -350,10 +350,23 @@ public class AdapterInputConnection extends BaseInputConnection {
return deleteSurroundingTextImpl(beforeLength, afterLength, false); return deleteSurroundingTextImpl(beforeLength, afterLength, false);
} }
/**
* Check if the given {@code index} is between UTF-16 surrogate pair.
* @param str The String.
* @param index The index
* @return True if the index is between UTF-16 surrogate pair, false otherwise.
*/
@VisibleForTesting
static boolean isIndexBetweenUtf16SurrogatePair(CharSequence str, int index) {
return index > 0 && index < str.length() && Character.isHighSurrogate(str.charAt(index - 1))
&& Character.isLowSurrogate(str.charAt(index));
}
private boolean deleteSurroundingTextImpl( private boolean deleteSurroundingTextImpl(
int beforeLength, int afterLength, boolean fromPhysicalKey) { int beforeLength, int afterLength, boolean fromPhysicalKey) {
if (DEBUG) { if (DEBUG) {
Log.w(TAG, "deleteSurroundingText [" + beforeLength + " " + afterLength + "]"); Log.w(TAG, "deleteSurroundingText [" + beforeLength + " " + afterLength + " "
+ fromPhysicalKey + "]");
} }
if (mPendingAccent != 0) { if (mPendingAccent != 0) {
...@@ -362,10 +375,22 @@ public class AdapterInputConnection extends BaseInputConnection { ...@@ -362,10 +375,22 @@ public class AdapterInputConnection extends BaseInputConnection {
int originalBeforeLength = beforeLength; int originalBeforeLength = beforeLength;
int originalAfterLength = afterLength; int originalAfterLength = afterLength;
int availableBefore = Selection.getSelectionStart(mEditable); int selectionStart = Selection.getSelectionStart(mEditable);
int availableAfter = mEditable.length() - Selection.getSelectionEnd(mEditable); int selectionEnd = Selection.getSelectionEnd(mEditable);
int availableBefore = selectionStart;
int availableAfter = mEditable.length() - selectionEnd;
beforeLength = Math.min(beforeLength, availableBefore); beforeLength = Math.min(beforeLength, availableBefore);
afterLength = Math.min(afterLength, availableAfter); afterLength = Math.min(afterLength, availableAfter);
// Adjust these values even before calling super.deleteSurroundingText() to be consistent
// with the super class.
if (isIndexBetweenUtf16SurrogatePair(mEditable, selectionStart - beforeLength)) {
beforeLength += 1;
}
if (isIndexBetweenUtf16SurrogatePair(mEditable, selectionEnd + afterLength)) {
afterLength += 1;
}
super.deleteSurroundingText(beforeLength, afterLength); super.deleteSurroundingText(beforeLength, afterLength);
updateSelectionIfRequired(); updateSelectionIfRequired();
......
...@@ -8,6 +8,7 @@ import android.content.Context; ...@@ -8,6 +8,7 @@ import android.content.Context;
import android.os.IBinder; import android.os.IBinder;
import android.os.ResultReceiver; import android.os.ResultReceiver;
import android.test.suitebuilder.annotation.MediumTest; import android.test.suitebuilder.annotation.MediumTest;
import android.test.suitebuilder.annotation.SmallTest;
import android.text.Editable; import android.text.Editable;
import android.text.Selection; import android.text.Selection;
import android.view.KeyEvent; import android.view.KeyEvent;
...@@ -45,6 +46,23 @@ public class AdapterInputConnectionTest extends ContentShellTestBase { ...@@ -45,6 +46,23 @@ public class AdapterInputConnectionTest extends ContentShellTestBase {
getContentViewCore().getContainerView(), mImeAdapter, mEditable, info); getContentViewCore().getContainerView(), mImeAdapter, mEditable, info);
} }
@SmallTest
@Feature({"TextInput", "Main"})
public void testAdjustLengthBeforeAndAfterSelection() throws Throwable {
final String ga = "\uAC00";
final String smiley = "\uD83D\uDE0A"; // multi character codepoint
// No need to adjust length.
assertFalse(AdapterInputConnection.isIndexBetweenUtf16SurrogatePair("a", 0));
assertFalse(AdapterInputConnection.isIndexBetweenUtf16SurrogatePair(ga, 0));
assertFalse(AdapterInputConnection.isIndexBetweenUtf16SurrogatePair("aa", 1));
assertFalse(AdapterInputConnection.isIndexBetweenUtf16SurrogatePair("a" + smiley + "a", 1));
// Needs to adjust length.
assertTrue(AdapterInputConnection.isIndexBetweenUtf16SurrogatePair(smiley, 1));
assertTrue(AdapterInputConnection.isIndexBetweenUtf16SurrogatePair(smiley + "a", 1));
}
@MediumTest @MediumTest
@Feature({"TextInput", "Main"}) @Feature({"TextInput", "Main"})
@RerunWithUpdatedContainerView @RerunWithUpdatedContainerView
......
...@@ -13,12 +13,62 @@ import android.view.inputmethod.InputConnection; ...@@ -13,12 +13,62 @@ import android.view.inputmethod.InputConnection;
import org.chromium.content.browser.ContentViewCore; import org.chromium.content.browser.ContentViewCore;
import org.chromium.content.browser.input.InputMethodManagerWrapper; import org.chromium.content.browser.input.InputMethodManagerWrapper;
/**
* Overrides InputMethodManagerWrapper for testing purposes.
*/
public class TestInputMethodManagerWrapper extends InputMethodManagerWrapper { public class TestInputMethodManagerWrapper extends InputMethodManagerWrapper {
/**
* A simple class to set start and end in int type.
*/
public static class Range {
private int mStart;
private int mEnd;
public Range(int start, int end) {
mStart = start;
mEnd = end;
}
public void set(int start, int end) {
mStart = start;
mEnd = end;
}
public int start() {
return mStart;
}
public int end() {
return mEnd;
}
@Override
public boolean equals(Object o) {
if (!(o instanceof Range)) return false;
if (o == this) return true;
Range r = (Range) o;
return mStart == r.mStart && mEnd == r.mEnd;
}
@Override
public int hashCode() {
final int prime = 31;
return prime * mStart + mEnd;
}
@Override
public String toString() {
return "[ " + mStart + ", " + mEnd + " ]";
}
}
private final ContentViewCore mContentViewCore; private final ContentViewCore mContentViewCore;
private InputConnection mInputConnection; private InputConnection mInputConnection;
private int mShowSoftInputCounter = 0; private int mShowSoftInputCounter = 0;
private int mUpdateSelectionCounter = 0; private int mUpdateSelectionCounter = 0;
private EditorInfo mEditorInfo; private EditorInfo mEditorInfo;
private final Range mSelection = new Range(0, 0);
private final Range mComposition = new Range(-1, -1);
public TestInputMethodManagerWrapper(ContentViewCore contentViewCore) { public TestInputMethodManagerWrapper(ContentViewCore contentViewCore) {
super(null); super(null);
...@@ -57,6 +107,8 @@ public class TestInputMethodManagerWrapper extends InputMethodManagerWrapper { ...@@ -57,6 +107,8 @@ public class TestInputMethodManagerWrapper extends InputMethodManagerWrapper {
public void updateSelection(View view, int selStart, int selEnd, public void updateSelection(View view, int selStart, int selEnd,
int candidatesStart, int candidatesEnd) { int candidatesStart, int candidatesEnd) {
mUpdateSelectionCounter++; mUpdateSelectionCounter++;
mSelection.set(selStart, selEnd);
mComposition.set(candidatesStart, candidatesEnd);
} }
public int getShowSoftInputCounter() { public int getShowSoftInputCounter() {
...@@ -70,5 +122,12 @@ public class TestInputMethodManagerWrapper extends InputMethodManagerWrapper { ...@@ -70,5 +122,12 @@ public class TestInputMethodManagerWrapper extends InputMethodManagerWrapper {
public EditorInfo getEditorInfo() { public EditorInfo getEditorInfo() {
return mEditorInfo; return mEditorInfo;
} }
}
public Range getSelection() {
return mSelection;
}
public Range getComposition() {
return mComposition;
}
}
\ No newline at end of file
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