From 490fa47a46015f2c8bd8f9010d236bfe5aecd4bb Mon Sep 17 00:00:00 2001 From: Yohei Yukaw Date: Fri, 17 Apr 2015 16:48:10 -0700 Subject: [PATCH] Always specify non-null Locale object to SuggestionSpan Confusingly, specifying a null Locale object to the constructor of SuggestionSpan does not necessarily mean that SuggestionSpan#getLocale() returns null. The constructor in question also receives Context object, and Context's locale can be used as a fallback locale to initialize locale of SuggestionSpan. With this CL, LatinIME always specify non-null Locale object when instantiating SuggestionSpan object. It basically corresponds to the active main dictionary, but can be Locale#ROOT when one locale is not determined for some reasons. BUG: 20435013 Change-Id: I2c152466410327300e7dba4d7ed9a22f57c17c4f --- .../compat/SuggestionSpanUtils.java | 11 ++--- .../latin/inputlogic/InputLogic.java | 18 +++++++- .../compat/SuggestionSpanUtilsTest.java | 46 ++++++++++++++++--- 3 files changed, 61 insertions(+), 14 deletions(-) diff --git a/java/src/com/android/inputmethod/compat/SuggestionSpanUtils.java b/java/src/com/android/inputmethod/compat/SuggestionSpanUtils.java index 4d2925d30..3f621913c 100644 --- a/java/src/com/android/inputmethod/compat/SuggestionSpanUtils.java +++ b/java/src/com/android/inputmethod/compat/SuggestionSpanUtils.java @@ -33,6 +33,7 @@ import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Locale; +import javax.annotation.Nonnull; import javax.annotation.Nullable; public final class SuggestionSpanUtils { @@ -57,13 +58,12 @@ public final class SuggestionSpanUtils { @UsedForTesting public static CharSequence getTextWithAutoCorrectionIndicatorUnderline( - final Context context, final String text) { + final Context context, final String text, @Nonnull final Locale locale) { if (TextUtils.isEmpty(text) || OBJ_FLAG_AUTO_CORRECTION == null) { return text; } final Spannable spannable = new SpannableString(text); - // TODO: Set locale if it is feasible. - final SuggestionSpan suggestionSpan = new SuggestionSpan(context, null /* locale */, + final SuggestionSpan suggestionSpan = new SuggestionSpan(context, locale, new String[] {} /* suggestions */, OBJ_FLAG_AUTO_CORRECTION, null); spannable.setSpan(suggestionSpan, 0, text.length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE | Spanned.SPAN_COMPOSING); @@ -72,7 +72,7 @@ public final class SuggestionSpanUtils { @UsedForTesting public static CharSequence getTextWithSuggestionSpan(final Context context, - final String pickedWord, final SuggestedWords suggestedWords) { + final String pickedWord, final SuggestedWords suggestedWords, final Locale locale) { if (TextUtils.isEmpty(pickedWord) || suggestedWords.isEmpty() || suggestedWords.isPrediction() || suggestedWords.isPunctuationSuggestions()) { return pickedWord; @@ -92,8 +92,7 @@ public final class SuggestionSpanUtils { suggestionsList.add(word.toString()); } } - // TODO: Set locale if it is feasible. - final SuggestionSpan suggestionSpan = new SuggestionSpan(context, null /* locale */, + final SuggestionSpan suggestionSpan = new SuggestionSpan(context, locale, suggestionsList.toArray(new String[suggestionsList.size()]), 0 /* flags */, null); final Spannable spannable = new SpannableString(pickedWord); spannable.setSpan(suggestionSpan, 0, pickedWord.length(), Spanned.SPAN_EXCLUSIVE_EXCLUSIVE); diff --git a/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java b/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java index 324ae3a19..f7dbc0a4d 100644 --- a/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java +++ b/java/src/com/android/inputmethod/latin/inputlogic/InputLogic.java @@ -60,6 +60,7 @@ import com.android.inputmethod.latin.utils.StatsUtils; import com.android.inputmethod.latin.utils.TextRange; import java.util.ArrayList; +import java.util.Locale; import java.util.TreeSet; import java.util.concurrent.TimeUnit; @@ -1902,6 +1903,15 @@ public final class InputLogic { SuggestedWords.NOT_A_SEQUENCE_NUMBER); } + /** + * @return the {@link Locale} of the {@link #mDictionaryFacilitator} if available. Otherwise + * {@link Locale#ROOT}. + */ + @Nonnull + private Locale getDictionaryFacilitatorLocale() { + return mDictionaryFacilitator != null ? mDictionaryFacilitator.getLocale() : Locale.ROOT; + } + /** * Gets a chunk of text with or the auto-correction indicator underline span as appropriate. * @@ -1921,8 +1931,10 @@ public final class InputLogic { */ // TODO: Shouldn't this go in some *Utils class instead? private CharSequence getTextWithUnderline(final String text) { + // TODO: Locale should be determined based on context and the text given. return mIsAutoCorrectionIndicatorOn - ? SuggestionSpanUtils.getTextWithAutoCorrectionIndicatorUnderline(mLatinIME, text) + ? SuggestionSpanUtils.getTextWithAutoCorrectionIndicatorUnderline( + mLatinIME, text, getDictionaryFacilitatorLocale()) : text; } @@ -2122,9 +2134,11 @@ public final class InputLogic { Log.d(TAG, "commitChosenWord() : [" + chosenWord + "]"); } final SuggestedWords suggestedWords = mSuggestedWords; + // TODO: Locale should be determined based on context and the text given. + final Locale locale = getDictionaryFacilitatorLocale(); final CharSequence chosenWordWithSuggestions = SuggestionSpanUtils.getTextWithSuggestionSpan(mLatinIME, chosenWord, - suggestedWords); + suggestedWords, locale); if (DebugFlags.DEBUG_ENABLED) { long runTimeMillis = System.currentTimeMillis() - startTimeMillis; Log.d(TAG, "commitChosenWord() : " + runTimeMillis + " ms to run " diff --git a/tests/src/com/android/inputmethod/compat/SuggestionSpanUtilsTest.java b/tests/src/com/android/inputmethod/compat/SuggestionSpanUtilsTest.java index daf412cc3..2d6d28f2b 100644 --- a/tests/src/com/android/inputmethod/compat/SuggestionSpanUtilsTest.java +++ b/tests/src/com/android/inputmethod/compat/SuggestionSpanUtilsTest.java @@ -31,6 +31,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Locale; +import javax.annotation.Nullable; + @SmallTest public class SuggestionSpanUtilsTest extends AndroidTestCase { @@ -62,7 +64,7 @@ public class SuggestionSpanUtilsTest extends AndroidTestCase { private static void assertSuggestionSpan(final String expectedText, final int reuiredSuggestionSpanFlags, final int requiredSpanFlags, - final String[] expectedSuggestions, + final String[] expectedSuggestions, @Nullable final Locale expectedLocale, final CharSequence actualText) { assertTrue(TextUtils.equals(expectedText, actualText)); assertTrue(actualText instanceof Spanned); @@ -84,22 +86,39 @@ public class SuggestionSpanUtilsTest extends AndroidTestCase { assertEquals(expectedSuggestions[i], actualSuggestions[i]); } } + // CAVEAT: SuggestionSpan#getLocale() returns String rather than Locale object. + assertEquals(expectedLocale.toString(), suggestionSpan.getLocale()); } @TargetApi(Build.VERSION_CODES.ICE_CREAM_SANDWICH_MR1) public void testGetTextWithAutoCorrectionIndicatorUnderline() { final String ORIGINAL_TEXT = "Hey!"; + final Locale NONNULL_LOCALE = new Locale("en", "GB"); final CharSequence text = SuggestionSpanUtils.getTextWithAutoCorrectionIndicatorUnderline( - getContext(), ORIGINAL_TEXT); + getContext(), ORIGINAL_TEXT, NONNULL_LOCALE); if (Build.VERSION.SDK_INT < Build.VERSION_CODES.ICE_CREAM_SANDWICH_MR1) { assertNotSuggestionSpan(ORIGINAL_TEXT, text); return; } - assertSuggestionSpan(ORIGINAL_TEXT, SuggestionSpan.FLAG_AUTO_CORRECTION /* reuiredSuggestionSpanFlags */, Spanned.SPAN_COMPOSING | Spanned.SPAN_EXCLUSIVE_EXCLUSIVE /* requiredSpanFlags */, - new String[]{}, text); + new String[]{}, NONNULL_LOCALE, text); + } + + @TargetApi(Build.VERSION_CODES.ICE_CREAM_SANDWICH_MR1) + public void testGetTextWithAutoCorrectionIndicatorUnderlineRootLocale() { + final String ORIGINAL_TEXT = "Hey!"; + final CharSequence text = SuggestionSpanUtils.getTextWithAutoCorrectionIndicatorUnderline( + getContext(), ORIGINAL_TEXT, Locale.ROOT); + if (Build.VERSION.SDK_INT < Build.VERSION_CODES.ICE_CREAM_SANDWICH_MR1) { + assertNotSuggestionSpan(ORIGINAL_TEXT, text); + return; + } + assertSuggestionSpan(ORIGINAL_TEXT, + SuggestionSpan.FLAG_AUTO_CORRECTION /* reuiredSuggestionSpanFlags */, + Spanned.SPAN_COMPOSING | Spanned.SPAN_EXCLUSIVE_EXCLUSIVE /* requiredSpanFlags */, + new String[]{}, Locale.ROOT, text); } public void testGetTextWithSuggestionSpan() { @@ -119,6 +138,8 @@ public class SuggestionSpanUtilsTest extends AndroidTestCase { corrections[i] = createWordInfo("correction" + i, SuggestedWordInfo.KIND_CORRECTION); } + final Locale NONNULL_LOCALE = new Locale("en", "GB"); + // SuggestionSpan will not be attached when {@link SuggestedWords#INPUT_STYLE_PREDICTION} // is specified. { @@ -132,10 +153,11 @@ public class SuggestionSpanUtilsTest extends AndroidTestCase { SuggestedWords.INPUT_STYLE_PREDICTION, SuggestedWords.NOT_A_SEQUENCE_NUMBER); final String PICKED_WORD = prediction2.mWord; + // Note that the framework uses the context locale as a fallback locale. assertNotSuggestionSpan( PICKED_WORD, SuggestionSpanUtils.getTextWithSuggestionSpan(getContext(), PICKED_WORD, - predictedWords)); + predictedWords, NONNULL_LOCALE)); } final ArrayList suggestedWordList = new ArrayList<>(); @@ -174,13 +196,25 @@ public class SuggestionSpanUtilsTest extends AndroidTestCase { expectedSuggestions.add(suggestedWord); } + // non-null locale assertSuggestionSpan( PICKED_WORD, 0 /* reuiredSuggestionSpanFlags */, Spanned.SPAN_EXCLUSIVE_EXCLUSIVE /* requiredSpanFlags */, expectedSuggestions.toArray(new String[expectedSuggestions.size()]), + NONNULL_LOCALE, SuggestionSpanUtils.getTextWithSuggestionSpan(getContext(), PICKED_WORD, - typedAndCollectedWords)); + typedAndCollectedWords, NONNULL_LOCALE)); + + // root locale + assertSuggestionSpan( + PICKED_WORD, + 0 /* reuiredSuggestionSpanFlags */, + Spanned.SPAN_EXCLUSIVE_EXCLUSIVE /* requiredSpanFlags */, + expectedSuggestions.toArray(new String[expectedSuggestions.size()]), + Locale.ROOT, + SuggestionSpanUtils.getTextWithSuggestionSpan(getContext(), PICKED_WORD, + typedAndCollectedWords, Locale.ROOT)); } }