From 5551302d275e3f54da9d86bcea633556ad12db8e Mon Sep 17 00:00:00 2001 From: Dan Zivkovic Date: Wed, 25 Feb 2015 15:57:59 -0800 Subject: [PATCH] Don't assume that correctable words are invalid Currently, the Delight3DictionaryFacilitator sets a boolean flag when the top suggestion score exceeds the auto-correction threshold. This flag is used to trigger auto-correction of the typed word. Also, the existing logic assumes that if allowsToBeAutoCorrected then the word is invalid, which is no longer true after we stopped using whitelists. Bug 19518376. Change-Id: Ifa7f6a09c07d25ac68c6cf3aec91f358bd88689f --- .../latin/DictionaryFacilitatorImpl.java | 3 +- .../android/inputmethod/latin/Suggest.java | 14 ++++---- .../inputmethod/latin/SuggestedWords.java | 33 ++++++++++++++----- .../latin/utils/SuggestionResults.java | 15 ++++----- .../latin/SuggestedWordsTests.java | 32 ++++++++++++++++++ 5 files changed, 72 insertions(+), 25 deletions(-) diff --git a/java/src/com/android/inputmethod/latin/DictionaryFacilitatorImpl.java b/java/src/com/android/inputmethod/latin/DictionaryFacilitatorImpl.java index e96300020..19b0c35bd 100644 --- a/java/src/com/android/inputmethod/latin/DictionaryFacilitatorImpl.java +++ b/java/src/com/android/inputmethod/latin/DictionaryFacilitatorImpl.java @@ -673,7 +673,8 @@ public class DictionaryFacilitatorImpl implements DictionaryFacilitator { int inputStyle, KeyboardLayout keyboardLayout) { final DictionaryGroup[] dictionaryGroups = mDictionaryGroups; final SuggestionResults suggestionResults = new SuggestionResults( - SuggestedWords.MAX_SUGGESTIONS, ngramContext.isBeginningOfSentenceContext()); + SuggestedWords.MAX_SUGGESTIONS, ngramContext.isBeginningOfSentenceContext(), + false /* firstSuggestionExceedsConfidenceThreshold */); final float[] weightOfLangModelVsSpatialModel = new float[] { Dictionary.NOT_A_WEIGHT_OF_LANG_MODEL_VS_SPATIAL_MODEL }; for (final DictionaryGroup dictionaryGroup : dictionaryGroups) { diff --git a/java/src/com/android/inputmethod/latin/Suggest.java b/java/src/com/android/inputmethod/latin/Suggest.java index 832fcbcb0..dd4f51667 100644 --- a/java/src/com/android/inputmethod/latin/Suggest.java +++ b/java/src/com/android/inputmethod/latin/Suggest.java @@ -215,7 +215,8 @@ public final class Suggest { } } - SuggestedWordInfo.removeDups(typedWordString, suggestionsContainer); + final int firstOcurrenceOfTypedWordInSuggestions = + SuggestedWordInfo.removeDups(typedWordString, suggestionsContainer); final SuggestedWordInfo whitelistedWordInfo = getWhitelistedWordInfoOrNull(suggestionsContainer); @@ -278,7 +279,8 @@ public final class Suggest { hasAutoCorrection = false; } else { final SuggestedWordInfo firstSuggestion = suggestionResults.first(); - if (suggestionResults.mAutocorrectRecommendation) { + if (suggestionResults.mFirstSuggestionExceedsConfidenceThreshold + && firstOcurrenceOfTypedWordInSuggestions != 0) { hasAutoCorrection = true; } else if (!AutoCorrectionUtils.suggestionExceedsThreshold( firstSuggestion, consideredWord, mAutoCorrectionThreshold)) { @@ -319,12 +321,12 @@ public final class Suggest { } else { inputStyle = inputStyleIfNotPrediction; } + + final boolean isTypedWordValid = firstOcurrenceOfTypedWordInSuggestions > -1 + || (!resultsArePredictions && !allowsToBeAutoCorrected); callback.onGetSuggestedWords(new SuggestedWords(suggestionsList, suggestionResults.mRawSuggestions, typedWordInfo, - // TODO: this first argument is lying. If this is a whitelisted word which is an - // actual word, it says typedWordValid = false, which looks wrong. We should either - // rename the attribute or change the value. - !resultsArePredictions && !allowsToBeAutoCorrected /* typedWordValid */, + isTypedWordValid, hasAutoCorrection /* willAutoCorrect */, false /* isObsoleteSuggestions */, inputStyle, sequenceNumber)); } diff --git a/java/src/com/android/inputmethod/latin/SuggestedWords.java b/java/src/com/android/inputmethod/latin/SuggestedWords.java index 913b63a61..3816c0870 100644 --- a/java/src/com/android/inputmethod/latin/SuggestedWords.java +++ b/java/src/com/android/inputmethod/latin/SuggestedWords.java @@ -17,6 +17,7 @@ package com.android.inputmethod.latin; import android.text.TextUtils; +import android.util.Log; import android.view.inputmethod.CompletionInfo; import com.android.inputmethod.annotations.UsedForTesting; @@ -375,31 +376,45 @@ public class SuggestedWords { return mWord + " (" + mDebugString + ")"; } - // This will always remove the higher index if a duplicate is found. - public static void removeDups(@Nullable final String typedWord, - @Nonnull ArrayList candidates) { + /** + * This will always remove the higher index if a duplicate is found. + * + * @return position of typed word in the candidate list + */ + public static int removeDups( + @Nullable final String typedWord, + @Nonnull final ArrayList candidates) { if (candidates.isEmpty()) { - return; + return -1; } + int firstOccurrenceOfWord = -1; if (!TextUtils.isEmpty(typedWord)) { - removeSuggestedWordInfoFromList(typedWord, candidates, -1 /* startIndexExclusive */); + firstOccurrenceOfWord = removeSuggestedWordInfoFromList( + typedWord, candidates, -1 /* startIndexExclusive */); } for (int i = 0; i < candidates.size(); ++i) { - removeSuggestedWordInfoFromList(candidates.get(i).mWord, candidates, - i /* startIndexExclusive */); + removeSuggestedWordInfoFromList( + candidates.get(i).mWord, candidates, i /* startIndexExclusive */); } + return firstOccurrenceOfWord; } - private static void removeSuggestedWordInfoFromList( - @Nonnull final String word, @Nonnull final ArrayList candidates, + private static int removeSuggestedWordInfoFromList( + @Nonnull final String word, + @Nonnull final ArrayList candidates, final int startIndexExclusive) { + int firstOccurrenceOfWord = -1; for (int i = startIndexExclusive + 1; i < candidates.size(); ++i) { final SuggestedWordInfo previous = candidates.get(i); if (word.equals(previous.mWord)) { + if (firstOccurrenceOfWord == -1) { + firstOccurrenceOfWord = i; + } candidates.remove(i); --i; } } + return firstOccurrenceOfWord; } } diff --git a/java/src/com/android/inputmethod/latin/utils/SuggestionResults.java b/java/src/com/android/inputmethod/latin/utils/SuggestionResults.java index 10e3994b6..981355115 100644 --- a/java/src/com/android/inputmethod/latin/utils/SuggestionResults.java +++ b/java/src/com/android/inputmethod/latin/utils/SuggestionResults.java @@ -33,21 +33,18 @@ public final class SuggestionResults extends TreeSet { // TODO: Instead of a boolean , we may want to include the context of this suggestion results, // such as {@link NgramContext}. public final boolean mIsBeginningOfSentence; - public final boolean mAutocorrectRecommendation; + public final boolean mFirstSuggestionExceedsConfidenceThreshold; private final int mCapacity; - public SuggestionResults(final int capacity, final boolean isBeginningOfSentence) { - this(sSuggestedWordInfoComparator, capacity, isBeginningOfSentence, false); - } - public SuggestionResults(final int capacity, final boolean isBeginningOfSentence, - final boolean autocorrectRecommendation) { + final boolean firstSuggestionExceedsConfidenceThreshold) { this(sSuggestedWordInfoComparator, capacity, isBeginningOfSentence, - autocorrectRecommendation); + firstSuggestionExceedsConfidenceThreshold); } private SuggestionResults(final Comparator comparator, final int capacity, - final boolean isBeginningOfSentence, final boolean autocorrectRecommendation) { + final boolean isBeginningOfSentence, + final boolean firstSuggestionExceedsConfidenceThreshold) { super(comparator); mCapacity = capacity; if (ProductionFlags.INCLUDE_RAW_SUGGESTIONS) { @@ -56,7 +53,7 @@ public final class SuggestionResults extends TreeSet { mRawSuggestions = null; } mIsBeginningOfSentence = isBeginningOfSentence; - mAutocorrectRecommendation = autocorrectRecommendation; + mFirstSuggestionExceedsConfidenceThreshold = firstSuggestionExceedsConfidenceThreshold; } @Override diff --git a/tests/src/com/android/inputmethod/latin/SuggestedWordsTests.java b/tests/src/com/android/inputmethod/latin/SuggestedWordsTests.java index f658d726b..657cec8d7 100644 --- a/tests/src/com/android/inputmethod/latin/SuggestedWordsTests.java +++ b/tests/src/com/android/inputmethod/latin/SuggestedWordsTests.java @@ -59,6 +59,14 @@ public class SuggestedWordsTests extends AndroidTestCase { SuggestedWordInfo.NOT_A_CONFIDENCE /* autoCommitFirstWordConfidence */); } + private static ArrayList createCorrectionWordInfos(final String... words) { + final ArrayList infos = new ArrayList<>(); + for (final String word : words) { + infos.add(createCorrectionWordInfo(word)); + } + return infos; + } + // Helper for testGetTransformedWordInfo private static SuggestedWordInfo transformWordInfo(final String info, final int trailingSingleQuotesCount) { @@ -72,6 +80,30 @@ public class SuggestedWordsTests extends AndroidTestCase { return returnedWordInfo; } + public void testRemoveDupesNoDupes() { + final ArrayList infos = createCorrectionWordInfos("a", "c"); + assertEquals(-1, SuggestedWordInfo.removeDups("b", infos)); + assertEquals(2, infos.size()); + } + + public void testRemoveDupesTypedWordNotDupe() { + final ArrayList infos = createCorrectionWordInfos("a", "a", "c"); + assertEquals(-1, SuggestedWordInfo.removeDups("b", infos)); + assertEquals(2, infos.size()); + } + + public void testRemoveDupesTypedWordOnlyDupe() { + final ArrayList infos = createCorrectionWordInfos("a", "b", "c"); + assertEquals(1, SuggestedWordInfo.removeDups("b", infos)); + assertEquals(2, infos.size()); + } + + public void testRemoveDupesTypedWordNotOnlyDupe() { + final ArrayList infos = createCorrectionWordInfos("a", "b", "b", "c"); + assertEquals(1, SuggestedWordInfo.removeDups("b", infos)); + assertEquals(2, infos.size()); + } + public void testGetTransformedSuggestedWordInfo() { SuggestedWordInfo result = transformWordInfo("word", 0); assertEquals(result.mWord, "word");