Commit bac56ba3 authored by dtrainor's avatar dtrainor Committed by Commit bot

Revert of Make sure multi chracter codepoints are deleted correctly (patchset...

Revert of Make sure multi chracter codepoints are deleted correctly (patchset #6 id:100001 of https://codereview.chromium.org/1165793007/)

Reason for revert:
Failing Android Tests (dbg) builder: http://build.chromium.org/p/chromium.linux/builders/Android%20Tests%20%28dbg%29/builds/28499

The newly added test seems to be flaky.

Original issue's description:
> 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
>
> Committed: https://crrev.com/5f6e036312bc4a978768f5b5971eee1a5ec9f272
> Cr-Commit-Position: refs/heads/master@{#333704}

TBR=aelias@chromium.org,aurimas@chromium.org,bcwhite@chromium.org,tedchoc@chromium.org,changwan@chromium.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=497091

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

Cr-Commit-Position: refs/heads/master@{#333765}
parent 43ac2661
...@@ -9,6 +9,7 @@ import android.text.Editable; ...@@ -9,6 +9,7 @@ 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;
...@@ -17,7 +18,6 @@ import android.view.inputmethod.EditorInfo; ...@@ -17,7 +18,6 @@ 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( getInputMethodManagerWrapper().updateSelection(mInternalView,
mInternalView, selectionStart, selectionEnd, compositionStart, compositionEnd); selectionStart, selectionEnd, compositionStart, compositionEnd);
mLastUpdateSelectionStart = selectionStart; mLastUpdateSelectionStart = selectionStart;
mLastUpdateSelectionEnd = selectionEnd; mLastUpdateSelectionEnd = selectionEnd;
mLastUpdateCompositionStart = compositionStart; mLastUpdateCompositionStart = compositionStart;
...@@ -350,23 +350,10 @@ public class AdapterInputConnection extends BaseInputConnection { ...@@ -350,23 +350,10 @@ 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) {
...@@ -375,22 +362,10 @@ public class AdapterInputConnection extends BaseInputConnection { ...@@ -375,22 +362,10 @@ public class AdapterInputConnection extends BaseInputConnection {
int originalBeforeLength = beforeLength; int originalBeforeLength = beforeLength;
int originalAfterLength = afterLength; int originalAfterLength = afterLength;
int selectionStart = Selection.getSelectionStart(mEditable); int availableBefore = Selection.getSelectionStart(mEditable);
int selectionEnd = Selection.getSelectionEnd(mEditable); int availableAfter = mEditable.length() - 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,7 +8,6 @@ import android.content.Context; ...@@ -8,7 +8,6 @@ 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;
...@@ -46,23 +45,6 @@ public class AdapterInputConnectionTest extends ContentShellTestBase { ...@@ -46,23 +45,6 @@ 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,62 +13,12 @@ import android.view.inputmethod.InputConnection; ...@@ -13,62 +13,12 @@ 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);
...@@ -107,8 +57,6 @@ public class TestInputMethodManagerWrapper extends InputMethodManagerWrapper { ...@@ -107,8 +57,6 @@ 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() {
...@@ -122,12 +70,5 @@ public class TestInputMethodManagerWrapper extends InputMethodManagerWrapper { ...@@ -122,12 +70,5 @@ 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