From fb684cebe8f5dac1bdb6cfa7085a07ddc66acede Mon Sep 17 00:00:00 2001 From: Jean Chalard Date: Thu, 2 Feb 2012 17:18:03 +0900 Subject: [PATCH] Stop inserting a space after a manually picked word. Bug: 4994861 Change-Id: I6ae256a94dbaddea4304304779d17025620c5025 --- .../keyboard/KeyboardActionListener.java | 7 +- .../inputmethod/latin/InputAttributes.java | 14 +- .../android/inputmethod/latin/LatinIME.java | 137 +++++++++--------- .../inputmethod/latin/InputLogicTests.java | 6 +- 4 files changed, 78 insertions(+), 86 deletions(-) diff --git a/java/src/com/android/inputmethod/keyboard/KeyboardActionListener.java b/java/src/com/android/inputmethod/keyboard/KeyboardActionListener.java index dce2c37f2..6e13b95b5 100644 --- a/java/src/com/android/inputmethod/keyboard/KeyboardActionListener.java +++ b/java/src/com/android/inputmethod/keyboard/KeyboardActionListener.java @@ -48,14 +48,17 @@ public interface KeyboardActionListener { * presses of a key adjacent to the intended key. * @param x x-coordinate pixel of touched event. If {@link #onCodeInput} is not called by * {@link PointerTracker#onTouchEvent} or so, the value should be - * {@link #NOT_A_TOUCH_COORDINATE}. + * {@link #NOT_A_TOUCH_COORDINATE}. If it's called on insertion from the suggestion + * strip, it should be {@link #SUGGESTION_STRIP_COORDINATE}. * @param y y-coordinate pixel of touched event. If {@link #onCodeInput} is not called by * {@link PointerTracker#onTouchEvent} or so, the value should be - * {@link #NOT_A_TOUCH_COORDINATE}. + * {@link #NOT_A_TOUCH_COORDINATE}. If it's called on insertion from the suggestion + * strip, it should be {@link #SUGGESTION_STRIP_COORDINATE}. */ public void onCodeInput(int primaryCode, int[] keyCodes, int x, int y); public static final int NOT_A_TOUCH_COORDINATE = -1; + public static final int SUGGESTION_STRIP_COORDINATE = -2; /** * Sends a sequence of characters to the listener. diff --git a/java/src/com/android/inputmethod/latin/InputAttributes.java b/java/src/com/android/inputmethod/latin/InputAttributes.java index f5cf953c4..3de5c1d48 100644 --- a/java/src/com/android/inputmethod/latin/InputAttributes.java +++ b/java/src/com/android/inputmethod/latin/InputAttributes.java @@ -28,7 +28,6 @@ import com.android.inputmethod.compat.InputTypeCompatUtils; public class InputAttributes { private final String TAG = InputAttributes.class.getSimpleName(); - final public boolean mInsertSpaceOnPickSuggestionManually; final public boolean mInputTypeNoAutoCorrect; final public boolean mIsSettingsSuggestionStripOn; final public boolean mApplicationSpecifiedCompletionOn; @@ -52,7 +51,6 @@ public class InputAttributes { + " imeOptions=0x%08x", inputType, editorInfo.imeOptions)); } - mInsertSpaceOnPickSuggestionManually = false; mIsSettingsSuggestionStripOn = false; mInputTypeNoAutoCorrect = false; mApplicationSpecifiedCompletionOn = false; @@ -80,15 +78,6 @@ public class InputAttributes { mIsSettingsSuggestionStripOn = true; } - if (InputTypeCompatUtils.isEmailVariation(variation) - || variation == InputType.TYPE_TEXT_VARIATION_PERSON_NAME) { - // The point in turning this off is that we don't want to insert a space after - // a name when filling a form: we can't delete trailing spaces when changing fields - mInsertSpaceOnPickSuggestionManually = false; - } else { - mInsertSpaceOnPickSuggestionManually = true; - } - // If it's a browser edit field and auto correct is not ON explicitly, then // disable auto correction, but keep suggestions on. // If NO_SUGGESTIONS is set, don't do prediction. @@ -109,8 +98,7 @@ public class InputAttributes { // Pretty print @Override public String toString() { - return "\n mInsertSpaceOnPickSuggestionManually = " + mInsertSpaceOnPickSuggestionManually - + "\n mInputTypeNoAutoCorrect = " + mInputTypeNoAutoCorrect + return "\n mInputTypeNoAutoCorrect = " + mInputTypeNoAutoCorrect + "\n mIsSettingsSuggestionStripOn = " + mIsSettingsSuggestionStripOn + "\n mApplicationSpecifiedCompletionOn = " + mApplicationSpecifiedCompletionOn; } diff --git a/java/src/com/android/inputmethod/latin/LatinIME.java b/java/src/com/android/inputmethod/latin/LatinIME.java index 1bc55a583..24007f95a 100644 --- a/java/src/com/android/inputmethod/latin/LatinIME.java +++ b/java/src/com/android/inputmethod/latin/LatinIME.java @@ -160,18 +160,21 @@ public class LatinIME extends InputMethodServiceCompatWrapper implements Keyboar SUGGESTION_VISIBILILTY_HIDE_VALUE }; - // Magic space: a space that should disappear on space/apostrophe insertion, move after the - // punctuation on punctuation insertion, and become a real space on alpha char insertion. - // Weak space: a space that should be swapped only by suggestion strip punctuation. + private static final int SPACE_STATE_NONE = 0; // Double space: the state where the user pressed space twice quickly, which LatinIME // resolved as period-space. Undoing this converts the period to a space. + private static final int SPACE_STATE_DOUBLE = 1; // Swap punctuation: the state where a (weak or magic) space and a punctuation from the // suggestion strip have just been swapped. Undoing this swaps them back. - private static final int SPACE_STATE_NONE = 0; - private static final int SPACE_STATE_DOUBLE = 1; private static final int SPACE_STATE_SWAP_PUNCTUATION = 2; - private static final int SPACE_STATE_MAGIC = 3; - private static final int SPACE_STATE_WEAK = 4; + // Weak space: a space that should be swapped only by suggestion strip punctuation. Weak + // spaces happen when the user presses space, accepting the current suggestion (whether + // it's an auto-correction or not). + private static final int SPACE_STATE_WEAK = 3; + // Phantom space: a not-yet-inserted space that should get inserted on the next input, + // character provided it's not a separator. If it's a separator, the phantom space is dropped. + // Phantom spaces happen when a user chooses a word from the suggestion strip. + private static final int SPACE_STATE_PHANTOM = 4; // Current space state of the input method. This can be any of the above constants. private int mSpaceState; @@ -1162,18 +1165,6 @@ public class LatinIME extends InputMethodServiceCompatWrapper implements Keyboar return false; } - // "ic" must not be null - private static void maybeRemovePreviousPeriod(final InputConnection ic, CharSequence text) { - // When the text's first character is '.', remove the previous period - // if there is one. - final CharSequence lastOne = ic.getTextBeforeCursor(1, 0); - if (lastOne != null && lastOne.length() == 1 - && lastOne.charAt(0) == Keyboard.CODE_PERIOD - && text.charAt(0) == Keyboard.CODE_PERIOD) { - ic.deleteSurroundingText(1, 0); - } - } - // "ic" may be null private static void removeTrailingSpaceWhileInBatchEdit(final InputConnection ic) { if (ic == null) return; @@ -1234,26 +1225,9 @@ public class LatinIME extends InputMethodServiceCompatWrapper implements Keyboar } private void insertPunctuationFromSuggestionStrip(final InputConnection ic, final int code) { - final CharSequence beforeText = ic != null ? ic.getTextBeforeCursor(1, 0) : null; - final int toLeft = TextUtils.isEmpty(beforeText) ? 0 : beforeText.charAt(0); - final boolean shouldRegisterSwapPunctuation; - // If we have a space left of the cursor and it's a weak or a magic space, then we should - // swap it, and override the space state with SPACESTATE_SWAP_PUNCTUATION. - // To swap it, we fool handleSeparator to think the previous space state was a - // magic space. - if (Keyboard.CODE_SPACE == toLeft && mSpaceState == SPACE_STATE_WEAK - && mSettingsValues.isMagicSpaceSwapper(code)) { - mSpaceState = SPACE_STATE_MAGIC; - shouldRegisterSwapPunctuation = true; - } else { - shouldRegisterSwapPunctuation = false; - } onCodeInput(code, new int[] { code }, - KeyboardActionListener.NOT_A_TOUCH_COORDINATE, - KeyboardActionListener.NOT_A_TOUCH_COORDINATE); - if (shouldRegisterSwapPunctuation) { - mSpaceState = SPACE_STATE_SWAP_PUNCTUATION; - } + KeyboardActionListener.SUGGESTION_STRIP_COORDINATE, + KeyboardActionListener.SUGGESTION_STRIP_COORDINATE); } // Implementation of {@link KeyboardActionListener}. @@ -1331,7 +1305,10 @@ public class LatinIME extends InputMethodServiceCompatWrapper implements Keyboar if (ic == null) return; ic.beginBatchEdit(); commitTyped(ic); - maybeRemovePreviousPeriod(ic, text); + text = specificTldProcessingOnTextInput(ic, text); + if (SPACE_STATE_PHANTOM == mSpaceState) { + sendKeyChar((char)Keyboard.CODE_SPACE); + } ic.commitText(text, 1); ic.endBatchEdit(); mKeyboardSwitcher.updateShiftState(); @@ -1341,6 +1318,24 @@ public class LatinIME extends InputMethodServiceCompatWrapper implements Keyboar resetComposingState(true /* alsoResetLastComposedWord */); } + // ic may not be null + private CharSequence specificTldProcessingOnTextInput(final InputConnection ic, + final CharSequence text) { + if (text.length() <= 1 || text.charAt(0) != Keyboard.CODE_PERIOD + || !Character.isLetter(text.charAt(1))) { + // Not a tld: do nothing. + return text; + } + final CharSequence lastOne = ic.getTextBeforeCursor(1, 0); + if (lastOne != null && lastOne.length() == 1 + && lastOne.charAt(0) == Keyboard.CODE_PERIOD) { + mSpaceState = SPACE_STATE_NONE; + return text.subSequence(1, text.length()); + } else { + return text; + } + } + @Override public void onCancelInput() { // User released a finger outside any key @@ -1492,13 +1487,18 @@ public class LatinIME extends InputMethodServiceCompatWrapper implements Keyboar // "ic" may be null without this crashing, but the behavior will be really strange private void handleCharacterWhileInBatchEdit(final int primaryCode, final int[] keyCodes, final int x, final int y, final int spaceState, final InputConnection ic) { - if (SPACE_STATE_MAGIC == spaceState - && mSettingsValues.isMagicSpaceStripper(primaryCode)) { - if (null != ic) removeTrailingSpaceWhileInBatchEdit(ic); - } - boolean isComposingWord = mWordComposer.isComposingWord(); int code = primaryCode; + + if (SPACE_STATE_PHANTOM == spaceState && + !mSettingsValues.isSymbolExcludedFromWordSeparators(primaryCode)) { + if (isComposingWord) { + // Sanity check + throw new RuntimeException("Should not be composing here"); + } + sendKeyChar((char)Keyboard.CODE_SPACE); + } + if ((isAlphabet(code) || mSettingsValues.isSymbolExcludedFromWordSeparators(code)) && isSuggestionsRequested() && !isCursorTouchingWord()) { if (!isComposingWord) { @@ -1530,10 +1530,6 @@ public class LatinIME extends InputMethodServiceCompatWrapper implements Keyboar } else { sendKeyChar((char)code); } - if (SPACE_STATE_MAGIC == spaceState - && mSettingsValues.isMagicSpaceSwapper(primaryCode)) { - if (null != ic) swapSwapperAndSpaceWhileInBatchEdit(ic); - } if (mSettingsValues.isWordSeparator(code)) { Utils.Stats.onSeparator((char)code, x, y); @@ -1575,24 +1571,28 @@ public class LatinIME extends InputMethodServiceCompatWrapper implements Keyboar } } - final boolean swapMagicSpace; - if (Keyboard.CODE_ENTER == primaryCode && (SPACE_STATE_MAGIC == spaceState - || SPACE_STATE_SWAP_PUNCTUATION == spaceState)) { + final boolean swapWeakSpace; + if (Keyboard.CODE_ENTER == primaryCode && SPACE_STATE_SWAP_PUNCTUATION == spaceState) { removeTrailingSpaceWhileInBatchEdit(ic); - swapMagicSpace = false; - } else if (SPACE_STATE_MAGIC == spaceState) { + swapWeakSpace = false; + } else if ((SPACE_STATE_WEAK == spaceState || SPACE_STATE_SWAP_PUNCTUATION == spaceState) + && KeyboardActionListener.SUGGESTION_STRIP_COORDINATE == x) { if (mSettingsValues.isMagicSpaceSwapper(primaryCode)) { - swapMagicSpace = true; + swapWeakSpace = true; } else { - swapMagicSpace = false; + swapWeakSpace = false; if (mSettingsValues.isMagicSpaceStripper(primaryCode)) { removeTrailingSpaceWhileInBatchEdit(ic); } } } else { - swapMagicSpace = false; + swapWeakSpace = false; } + // TODO: rethink interactions of sendKeyChar, commitText("\n") and actions. sendKeyChar + // with a CODE_ENTER parameter will have the default InputMethodService implementation + // possibly redirect on the keyboard action. That may be the right thing to do, but + // on Shift+Enter, it's generally not, so we may want to do the redirection right here. sendKeyChar((char)primaryCode); if (Keyboard.CODE_SPACE == primaryCode) { @@ -1610,9 +1610,17 @@ public class LatinIME extends InputMethodServiceCompatWrapper implements Keyboar mHandler.postUpdateBigramPredictions(); } } else { - if (swapMagicSpace) { + if (swapWeakSpace) { swapSwapperAndSpaceWhileInBatchEdit(ic); - mSpaceState = SPACE_STATE_MAGIC; + mSpaceState = SPACE_STATE_WEAK; + } else if (SPACE_STATE_PHANTOM == spaceState) { + // If we are in phantom space state, and the user presses a separator, we want to + // stay in phantom space state so that the next keypress has a chance to add the + // space. For example, if I type "Good dat", pick "day" from the suggestion strip + // then insert a comma and go on to typing the next word, I want the space to be + // inserted automatically before the next word, the same way it is when I don't + // input the comma. + mSpaceState = SPACE_STATE_PHANTOM; } // Set punctuation right away. onUpdateSelection will fire but tests whether it is @@ -1921,10 +1929,9 @@ public class LatinIME extends InputMethodServiceCompatWrapper implements Keyboar } else { addToOnlyBigramDictionary(suggestion, 1); } - // Follow it with a space - if (mInputAttributes.mInsertSpaceOnPickSuggestionManually) { - sendMagicSpace(); - } + mSpaceState = SPACE_STATE_PHANTOM; + // TODO: is this necessary? + mKeyboardSwitcher.updateShiftState(); // We should show the "Touch again to save" hint if the user pressed the first entry // AND either: @@ -2259,12 +2266,6 @@ public class LatinIME extends InputMethodServiceCompatWrapper implements Keyboar return mSettingsValues.isWordSeparator(code); } - private void sendMagicSpace() { - sendKeyChar((char)Keyboard.CODE_SPACE); - mSpaceState = SPACE_STATE_MAGIC; - mKeyboardSwitcher.updateShiftState(); - } - public boolean preferCapitalization() { return mWordComposer.isFirstCharCapitalized(); } diff --git a/tests/src/com/android/inputmethod/latin/InputLogicTests.java b/tests/src/com/android/inputmethod/latin/InputLogicTests.java index ee3a97fab..693352c85 100644 --- a/tests/src/com/android/inputmethod/latin/InputLogicTests.java +++ b/tests/src/com/android/inputmethod/latin/InputLogicTests.java @@ -186,7 +186,7 @@ public class InputLogicTests extends ServiceTestCase { } public void testCancelDoubleSpace() { - final String STRING_TO_TYPE = "tgis "; + final String STRING_TO_TYPE = "this "; final String EXPECTED_RESULT = "this "; type(STRING_TO_TYPE); type(Keyboard.CODE_DELETE); @@ -202,7 +202,7 @@ public class InputLogicTests extends ServiceTestCase { mInputConnection.setSelection(NEW_CURSOR_POSITION, NEW_CURSOR_POSITION); mLatinIME.onUpdateSelection(0, 0, NEW_CURSOR_POSITION, NEW_CURSOR_POSITION, -1, -1); type(Keyboard.CODE_DELETE); - assertEquals("auto correct then move curor to start of line then backspace", + assertEquals("auto correct then move cursor to start of line then backspace", EXPECTED_RESULT, mTextView.getText().toString()); } @@ -215,7 +215,7 @@ public class InputLogicTests extends ServiceTestCase { mInputConnection.setSelection(NEW_CURSOR_POSITION, NEW_CURSOR_POSITION); mLatinIME.onUpdateSelection(0, 0, NEW_CURSOR_POSITION, NEW_CURSOR_POSITION, -1, -1); type(Keyboard.CODE_DELETE); - assertEquals("auto correct then move curor then backspace", + assertEquals("auto correct then move cursor then backspace", EXPECTED_RESULT, mTextView.getText().toString()); }