From 77f7c0c078b54d184d1bd381ce359a58d9236717 Mon Sep 17 00:00:00 2001 From: Lan Wei Date: Wed, 21 Sep 2022 16:07:02 +0800 Subject: [PATCH] Fix the issue of committing text with newCursorPosition=0 Fix the issue that cusor is incorrectly positioned with the following API: - commitText(text, 0) - setComposingText(text, 0) Bug: 21476564 Bug: 240359838 Fix: 21476564 Test: atest FrameworksCoreTests:BaseInputConnectionTest Change-Id: I537dc384deffaf74f38bbd158e0f433ca67fa047 --- .../view/inputmethod/BaseInputConnection.java | 43 +++++++++++++------ .../inputmethod/BaseInputConnectionTest.java | 30 ++++++------- 2 files changed, 45 insertions(+), 28 deletions(-) diff --git a/core/java/android/view/inputmethod/BaseInputConnection.java b/core/java/android/view/inputmethod/BaseInputConnection.java index a72f0d50a3a9b..89ea060de5146 100644 --- a/core/java/android/view/inputmethod/BaseInputConnection.java +++ b/core/java/android/view/inputmethod/BaseInputConnection.java @@ -237,7 +237,7 @@ public class BaseInputConnection implements InputConnection { */ @Override public boolean commitText(CharSequence text, int newCursorPosition) { - if (DEBUG) Log.v(TAG, "commitText " + text); + if (DEBUG) Log.v(TAG, "commitText(" + text + ", " + newCursorPosition + ")"); replaceText(text, newCursorPosition, false); sendCurrentText(); return true; @@ -260,7 +260,7 @@ public class BaseInputConnection implements InputConnection { */ @Override public boolean deleteSurroundingText(int beforeLength, int afterLength) { - if (DEBUG) Log.v(TAG, "deleteSurroundingText " + beforeLength + " / " + afterLength); + if (DEBUG) Log.v(TAG, "deleteSurroundingText(" + beforeLength + ", " + afterLength + ")"); final Editable content = getEditable(); if (content == null) return false; @@ -747,13 +747,14 @@ public class BaseInputConnection implements InputConnection { */ @Override public boolean setComposingText(CharSequence text, int newCursorPosition) { - if (DEBUG) Log.v(TAG, "setComposingText " + text); + if (DEBUG) Log.v(TAG, "setComposingText(" + text + ", " + newCursorPosition + ")"); replaceText(text, newCursorPosition, true); return true; } @Override public boolean setComposingRegion(int start, int end) { + if (DEBUG) Log.v(TAG, "setComposingRegion(" + start + ", " + end + ")"); final Editable content = getEditable(); if (content != null) { beginBatchEdit(); @@ -797,7 +798,7 @@ public class BaseInputConnection implements InputConnection { /** The default implementation changes the selection position in the current editable text. */ @Override public boolean setSelection(int start, int end) { - if (DEBUG) Log.v(TAG, "setSelection " + start + ", " + end); + if (DEBUG) Log.v(TAG, "setSelection(" + start + ", " + end + ")"); final Editable content = getEditable(); if (content == null) return false; int len = content.length(); @@ -950,11 +951,22 @@ public class BaseInputConnection implements InputConnection { setComposingSpans(sp); } - if (DEBUG) Log.v(TAG, "Replacing from " + a + " to " + b + " with \"" - + text + "\", composing=" + composing - + ", type=" + text.getClass().getCanonicalName()); - if (DEBUG) { + Log.v( + TAG, + "Replacing from " + + a + + " to " + + b + + " with \"" + + text + + "\", composing=" + + composing + + ", newCursorPosition=" + + newCursorPosition + + ", type=" + + text.getClass().getCanonicalName()); + LogPrinter lp = new LogPrinter(Log.VERBOSE, TAG); lp.println("Current text:"); TextUtils.dumpSpans(content, lp, " "); @@ -962,10 +974,10 @@ public class BaseInputConnection implements InputConnection { TextUtils.dumpSpans(text, lp, " "); } - // Position the cursor appropriately, so that after replacing the - // desired range of text it will be located in the correct spot. - // This allows us to deal with filters performing edits on the text - // we are providing here. + // Position the cursor appropriately, so that after replacing the desired range of text it + // will be located in the correct spot. + // This allows us to deal with filters performing edits on the text we are providing here. + int requestedNewCursorPosition = newCursorPosition; if (newCursorPosition > 0) { newCursorPosition += b - 1; } else { @@ -977,6 +989,13 @@ public class BaseInputConnection implements InputConnection { content.replace(a, b, text); + // Replace (or insert) to the cursor (a==b==newCursorPosition) will position the cursor to + // the end of the new replaced/inserted text, we need to re-position the cursor to the start + // according the API definition: "if <= 0, this is relative to the start of the text". + if (requestedNewCursorPosition == 0 && a == b) { + Selection.setSelection(content, newCursorPosition); + } + if (DEBUG) { LogPrinter lp = new LogPrinter(Log.VERBOSE, TAG); lp.println("Final text:"); diff --git a/core/tests/coretests/src/android/view/inputmethod/BaseInputConnectionTest.java b/core/tests/coretests/src/android/view/inputmethod/BaseInputConnectionTest.java index 2bb5abefd304e..c1ecc53bbe3d3 100644 --- a/core/tests/coretests/src/android/view/inputmethod/BaseInputConnectionTest.java +++ b/core/tests/coretests/src/android/view/inputmethod/BaseInputConnectionTest.java @@ -78,25 +78,24 @@ public class BaseInputConnectionTest { verifyContent("text1text2text3", 15, 15, -1, -1); // before commit: "text1text2text3|" - // after commit: "text1text2text3text4|" - // BUG(b/21476564): this behavior is inconsistent with API description. + // after commit: "text1text2text3|text4" assertThat(mBaseInputConnection.commitText("text4", 0)).isTrue(); - verifyContent("text1text2text3text4", 20, 20, -1, -1); + verifyContent("text1text2text3text4", 15, 15, -1, -1); - // before commit: "text1text2text3text4|" - // after commit: "text1text2text3text|4text5" + // before commit: "text1text2text3|text4" + // after commit: "text1text2text|3text5text4" assertThat(mBaseInputConnection.commitText("text5", -1)).isTrue(); - verifyContent("text1text2text3text4text5", 19, 19, -1, -1); + verifyContent("text1text2text3text5text4", 14, 14, -1, -1); - // before commit: "text1text2text3text|4text5" - // after commit: "text1text2text3te|xttext64text5" + // before commit: "text1text2text|3text5text4" + // after commit: "text1text2te|xttext63text5text4" assertThat(mBaseInputConnection.commitText("text6", -2)).isTrue(); - verifyContent("text1text2text3texttext64text5", 17, 17, -1, -1); + verifyContent("text1text2texttext63text5text4", 12, 12, -1, -1); - // before commit: "text1text2text3te|xttext64text5" - // after commit: "|text1text2text3tetext7xttext64text5" + // before commit: "text1text2te|xttext63text5text4" + // after commit: "|text1text2tetext7xttext63text5text4" assertThat(mBaseInputConnection.commitText("text7", -100)).isTrue(); - verifyContent("text1text2text3tetext7xttext64text5", 0, 0, -1, -1); + verifyContent("text1text2tetext7xttext63text5text4", 0, 0, -1, -1); } @Test @@ -296,12 +295,11 @@ public class BaseInputConnectionTest { verifyContent("abcdef", 6, 6, 3, 6); // before set composing text: "abc|" - // after set composing text: "abcdef|" - // --- - // BUG(b/21476564): this behavior is inconsistent with API description. + // after set composing text: "abc|def" + // --- prepareContent("abc", 3, 3, -1, -1); assertThat(mBaseInputConnection.setComposingText("def", 0)).isTrue(); - verifyContent("abcdef", 6, 6, 3, 6); + verifyContent("abcdef", 3, 3, 3, 6); // before set composing text: "abc|" // after set composing text: "ab|cdef"