From f0af452ce261590b5978a1bb679ce27b71f9dc70 Mon Sep 17 00:00:00 2001 From: Jean Chalard Date: Tue, 25 Jun 2013 19:26:30 +0900 Subject: [PATCH] Do not re-resume suggestion if it's not needed. This is much better interface-wise. It eliminates all blinking of the line in the practice. Bug: 8874148 Bug: 8864306 Change-Id: I87754e44784327c2e9c8b162d598d145e20668e8 --- .../android/inputmethod/latin/LatinIME.java | 7 +- .../inputmethod/latin/WordComposer.java | 34 +++++++ .../inputmethod/latin/WordComposerTests.java | 93 +++++++++++++++++++ 3 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 tests/src/com/android/inputmethod/latin/WordComposerTests.java diff --git a/java/src/com/android/inputmethod/latin/LatinIME.java b/java/src/com/android/inputmethod/latin/LatinIME.java index 0560cf528..f3fc3177b 100644 --- a/java/src/com/android/inputmethod/latin/LatinIME.java +++ b/java/src/com/android/inputmethod/latin/LatinIME.java @@ -961,7 +961,11 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen // TODO: is it still necessary to test for composingSpan related stuff? final boolean selectionChangedOrSafeToReset = selectionChanged || (!mWordComposer.isComposingWord()) || noComposingSpan; - if (selectionChangedOrSafeToReset) { + final boolean hasOrHadSelection = (oldSelStart != oldSelEnd + || newSelStart != newSelEnd); + final int moveAmount = newSelStart - oldSelStart; + if (selectionChangedOrSafeToReset && (hasOrHadSelection + || !mWordComposer.moveCursorByAndReturnIfInsideComposingWord(moveAmount))) { // If we are composing a word and moving the cursor, we would want to set a // suggestion span for recorrection to work correctly. Unfortunately, that // would involve the keyboard committing some new text, which would move the @@ -2535,6 +2539,7 @@ public class LatinIME extends InputMethodService implements KeyboardActionListen } } mWordComposer.setComposingWord(typedWord, mKeyboardSwitcher.getKeyboard()); + // TODO: this is in chars but the callee expects code points! mWordComposer.setCursorPositionWithinWord(numberOfCharsInWordBeforeCursor); mConnection.setComposingRegion( mLastSelectionStart - numberOfCharsInWordBeforeCursor, diff --git a/java/src/com/android/inputmethod/latin/WordComposer.java b/java/src/com/android/inputmethod/latin/WordComposer.java index e078f03f4..2babe8b0c 100644 --- a/java/src/com/android/inputmethod/latin/WordComposer.java +++ b/java/src/com/android/inputmethod/latin/WordComposer.java @@ -192,6 +192,40 @@ public final class WordComposer { return mCursorPositionWithinWord != mCodePointSize; } + /** + * When the cursor is moved by the user, we need to update its position. + * If it falls inside the currently composing word, we don't reset the composition, and + * only update the cursor position. + * + * @param expectedMoveAmount How many java chars to move the cursor. Negative values move + * the cursor backward, positive values move the cursor forward. + * @return true if the cursor is still inside the composing word, false otherwise. + */ + public boolean moveCursorByAndReturnIfInsideComposingWord(final int expectedMoveAmount) { + int actualMoveAmountWithinWord = 0; + int cursorPos = mCursorPositionWithinWord; + if (expectedMoveAmount >= 0) { + // Moving the cursor forward for the expected amount or until the end of the word has + // been reached, whichever comes first. + while (actualMoveAmountWithinWord < expectedMoveAmount && cursorPos < mCodePointSize) { + actualMoveAmountWithinWord += Character.charCount(mPrimaryKeyCodes[cursorPos]); + ++cursorPos; + } + } else { + // Moving the cursor backward for the expected amount or until the start of the word + // has been reached, whichever comes first. + while (actualMoveAmountWithinWord > expectedMoveAmount && cursorPos > 0) { + --cursorPos; + actualMoveAmountWithinWord -= Character.charCount(mPrimaryKeyCodes[cursorPos]); + } + } + // If the actual and expected amounts differ, we crossed the start or the end of the word + // so the result would not be inside the composing word. + if (actualMoveAmountWithinWord != expectedMoveAmount) return false; + mCursorPositionWithinWord = cursorPos; + return true; + } + public void setBatchInputPointers(final InputPointers batchPointers) { mInputPointers.set(batchPointers); mIsBatchMode = true; diff --git a/tests/src/com/android/inputmethod/latin/WordComposerTests.java b/tests/src/com/android/inputmethod/latin/WordComposerTests.java new file mode 100644 index 000000000..1434c6b63 --- /dev/null +++ b/tests/src/com/android/inputmethod/latin/WordComposerTests.java @@ -0,0 +1,93 @@ +/* + * Copyright (C) 2013 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.inputmethod.latin; + +import android.test.AndroidTestCase; +import android.test.suitebuilder.annotation.SmallTest; + +/** + * Unit tests for WordComposer. + */ +@SmallTest +public class WordComposerTests extends AndroidTestCase { + public void testMoveCursor() { + final WordComposer wc = new WordComposer(); + final String STR_WITHIN_BMP = "abcdef"; + wc.setComposingWord(STR_WITHIN_BMP, null); + assertEquals(wc.size(), + STR_WITHIN_BMP.codePointCount(0, STR_WITHIN_BMP.length())); + assertFalse(wc.isCursorFrontOrMiddleOfComposingWord()); + wc.setCursorPositionWithinWord(2); + assertTrue(wc.isCursorFrontOrMiddleOfComposingWord()); + // Move the cursor to after the 'd' + assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(2)); + assertTrue(wc.isCursorFrontOrMiddleOfComposingWord()); + // Move the cursor to after the 'e' + assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(1)); + assertTrue(wc.isCursorFrontOrMiddleOfComposingWord()); + assertEquals(wc.size(), 6); + // Move the cursor to after the 'f' + assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(1)); + assertFalse(wc.isCursorFrontOrMiddleOfComposingWord()); + // Move the cursor past the end of the word + assertFalse(wc.moveCursorByAndReturnIfInsideComposingWord(1)); + assertFalse(wc.moveCursorByAndReturnIfInsideComposingWord(15)); + + // \uD861\uDED7 is 𨛗, a character outside the BMP + final String STR_WITH_SUPPLEMENTARY_CHAR = "abcde\uD861\uDED7fgh"; + wc.setComposingWord(STR_WITH_SUPPLEMENTARY_CHAR, null); + assertEquals(wc.size(), STR_WITH_SUPPLEMENTARY_CHAR.codePointCount(0, + STR_WITH_SUPPLEMENTARY_CHAR.length())); + assertFalse(wc.isCursorFrontOrMiddleOfComposingWord()); + wc.setCursorPositionWithinWord(3); + assertTrue(wc.isCursorFrontOrMiddleOfComposingWord()); + assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(6)); + assertTrue(wc.isCursorFrontOrMiddleOfComposingWord()); + assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(1)); + assertFalse(wc.isCursorFrontOrMiddleOfComposingWord()); + + wc.setComposingWord(STR_WITH_SUPPLEMENTARY_CHAR, null); + wc.setCursorPositionWithinWord(3); + assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(7)); + + wc.setComposingWord(STR_WITH_SUPPLEMENTARY_CHAR, null); + wc.setCursorPositionWithinWord(3); + assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(7)); + + wc.setComposingWord(STR_WITH_SUPPLEMENTARY_CHAR, null); + wc.setCursorPositionWithinWord(3); + assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(-3)); + assertFalse(wc.moveCursorByAndReturnIfInsideComposingWord(-1)); + + wc.setComposingWord(STR_WITH_SUPPLEMENTARY_CHAR, null); + wc.setCursorPositionWithinWord(3); + assertFalse(wc.moveCursorByAndReturnIfInsideComposingWord(-9)); + + wc.setComposingWord(STR_WITH_SUPPLEMENTARY_CHAR, null); + assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(-10)); + + wc.setComposingWord(STR_WITH_SUPPLEMENTARY_CHAR, null); + assertFalse(wc.moveCursorByAndReturnIfInsideComposingWord(-11)); + + wc.setComposingWord(STR_WITH_SUPPLEMENTARY_CHAR, null); + assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(0)); + + wc.setComposingWord(STR_WITH_SUPPLEMENTARY_CHAR, null); + wc.setCursorPositionWithinWord(2); + assertTrue(wc.moveCursorByAndReturnIfInsideComposingWord(0)); + } +}