From b00a1d0c0adbdfc507676772201e979e539a2801 Mon Sep 17 00:00:00 2001 From: Amith Yamasani Date: Thu, 26 Aug 2010 12:22:58 -0700 Subject: [PATCH] Correction improvements - use the new InputConnection APIs for underlining and fetching the selected text. Bug: 2950652 Some refinements to correction behavior, such as retaining the tap-to-save when in correction mode. Use reflection to access the new InputConnection APIs with fallbacks, in case we run on older OS versions. Some refactoring to separate lookup for voice alternatives and typed alternatives. Change-Id: I7c4178bf7e6b47ee274c49fa7a694f8c2d50cea7 --- .../inputmethod/latin/CandidateView.java | 5 +- .../inputmethod/latin/EditingUtil.java | 190 +++++++++--- .../android/inputmethod/latin/LatinIME.java | 271 +++++++++--------- 3 files changed, 288 insertions(+), 178 deletions(-) diff --git a/java/src/com/android/inputmethod/latin/CandidateView.java b/java/src/com/android/inputmethod/latin/CandidateView.java index 7fcc3d532..4995727da 100755 --- a/java/src/com/android/inputmethod/latin/CandidateView.java +++ b/java/src/com/android/inputmethod/latin/CandidateView.java @@ -107,7 +107,6 @@ public class CandidateView extends View { } break; } - } }; @@ -333,6 +332,10 @@ public class CandidateView extends View { requestLayout(); } + public boolean isShowingAddToDictionaryHint() { + return mShowingAddToDictionary; + } + public void showAddToDictionaryHint(CharSequence word) { ArrayList suggestions = new ArrayList(); suggestions.add(word); diff --git a/java/src/com/android/inputmethod/latin/EditingUtil.java b/java/src/com/android/inputmethod/latin/EditingUtil.java index be31cb787..781d7fd4a 100644 --- a/java/src/com/android/inputmethod/latin/EditingUtil.java +++ b/java/src/com/android/inputmethod/latin/EditingUtil.java @@ -16,10 +16,13 @@ package com.android.inputmethod.latin; +import android.text.TextUtils; import android.view.inputmethod.ExtractedText; import android.view.inputmethod.ExtractedTextRequest; import android.view.inputmethod.InputConnection; +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.regex.Pattern; /** @@ -31,6 +34,11 @@ public class EditingUtil { */ private static final int LOOKBACK_CHARACTER_NUM = 15; + // Cache Method pointers + private static boolean sMethodsInitialized; + private static Method sMethodGetSelectedText; + private static Method sMethodSetComposingRegion; + private EditingUtil() {}; /** @@ -65,36 +73,16 @@ public class EditingUtil { return extracted.startOffset + extracted.selectionStart; } - private static int getSelectionEnd(InputConnection connection) { - ExtractedText extracted = connection.getExtractedText( - new ExtractedTextRequest(), 0); - if (extracted == null) { - return -1; - } - return extracted.startOffset + extracted.selectionEnd; - } - /** * @param connection connection to the current text field. * @param sep characters which may separate words + * @param range the range object to store the result into * @return the word that surrounds the cursor, including up to one trailing * separator. For example, if the field contains "he|llo world", where | * represents the cursor, then "hello " will be returned. */ public static String getWordAtCursor( - InputConnection connection, String separators) { - return getWordAtCursor(connection, separators, null); - } - - /** - * @param connection connection to the current text field. - * @param sep characters which may separate words - * @return the word that surrounds the cursor, including up to one trailing - * separator. For example, if the field contains "he|llo world", where | - * represents the cursor, then "hello " will be returned. - */ - public static String getWordAtCursor( - InputConnection connection, String separators, Range range) { + InputConnection connection, String separators, Range range) { Range r = getWordRangeAtCursor(connection, separators, range); return (r == null) ? null : r.word; } @@ -204,26 +192,146 @@ public class EditingUtil { } } - /** - * Checks if the cursor is touching/inside a word or the selection is for a whole - * word and no more and no less. - * @param range the Range object that contains the bounds of the word around the cursor - * @param start the start of the selection - * @param end the end of the selection, which could be the same as the start, if text is not - * in selection mode - * @return false if the selection is a partial word or straddling multiple words, true if - * the selection is a full word or there is no selection. - */ - public static boolean isFullWordOrInside(Range range, int start, int end) { - // Is the cursor inside or touching a word? - if (start == end) return true; + public static class SelectedWord { + public int start; + public int end; + public CharSequence word; + } - // Is it a selection? Then is the start of the selection the start of the word and - // the size of the selection the size of the word? Then return true - if (start < end - && (range.charsBefore == 0 && range.charsAfter == end - start)) { - return true; + /** + * Takes a character sequence with a single character and checks if the character occurs + * in a list of word separators or is empty. + * @param singleChar A CharSequence with null, zero or one character + * @param wordSeparators A String containing the word separators + * @return true if the character is at a word boundary, false otherwise + */ + private static boolean isWordBoundary(CharSequence singleChar, String wordSeparators) { + return TextUtils.isEmpty(singleChar) || wordSeparators.contains(singleChar); + } + + /** + * Checks if the cursor is inside a word or the current selection is a whole word. + * @param ic the InputConnection for accessing the text field + * @param selStart the start position of the selection within the text field + * @param selEnd the end position of the selection within the text field. This could be + * the same as selStart, if there's no selection. + * @param wordSeparators the word separator characters for the current language + * @return an object containing the text and coordinates of the selected/touching word, + * null if the selection/cursor is not marking a whole word. + */ + public static SelectedWord getWordAtCursorOrSelection(final InputConnection ic, + int selStart, int selEnd, String wordSeparators) { + if (selStart == selEnd) { + // There is just a cursor, so get the word at the cursor + EditingUtil.Range range = new EditingUtil.Range(); + CharSequence touching = getWordAtCursor(ic, wordSeparators, range); + if (!TextUtils.isEmpty(touching)) { + SelectedWord selWord = new SelectedWord(); + selWord.word = touching; + selWord.start = selStart - range.charsBefore; + selWord.end = selEnd + range.charsAfter; + return selWord; + } + } else { + // Is the previous character empty or a word separator? If not, return null. + CharSequence charsBefore = ic.getTextBeforeCursor(1, 0); + if (!isWordBoundary(charsBefore, wordSeparators)) { + return null; + } + + // Is the next character empty or a word separator? If not, return null. + CharSequence charsAfter = ic.getTextAfterCursor(1, 0); + if (!isWordBoundary(charsAfter, wordSeparators)) { + return null; + } + + // Extract the selection alone + CharSequence touching = getSelectedText(ic, selStart, selEnd); + if (TextUtils.isEmpty(touching)) return null; + // Is any part of the selection a separator? If so, return null. + final int length = touching.length(); + for (int i = 0; i < length; i++) { + if (wordSeparators.contains(touching.subSequence(i, i + 1))) { + return null; + } + } + // Prepare the selected word + SelectedWord selWord = new SelectedWord(); + selWord.start = selStart; + selWord.end = selEnd; + selWord.word = touching; + return selWord; + } + return null; + } + + /** + * Cache method pointers for performance + */ + private static void initializeMethodsForReflection() { + try { + // These will either both exist or not, so no need for separate try/catch blocks. + // If other methods are added later, use separate try/catch blocks. + sMethodGetSelectedText = InputConnection.class.getMethod("getSelectedText", int.class); + sMethodSetComposingRegion = InputConnection.class.getMethod("setComposingRegion", + int.class, int.class); + } catch (NoSuchMethodException exc) { + // Ignore + } + sMethodsInitialized = true; + } + + /** + * Returns the selected text between the selStart and selEnd positions. + */ + private static CharSequence getSelectedText(InputConnection ic, int selStart, int selEnd) { + // Use reflection, for backward compatibility + CharSequence result = null; + if (!sMethodsInitialized) { + initializeMethodsForReflection(); + } + if (sMethodGetSelectedText != null) { + try { + result = (CharSequence) sMethodGetSelectedText.invoke(ic, 0); + return result; + } catch (InvocationTargetException exc) { + // Ignore + } catch (IllegalArgumentException e) { + // Ignore + } catch (IllegalAccessException e) { + // Ignore + } + } + // Reflection didn't work, try it the poor way, by moving the cursor to the start, + // getting the text after the cursor and moving the text back to selected mode. + // TODO: Verify that this works properly in conjunction with + // LatinIME#onUpdateSelection + ic.setSelection(selStart, selEnd); + result = ic.getTextAfterCursor(selEnd - selStart, 0); + ic.setSelection(selStart, selEnd); + return result; + } + + /** + * Tries to set the text into composition mode if there is support for it in the framework. + */ + public static void underlineWord(InputConnection ic, SelectedWord word) { + // Use reflection, for backward compatibility + // If method not found, there's nothing we can do. It still works but just wont underline + // the word. + if (!sMethodsInitialized) { + initializeMethodsForReflection(); + } + if (sMethodSetComposingRegion != null) { + try { + sMethodSetComposingRegion.invoke(ic, word.start, word.end); + } catch (InvocationTargetException exc) { + // Ignore + } catch (IllegalArgumentException e) { + // Ignore + } catch (IllegalAccessException e) { + // Ignore + } } - return false; } } diff --git a/java/src/com/android/inputmethod/latin/LatinIME.java b/java/src/com/android/inputmethod/latin/LatinIME.java index 9312ce2c8..76f774c96 100644 --- a/java/src/com/android/inputmethod/latin/LatinIME.java +++ b/java/src/com/android/inputmethod/latin/LatinIME.java @@ -86,7 +86,6 @@ public class LatinIME extends InputMethodService static final boolean TRACE = false; static final boolean VOICE_INSTALLED = true; static final boolean ENABLE_VOICE_BUTTON = true; - private static final boolean MODIFY_TEXT_FOR_CORRECTION = false; private static final String PREF_VIBRATE_ON = "vibrate_on"; private static final String PREF_SOUND_ON = "sound_on"; @@ -767,16 +766,21 @@ public class LatinIME extends InputMethodService mLastSelectionEnd = newSelEnd; - // Check if we should go in or out of correction mode. - if (isPredictionOn() && mJustRevertedSeparator == null - && (candidatesStart == candidatesEnd || newSelStart != oldSelStart - || TextEntryState.isCorrecting()) - && (newSelStart < newSelEnd - 1 || (!mPredicting)) - && !mVoiceInputHighlighted) { - if (isCursorTouchingWord() || mLastSelectionStart < mLastSelectionEnd) { - postUpdateOldSuggestions(); - } else { - abortCorrection(false); + // Don't look for corrections if the keyboard is not visible + if (mKeyboardSwitcher != null && mKeyboardSwitcher.getInputView() != null + && mKeyboardSwitcher.getInputView().isShown()) { + // Check if we should go in or out of correction mode. + if (isPredictionOn() + && mJustRevertedSeparator == null + && (candidatesStart == candidatesEnd || newSelStart != oldSelStart + || TextEntryState.isCorrecting()) + && (newSelStart < newSelEnd - 1 || (!mPredicting)) + && !mVoiceInputHighlighted) { + if (isCursorTouchingWord() || mLastSelectionStart < mLastSelectionEnd) { + postUpdateOldSuggestions(); + } else { + abortCorrection(false); + } } } } @@ -818,7 +822,7 @@ public class LatinIME extends InputMethodService if (mCompletionOn) { mCompletions = completions; if (completions == null) { - setSuggestions(null, false, false, false); + clearSuggestions(); return; } @@ -1253,7 +1257,7 @@ public class LatinIME extends InputMethodService private void abortCorrection(boolean force) { if (force || TextEntryState.isCorrecting()) { getCurrentInputConnection().finishComposingText(); - setSuggestions(null, false, false, false); + clearSuggestions(); } } @@ -1266,7 +1270,9 @@ public class LatinIME extends InputMethodService // Assume input length is 1. This assumption fails for smiley face insertions. mVoiceInput.incrementTextModificationInsertCount(1); } - abortCorrection(false); + if (mLastSelectionStart == mLastSelectionEnd && TextEntryState.isCorrecting()) { + abortCorrection(false); + } if (isAlphabet(primaryCode) && isPredictionOn() && !isCursorTouchingWord()) { if (!mPredicting) { @@ -1495,7 +1501,7 @@ public class LatinIME extends InputMethodService } // Clear N-best suggestions - setSuggestions(null, false, false, true); + clearSuggestions(); FieldContext context = new FieldContext( getCurrentInputConnection(), @@ -1602,13 +1608,15 @@ public class LatinIME extends InputMethodService mVoiceInputHighlighted = true; mWordToSuggestions.putAll(mVoiceResults.alternatives); + } + private void clearSuggestions() { + setSuggestions(null, false, false, false); } private void setSuggestions( List suggestions, boolean completions, - boolean typedWordValid, boolean haveMinimalSuggestion) { @@ -1652,14 +1660,14 @@ public class LatinIME extends InputMethodService } private void showSuggestions(WordComposer word) { - //long startTime = System.currentTimeMillis(); // TIME MEASUREMENT! + // long startTime = System.currentTimeMillis(); // TIME MEASUREMENT! // TODO Maybe need better way of retrieving previous word CharSequence prevWord = EditingUtil.getPreviousWord(getCurrentInputConnection(), mWordSeparators); List stringList = mSuggest.getSuggestions( - mKeyboardSwitcher.getInputView(), word, false, prevWord); - //long stopTime = System.currentTimeMillis(); // TIME MEASUREMENT! - //Log.d("LatinIME","Suggest Total Time - " + (stopTime - startTime)); + mKeyboardSwitcher.getInputView(), word, false, prevWord); + // long stopTime = System.currentTimeMillis(); // TIME MEASUREMENT! + // Log.d("LatinIME","Suggest Total Time - " + (stopTime - startTime)); int[] nextLettersFrequencies = mSuggest.getNextLettersFrequencies(); @@ -1780,18 +1788,23 @@ public class LatinIME extends InputMethodService mJustAddedAutoSpace = true; } - // Fool the state watcher so that a subsequent backspace will not do a revert, unless - // we just did a correction, in which case we need to stay in - // TextEntryState.State.PICKED_SUGGESTION state. + final boolean showingAddToDictionaryHint = index == 0 && mCorrectionMode > 0 + && !mSuggest.isValidWord(suggestion) + && !mSuggest.isValidWord(suggestion.toString().toLowerCase()); + if (!correcting) { + // Fool the state watcher so that a subsequent backspace will not do a revert, unless + // we just did a correction, in which case we need to stay in + // TextEntryState.State.PICKED_SUGGESTION state. TextEntryState.typedCharacter((char) KEYCODE_SPACE, true); setNextSuggestions(); - } else { + } else if (!showingAddToDictionaryHint) { + // If we're not showing the "Tap again to save hint", then show corrections again. // In case the cursor position doesn't change, make sure we show the suggestions again. + clearSuggestions(); postUpdateOldSuggestions(); } - if (index == 0 && mCorrectionMode > 0 && !mSuggest.isValidWord(suggestion) - && !mSuggest.isValidWord(suggestion.toString().toLowerCase())) { + if (showingAddToDictionaryHint) { mCandidateView.showAddToDictionaryHint(suggestion); } if (ic != null) { @@ -1841,16 +1854,6 @@ public class LatinIME extends InputMethodService InputConnection ic = getCurrentInputConnection(); if (ic != null) { rememberReplacedWord(suggestion); - // If text is in correction mode and we're not using composing - // text to underline, then the word at the cursor position needs - // to be removed before committing the correction - if (correcting && !MODIFY_TEXT_FOR_CORRECTION) { - if (mLastSelectionStart < mLastSelectionEnd) { - ic.setSelection(mLastSelectionStart, mLastSelectionStart); - } - EditingUtil.deleteWordAtCursor(ic, getWordSeparators()); - } - ic.commitText(suggestion, 1); } saveWordInHistory(suggestion); @@ -1864,96 +1867,108 @@ public class LatinIME extends InputMethodService updateShiftKeyState(getCurrentInputEditorInfo()); } + /** + * Tries to apply any voice alternatives for the word if this was a spoken word and + * there are voice alternatives. + * @param touching The word that the cursor is touching, with position information + * @return true if an alternative was found, false otherwise. + */ + private boolean applyVoiceAlternatives(EditingUtil.SelectedWord touching) { + // Search for result in spoken word alternatives + String selectedWord = touching.word.toString().trim(); + if (!mWordToSuggestions.containsKey(selectedWord)) { + selectedWord = selectedWord.toLowerCase(); + } + if (mWordToSuggestions.containsKey(selectedWord)) { + mShowingVoiceSuggestions = true; + List suggestions = mWordToSuggestions.get(selectedWord); + // If the first letter of touching is capitalized, make all the suggestions + // start with a capital letter. + if (Character.isUpperCase((char) touching.word.charAt(0))) { + for (int i = 0; i < suggestions.size(); i++) { + String origSugg = (String) suggestions.get(i); + String capsSugg = origSugg.toUpperCase().charAt(0) + + origSugg.subSequence(1, origSugg.length()).toString(); + suggestions.set(i, capsSugg); + } + } + setSuggestions(suggestions, false, true, true); + setCandidatesViewShown(true); + return true; + } + return false; + } + + /** + * Tries to apply any typed alternatives for the word if we have any cached alternatives, + * otherwise tries to find new corrections and completions for the word. + * @param touching The word that the cursor is touching, with position information + * @return true if an alternative was found, false otherwise. + */ + private boolean applyTypedAlternatives(EditingUtil.SelectedWord touching) { + // If we didn't find a match, search for result in typed word history + WordComposer foundWord = null; + WordAlternatives alternatives = null; + for (WordAlternatives entry : mWordHistory) { + if (TextUtils.equals(entry.getChosenWord(), touching.word)) { + if (entry instanceof TypedWordAlternatives) { + foundWord = ((TypedWordAlternatives) entry).word; + } + alternatives = entry; + break; + } + } + // If we didn't find a match, at least suggest completions + if (foundWord == null + && (mSuggest.isValidWord(touching.word) + || mSuggest.isValidWord(touching.word.toString().toLowerCase()))) { + foundWord = new WordComposer(); + for (int i = 0; i < touching.word.length(); i++) { + foundWord.add(touching.word.charAt(i), new int[] { + touching.word.charAt(i) + }); + } + foundWord.setCapitalized(Character.isUpperCase(touching.word.charAt(0))); + } + // Found a match, show suggestions + if (foundWord != null || alternatives != null) { + if (alternatives == null) { + alternatives = new TypedWordAlternatives(touching.word, foundWord); + } + showCorrections(alternatives); + if (foundWord != null) { + mWord = new WordComposer(foundWord); + } else { + mWord.reset(); + } + return true; + } + return false; + } + private void setOldSuggestions() { - // TODO: Inefficient to check if touching word and then get the touching word. Do it - // in one go. mShowingVoiceSuggestions = false; + if (mCandidateView != null && mCandidateView.isShowingAddToDictionaryHint()) { + return; + } InputConnection ic = getCurrentInputConnection(); if (ic == null) return; - ic.beginBatchEdit(); - // If there is a selection, then undo the selection first. Unfortunately this causes - // a flicker. TODO: Add getSelectionText() to InputConnection API. - if (mLastSelectionStart < mLastSelectionEnd) { - ic.setSelection(mLastSelectionStart, mLastSelectionStart); - } - if (!mPredicting && isCursorTouchingWord()) { - EditingUtil.Range range = new EditingUtil.Range(); - CharSequence touching = EditingUtil.getWordAtCursor(getCurrentInputConnection(), - mWordSeparators, range); - // If it's a selection, check if it's an entire word and no more, no less. - boolean fullword = EditingUtil.isFullWordOrInside(range, mLastSelectionStart, - mLastSelectionEnd); - if (fullword && touching != null && touching.length() > 1) { - // Strip out any trailing word separator - if (mWordSeparators.indexOf(touching.charAt(touching.length() - 1)) > 0) { - touching = touching.toString().substring(0, touching.length() - 1); + if (!mPredicting) { + // Extract the selected or touching text + EditingUtil.SelectedWord touching = EditingUtil.getWordAtCursorOrSelection(ic, + mLastSelectionStart, mLastSelectionEnd, mWordSeparators); + + if (touching != null && touching.word.length() > 1) { + ic.beginBatchEdit(); + + if (!applyVoiceAlternatives(touching) && !applyTypedAlternatives(touching)) { + abortCorrection(true); + } else { + TextEntryState.selectedForCorrection(); + EditingUtil.underlineWord(ic, touching); } - // Search for result in spoken word alternatives - String selectedWord = touching.toString().trim(); - if (!mWordToSuggestions.containsKey(selectedWord)){ - selectedWord = selectedWord.toLowerCase(); - } - if (mWordToSuggestions.containsKey(selectedWord)){ - mShowingVoiceSuggestions = true; - underlineWord(touching, range.charsBefore, range.charsAfter); - List suggestions = mWordToSuggestions.get(selectedWord); - // If the first letter of touching is capitalized, make all the suggestions - // start with a capital letter. - if (Character.isUpperCase((char) touching.charAt(0))) { - for (int i=0; i< suggestions.size(); i++) { - String origSugg = (String) suggestions.get(i); - String capsSugg = origSugg.toUpperCase().charAt(0) - + origSugg.subSequence(1, origSugg.length()).toString(); - suggestions.set(i,capsSugg); - } - } - setSuggestions(suggestions, false, true, true); - setCandidatesViewShown(true); - TextEntryState.selectedForCorrection(); - ic.endBatchEdit(); - return; - } - - // If we didn't find a match, search for result in typed word history - WordComposer foundWord = null; - WordAlternatives alternatives = null; - for (WordAlternatives entry : mWordHistory) { - if (TextUtils.equals(entry.getChosenWord(), touching)) { - if (entry instanceof TypedWordAlternatives) { - foundWord = ((TypedWordAlternatives)entry).word; - } - alternatives = entry; - break; - } - } - // If we didn't find a match, at least suggest completions - if (foundWord == null && mSuggest.isValidWord(touching)) { - foundWord = new WordComposer(); - for (int i = 0; i < touching.length(); i++) { - foundWord.add(touching.charAt(i), new int[] { touching.charAt(i) }); - } - } - // Found a match, show suggestions - if (foundWord != null || alternatives != null) { - underlineWord(touching, range.charsBefore, range.charsAfter); - TextEntryState.selectedForCorrection(); - if (alternatives == null) alternatives = new TypedWordAlternatives(touching, - foundWord); - showCorrections(alternatives); - if (foundWord != null) { - mWord = new WordComposer(foundWord); - } else { - mWord.reset(); - } - // Revert the selection - if (mLastSelectionStart < mLastSelectionEnd) { - ic.setSelection(mLastSelectionStart, mLastSelectionEnd); - } - ic.endBatchEdit(); - return; - } - abortCorrection(true); + ic.endBatchEdit(); } else { abortCorrection(true); setNextSuggestions(); @@ -1961,28 +1976,12 @@ public class LatinIME extends InputMethodService } else { abortCorrection(true); } - // Revert the selection - if (mLastSelectionStart < mLastSelectionEnd) { - ic.setSelection(mLastSelectionStart, mLastSelectionEnd); - } - ic.endBatchEdit(); } private void setNextSuggestions() { setSuggestions(mSuggestPuncList, false, false, false); } - private void underlineWord(CharSequence word, int left, int right) { - InputConnection ic = getCurrentInputConnection(); - if (ic == null) return; - if (MODIFY_TEXT_FOR_CORRECTION) { - ic.finishComposingText(); - ic.deleteSurroundingText(left, right); - ic.setComposingText(word, 1); - } - ic.setSelection(mLastSelectionStart, mLastSelectionStart); - } - private void addToDictionaries(CharSequence suggestion, int frequencyDelta) { checkAddToDictionary(suggestion, frequencyDelta, false); }