From 42275bc8b32f342cad7778d28ade59eea12a983c Mon Sep 17 00:00:00 2001 From: Yohei Yukawa Date: Thu, 3 Mar 2016 00:34:27 -0800 Subject: [PATCH] Fix a regression in InputMethodUtils. It turns out that my previous CL [1] unexpectedly changed the behavior of InputMethodUtils#getImplicitlyApplicableSubtypesLocked() in terms of when "EnabledWhenDefaultIsNotAsciiCapable" extra value is taken into account. Suppose if an IME X has the following three subtypes: A. locale: en_US mode: handwriting extraValue: B. locale: hi mode: keyboard extraValue: C. locale: en_US mode: keyboard extraValue: AsciiCapable D. locale: zz mode: keyboard extraValue: AsciiCapable, EnabledWhenDefaultIsNotAsciiCapable Given the above subtypes, here are results of what subtypes are enabled by InputMethodUtils#getImplicitlyApplicableSubtypesLocked() I) before the CL [1] and II) after the CL [1]. - system language: hi: I: B, D II: B, D - system language: hi-IN: I: B, D II: B, D - system language: en-US I: A, C II: A, C - system language: en-GB I: A, C II: A, C - system language: ja-JP I: B II: D What my previous CL actually broke is the the last one, and it's broken because we accidentally started using "EnabledWhenDefaultIsNotAsciiCapable" even when there is no subtype that matches to the requested language. Previously that attribute was used if and only if 1) there is a subtype that matches the requested language and 2) that subtype is not marked to be AsciiCapable. If there there is no subtype that matches to the requested language, what we had relied on is actually the result of InputMethodUtils#findLastResortApplicableSubtypeLocked() called with canIgnoreLocaleAsLastResort = true, which means that we had just picked up the first keyboard subtype as the last fallback candidate regardless of it's locale. This is why the subtype B should be picked up in the last case where system language is ja-JP. This CL fixes the above unexpected behavior change regarding "EnabledWhenDefaultIsNotAsciiCapable" so that the previous behavior can be preserved. [1] Iaf179d60c12b9a98b4f097e2449471c4184e049b e985c240e3feb62ea38d5b4e386be083ca0f215b Bug: 27129703 Bug: 27425459 Change-Id: Icd86cad955bf827a1eb41255f57fdf4ec315b93b --- .../inputmethod/InputMethodUtils.java | 34 ++++++----- .../inputmethod/InputMethodUtilsTest.java | 58 +++++++++++++++++++ 2 files changed, 76 insertions(+), 16 deletions(-) diff --git a/core/java/com/android/internal/inputmethod/InputMethodUtils.java b/core/java/com/android/internal/inputmethod/InputMethodUtils.java index 4e48e45d018ec..f04bcf2d02640 100644 --- a/core/java/com/android/internal/inputmethod/InputMethodUtils.java +++ b/core/java/com/android/internal/inputmethod/InputMethodUtils.java @@ -547,23 +547,25 @@ public class InputMethodUtils { LocaleUtils.filterByLanguage(keyboardSubtypes, sSubtypeToLocale, systemLocales, applicableSubtypes); - boolean hasAsciiCapableKeyboard = false; - final int numApplicationSubtypes = applicableSubtypes.size(); - for (int i = 0; i < numApplicationSubtypes; ++i) { - final InputMethodSubtype subtype = applicableSubtypes.get(i); - if (subtype.containsExtraValueKey(TAG_ASCII_CAPABLE)) { - hasAsciiCapableKeyboard = true; - break; + if (!applicableSubtypes.isEmpty()) { + boolean hasAsciiCapableKeyboard = false; + final int numApplicationSubtypes = applicableSubtypes.size(); + for (int i = 0; i < numApplicationSubtypes; ++i) { + final InputMethodSubtype subtype = applicableSubtypes.get(i); + if (subtype.containsExtraValueKey(TAG_ASCII_CAPABLE)) { + hasAsciiCapableKeyboard = true; + break; + } } - } - if (!hasAsciiCapableKeyboard) { - final int numKeyboardSubtypes = keyboardSubtypes.size(); - for (int i = 0; i < numKeyboardSubtypes; ++i) { - final InputMethodSubtype subtype = keyboardSubtypes.get(i); - final String mode = subtype.getMode(); - if (SUBTYPE_MODE_KEYBOARD.equals(mode) && subtype.containsExtraValueKey( - TAG_ENABLED_WHEN_DEFAULT_IS_NOT_ASCII_CAPABLE)) { - applicableSubtypes.add(subtype); + if (!hasAsciiCapableKeyboard) { + final int numKeyboardSubtypes = keyboardSubtypes.size(); + for (int i = 0; i < numKeyboardSubtypes; ++i) { + final InputMethodSubtype subtype = keyboardSubtypes.get(i); + final String mode = subtype.getMode(); + if (SUBTYPE_MODE_KEYBOARD.equals(mode) && subtype.containsExtraValueKey( + TAG_ENABLED_WHEN_DEFAULT_IS_NOT_ASCII_CAPABLE)) { + applicableSubtypes.add(subtype); + } } } } diff --git a/core/tests/coretests/src/com/android/internal/inputmethod/InputMethodUtilsTest.java b/core/tests/coretests/src/com/android/internal/inputmethod/InputMethodUtilsTest.java index ac020e48355ac..719b274f80f98 100644 --- a/core/tests/coretests/src/com/android/internal/inputmethod/InputMethodUtilsTest.java +++ b/core/tests/coretests/src/com/android/internal/inputmethod/InputMethodUtilsTest.java @@ -70,6 +70,7 @@ public class InputMethodUtilsTest extends InstrumentationTestCase { private static final Locale LOCALE_TH_TH_TH = new Locale("ht", "TH", "TH"); private static final String SUBTYPE_MODE_KEYBOARD = "keyboard"; private static final String SUBTYPE_MODE_VOICE = "voice"; + private static final String SUBTYPE_MODE_HANDWRITING = "handwriting"; private static final String SUBTYPE_MODE_ANY = null; private static final String EXTRA_VALUE_PAIR_SEPARATOR = ","; private static final String EXTRA_VALUE_ASCII_CAPABLE = "AsciiCapable"; @@ -215,6 +216,12 @@ public class InputMethodUtilsTest extends InstrumentationTestCase { final InputMethodSubtype nonAutoJa = createDummyInputMethodSubtype("ja", SUBTYPE_MODE_KEYBOARD, !IS_AUX, !IS_OVERRIDES_IMPLICITLY_ENABLED_SUBTYPE, !IS_ASCII_CAPABLE, !IS_ENABLED_WHEN_DEFAULT_IS_NOT_ASCII_CAPABLE); + final InputMethodSubtype nonAutoHi = createDummyInputMethodSubtype("hi", + SUBTYPE_MODE_KEYBOARD, !IS_AUX, !IS_OVERRIDES_IMPLICITLY_ENABLED_SUBTYPE, + !IS_ASCII_CAPABLE, !IS_ENABLED_WHEN_DEFAULT_IS_NOT_ASCII_CAPABLE); + final InputMethodSubtype nonAutoHandwritingEn = createDummyInputMethodSubtype("en", + SUBTYPE_MODE_HANDWRITING, !IS_AUX, !IS_OVERRIDES_IMPLICITLY_ENABLED_SUBTYPE, + !IS_ASCII_CAPABLE, !IS_ENABLED_WHEN_DEFAULT_IS_NOT_ASCII_CAPABLE); final InputMethodSubtype nonAutoEnabledWhenDefaultIsNotAsciiCalableSubtype = createDummyInputMethodSubtype("zz", SUBTYPE_MODE_KEYBOARD, !IS_AUX, !IS_OVERRIDES_IMPLICITLY_ENABLED_SUBTYPE, !IS_ASCII_CAPABLE, @@ -349,6 +356,57 @@ public class InputMethodUtilsTest extends InstrumentationTestCase { verifyEquality(nonAutoEnabledWhenDefaultIsNotAsciiCalableSubtype2, result.get(2)); } + // Make sure that if there is no subtype that matches the language requested, then we just + // use the first keyboard subtype. + { + final ArrayList subtypes = new ArrayList<>(); + subtypes.add(nonAutoHi); + subtypes.add(nonAutoEnUS); + subtypes.add(nonAutoHandwritingEn); + subtypes.add(nonAutoEnabledWhenDefaultIsNotAsciiCalableSubtype); + final InputMethodInfo imi = createDummyInputMethodInfo( + "com.android.apps.inputmethod.latin", + "com.android.apps.inputmethod.latin", "DummyLatinIme", !IS_AUX, IS_DEFAULT, + subtypes); + final ArrayList result = + InputMethodUtils.getImplicitlyApplicableSubtypesLocked( + getResourcesForLocales(LOCALE_JA_JP), imi); + assertEquals(1, result.size()); + verifyEquality(nonAutoHi, result.get(0)); + } + { + final ArrayList subtypes = new ArrayList<>(); + subtypes.add(nonAutoEnUS); + subtypes.add(nonAutoHi); + subtypes.add(nonAutoHandwritingEn); + subtypes.add(nonAutoEnabledWhenDefaultIsNotAsciiCalableSubtype); + final InputMethodInfo imi = createDummyInputMethodInfo( + "com.android.apps.inputmethod.latin", + "com.android.apps.inputmethod.latin", "DummyLatinIme", !IS_AUX, IS_DEFAULT, + subtypes); + final ArrayList result = + InputMethodUtils.getImplicitlyApplicableSubtypesLocked( + getResourcesForLocales(LOCALE_JA_JP), imi); + assertEquals(1, result.size()); + verifyEquality(nonAutoEnUS, result.get(0)); + } + { + final ArrayList subtypes = new ArrayList<>(); + subtypes.add(nonAutoHandwritingEn); + subtypes.add(nonAutoEnUS); + subtypes.add(nonAutoHi); + subtypes.add(nonAutoEnabledWhenDefaultIsNotAsciiCalableSubtype); + final InputMethodInfo imi = createDummyInputMethodInfo( + "com.android.apps.inputmethod.latin", + "com.android.apps.inputmethod.latin", "DummyLatinIme", !IS_AUX, IS_DEFAULT, + subtypes); + final ArrayList result = + InputMethodUtils.getImplicitlyApplicableSubtypesLocked( + getResourcesForLocales(LOCALE_JA_JP), imi); + assertEquals(1, result.size()); + verifyEquality(nonAutoEnUS, result.get(0)); + } + // Make sure that 3-letter language code can be handled. { final ArrayList subtypes = new ArrayList<>();