Commit fe62ee1b authored by changwan's avatar changwan Committed by Commit bot

Return null when selected text is empty

InputConnection#getSelectedText() should return null when the selection
is empty. ThreadedInputConnection was not doing this correctly, and confused
some OEM keyboards.

BUG=638201

Review-Url: https://codereview.chromium.org/2279293002
Cr-Commit-Position: refs/heads/master@{#414821}
parent a4fd5537
...@@ -53,6 +53,7 @@ public class TextInputState { ...@@ -53,6 +53,7 @@ public class TextInputState {
} }
public CharSequence getSelectedText() { public CharSequence getSelectedText() {
if (mSelection.start() == mSelection.end()) return null;
return TextUtils.substring(mText, mSelection.start(), mSelection.end()); return TextUtils.substring(mText, mSelection.start(), mSelection.end());
} }
......
...@@ -136,7 +136,7 @@ public class ImeTest extends ContentShellTestBase { ...@@ -136,7 +136,7 @@ public class ImeTest extends ContentShellTestBase {
setSelection(2, 2); setSelection(2, 2);
waitAndVerifyUpdateSelection(2, 2, 2, -1, -1); waitAndVerifyUpdateSelection(2, 2, 2, -1, -1);
deleteSurroundingText(0, 0); deleteSurroundingText(0, 0);
assertTextsAroundCursor("he", "", "llo"); assertTextsAroundCursor("he", null, "llo");
} }
// When newCursorPosition != 1, setComposingText doesn't work for ReplicaInputConnection // When newCursorPosition != 1, setComposingText doesn't work for ReplicaInputConnection
...@@ -200,12 +200,12 @@ public class ImeTest extends ContentShellTestBase { ...@@ -200,12 +200,12 @@ public class ImeTest extends ContentShellTestBase {
// With previous composition. // With previous composition.
setComposingText("", -3); setComposingText("", -3);
waitAndVerifyUpdateSelection(2, 2, 2, -1, -1); waitAndVerifyUpdateSelection(2, 2, 2, -1, -1);
assertTextsAroundCursor("he", "", "llo"); assertTextsAroundCursor("he", null, "llo");
// Without previous composition. // Without previous composition.
setComposingText("", 3); setComposingText("", 3);
waitAndVerifyUpdateSelection(3, 4, 4, -1, -1); waitAndVerifyUpdateSelection(3, 4, 4, -1, -1);
assertTextsAroundCursor("hell", "", "o"); assertTextsAroundCursor("hell", null, "o");
} }
@SmallTest @SmallTest
...@@ -229,7 +229,7 @@ public class ImeTest extends ContentShellTestBase { ...@@ -229,7 +229,7 @@ public class ImeTest extends ContentShellTestBase {
commitText("", 1); commitText("", 1);
waitAndVerifyUpdateSelection(5, 3, 3, -1, -1); waitAndVerifyUpdateSelection(5, 3, 3, -1, -1);
assertTextsAroundCursor("hel", "", ""); assertTextsAroundCursor("hel", null, "");
} }
@SmallTest @SmallTest
...@@ -245,11 +245,11 @@ public class ImeTest extends ContentShellTestBase { ...@@ -245,11 +245,11 @@ public class ImeTest extends ContentShellTestBase {
waitAndVerifyUpdateSelection(1, 1, 1, -1, -1); waitAndVerifyUpdateSelection(1, 1, 1, -1, -1);
// The second new line is not a user visible/editable one, it is a side-effect of Blink // The second new line is not a user visible/editable one, it is a side-effect of Blink
// using <br> internally. This only happens when \n is at the end. // using <br> internally. This only happens when \n is at the end.
assertTextsAroundCursor("\n", "", "\n"); assertTextsAroundCursor("\n", null, "\n");
commitText("world", 1); commitText("world", 1);
waitAndVerifyUpdateSelection(2, 6, 6, -1, -1); waitAndVerifyUpdateSelection(2, 6, 6, -1, -1);
assertTextsAroundCursor("\nworld", "", ""); assertTextsAroundCursor("\nworld", null, "");
} }
@SmallTest @SmallTest
...@@ -329,10 +329,6 @@ public class ImeTest extends ContentShellTestBase { ...@@ -329,10 +329,6 @@ public class ImeTest extends ContentShellTestBase {
assertEquals(before, getTextBeforeCursor(100, 0)); assertEquals(before, getTextBeforeCursor(100, 0));
CharSequence actualSelected = getSelectedText(0); CharSequence actualSelected = getSelectedText(0);
if (usingReplicaInputConnection() && TextUtils.isEmpty(actualSelected)) {
// ReplicaInputConnection will return null but ChromiumInputConnection will return "".
actualSelected = "";
}
assertEquals(selected, actualSelected); assertEquals(selected, actualSelected);
if (usingReplicaInputConnection() && after.equals("\n")) { if (usingReplicaInputConnection() && after.equals("\n")) {
...@@ -603,7 +599,7 @@ public class ImeTest extends ContentShellTestBase { ...@@ -603,7 +599,7 @@ public class ImeTest extends ContentShellTestBase {
cut(); cut();
waitAndVerifyUpdateSelection(2, 1, 1, -1, -1); waitAndVerifyUpdateSelection(2, 1, 1, -1, -1);
assertTextsAroundCursor("s", "", "ul"); assertTextsAroundCursor("s", null, "ul");
assertClipboardContents(getActivity(), "narf"); assertClipboardContents(getActivity(), "narf");
} }
...@@ -623,7 +619,7 @@ public class ImeTest extends ContentShellTestBase { ...@@ -623,7 +619,7 @@ public class ImeTest extends ContentShellTestBase {
paste(); paste();
// Paste is a two step process when there is a non-zero selection. // Paste is a two step process when there is a non-zero selection.
waitAndVerifyUpdateSelection(0, 5, 5, -1, -1); waitAndVerifyUpdateSelection(0, 5, 5, -1, -1);
assertTextsAroundCursor("blarg", "", ""); assertTextsAroundCursor("blarg", null, "");
setSelection(3, 5); setSelection(3, 5);
waitAndVerifyUpdateSelection(1, 3, 5, -1, -1); waitAndVerifyUpdateSelection(1, 3, 5, -1, -1);
...@@ -633,11 +629,11 @@ public class ImeTest extends ContentShellTestBase { ...@@ -633,11 +629,11 @@ public class ImeTest extends ContentShellTestBase {
// Paste is a two step process when there is a non-zero selection. // Paste is a two step process when there is a non-zero selection.
waitAndVerifyUpdateSelection(2, 3, 3, -1, -1); waitAndVerifyUpdateSelection(2, 3, 3, -1, -1);
waitAndVerifyUpdateSelection(3, 8, 8, -1, -1); waitAndVerifyUpdateSelection(3, 8, 8, -1, -1);
assertTextsAroundCursor("blablarg", "", ""); assertTextsAroundCursor("blablarg", null, "");
paste(); paste();
waitAndVerifyUpdateSelection(4, 13, 13, -1, -1); waitAndVerifyUpdateSelection(4, 13, 13, -1, -1);
assertTextsAroundCursor("blablargblarg", "", ""); assertTextsAroundCursor("blablargblarg", null, "");
} }
// Chrome can crash after pasting long text into textarea, becasue there is an overflow bug in // Chrome can crash after pasting long text into textarea, becasue there is an overflow bug in
...@@ -713,7 +709,7 @@ public class ImeTest extends ContentShellTestBase { ...@@ -713,7 +709,7 @@ public class ImeTest extends ContentShellTestBase {
setSelection(1, 1); setSelection(1, 1);
waitAndVerifyUpdateSelection(2, 1, 1, -1, -1); waitAndVerifyUpdateSelection(2, 1, 1, -1, -1);
assertTextsAroundCursor("h", "", "llo "); assertTextsAroundCursor("h", null, "llo ");
setComposingRegion(0, 4); setComposingRegion(0, 4);
waitAndVerifyUpdateSelection(3, 1, 1, 0, 4); waitAndVerifyUpdateSelection(3, 1, 1, 0, 4);
...@@ -723,7 +719,7 @@ public class ImeTest extends ContentShellTestBase { ...@@ -723,7 +719,7 @@ public class ImeTest extends ContentShellTestBase {
commitText("\n", 1); commitText("\n", 1);
waitAndVerifyUpdateSelection(5, 2, 2, -1, -1); waitAndVerifyUpdateSelection(5, 2, 2, -1, -1);
assertTextsAroundCursor("h\n", "", "llo "); assertTextsAroundCursor("h\n", null, "llo ");
} }
// http://crbug.com/445499 // http://crbug.com/445499
...@@ -807,7 +803,7 @@ public class ImeTest extends ContentShellTestBase { ...@@ -807,7 +803,7 @@ public class ImeTest extends ContentShellTestBase {
commitText(smiley, 1); commitText(smiley, 1);
waitAndVerifyUpdateSelection(0, 2, 2, -1, -1); waitAndVerifyUpdateSelection(0, 2, 2, -1, -1);
assertTextsAroundCursor(smiley, "", ""); assertTextsAroundCursor(smiley, null, "");
// DEL, sent via dispatchKeyEvent like it is in Android WebView or a physical keyboard. // DEL, sent via dispatchKeyEvent like it is in Android WebView or a physical keyboard.
dispatchKeyEvent(new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_DEL)); dispatchKeyEvent(new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_DEL));
...@@ -880,13 +876,13 @@ public class ImeTest extends ContentShellTestBase { ...@@ -880,13 +876,13 @@ public class ImeTest extends ContentShellTestBase {
dispatchKeyEvent(new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_ENTER)); dispatchKeyEvent(new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_ENTER));
dispatchKeyEvent(new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_ENTER)); dispatchKeyEvent(new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_ENTER));
waitAndVerifyUpdateSelection(1, 2, 2, -1, -1); waitAndVerifyUpdateSelection(1, 2, 2, -1, -1);
assertTextsAroundCursor("a\n", "", "\n"); assertTextsAroundCursor("a\n", null, "\n");
// Type 'b'. // Type 'b'.
dispatchKeyEvent(new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_B)); dispatchKeyEvent(new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_B));
dispatchKeyEvent(new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_B)); dispatchKeyEvent(new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_B));
waitAndVerifyUpdateSelection(2, 3, 3, -1, -1); waitAndVerifyUpdateSelection(2, 3, 3, -1, -1);
assertTextsAroundCursor("a\nb", "", ""); assertTextsAroundCursor("a\nb", null, "");
} }
@SmallTest @SmallTest
...@@ -989,11 +985,11 @@ public class ImeTest extends ContentShellTestBase { ...@@ -989,11 +985,11 @@ public class ImeTest extends ContentShellTestBase {
dispatchKeyEvent(new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_ENTER)); dispatchKeyEvent(new KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_ENTER));
dispatchKeyEvent(new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_ENTER)); dispatchKeyEvent(new KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_ENTER));
waitAndVerifyUpdateSelection(1, 6, 6, -1, -1); waitAndVerifyUpdateSelection(1, 6, 6, -1, -1);
assertTextsAroundCursor("hello\n", "", "\n"); assertTextsAroundCursor("hello\n", null, "\n");
commitText("world", 1); commitText("world", 1);
waitAndVerifyUpdateSelection(2, 11, 11, -1, -1); waitAndVerifyUpdateSelection(2, 11, 11, -1, -1);
assertTextsAroundCursor("hello\nworld", "", ""); assertTextsAroundCursor("hello\nworld", null, "");
} }
@SmallTest @SmallTest
...@@ -1017,7 +1013,7 @@ public class ImeTest extends ContentShellTestBase { ...@@ -1017,7 +1013,7 @@ public class ImeTest extends ContentShellTestBase {
commitText("world", 1); commitText("world", 1);
waitAndVerifyUpdateSelection(3, 11, 11, -1, -1); waitAndVerifyUpdateSelection(3, 11, 11, -1, -1);
assertTextsAroundCursor("hello\nworld", "", ""); assertTextsAroundCursor("hello\nworld", null, "");
} }
@SmallTest @SmallTest
...@@ -1048,7 +1044,7 @@ public class ImeTest extends ContentShellTestBase { ...@@ -1048,7 +1044,7 @@ public class ImeTest extends ContentShellTestBase {
// make take some round trip time until we get the correct value. // make take some round trip time until we get the correct value.
waitUntilGetCharacterBeforeCursorBecomes("l"); waitUntilGetCharacterBeforeCursorBecomes("l");
} else { } else {
assertTextsAroundCursor("hell", "", "o"); assertTextsAroundCursor("hell", null, "o");
} }
} }
...@@ -1100,7 +1096,7 @@ public class ImeTest extends ContentShellTestBase { ...@@ -1100,7 +1096,7 @@ public class ImeTest extends ContentShellTestBase {
cut(); cut();
waitAndVerifyUpdateSelection(2, 0, 0, -1, -1); waitAndVerifyUpdateSelection(2, 0, 0, -1, -1);
assertTextsAroundCursor("", "", ""); assertTextsAroundCursor("", null, "");
DOMUtils.longPressNode(this, mContentViewCore, "input_text"); DOMUtils.longPressNode(this, mContentViewCore, "input_text");
CriteriaHelper.pollUiThread(new Criteria() { CriteriaHelper.pollUiThread(new Criteria() {
......
...@@ -27,7 +27,7 @@ public class TextInputStateTest { ...@@ -27,7 +27,7 @@ public class TextInputStateTest {
assertEquals("hel", state.getTextBeforeSelection(3)); assertEquals("hel", state.getTextBeforeSelection(3));
assertEquals("el", state.getTextBeforeSelection(2)); assertEquals("el", state.getTextBeforeSelection(2));
assertEquals("", state.getTextBeforeSelection(0)); assertEquals("", state.getTextBeforeSelection(0));
assertEquals("", state.getSelectedText()); assertEquals(null, state.getSelectedText());
} }
@Test @Test
......
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