From ce487188a07871f29350df61f76d00da8a7209b7 Mon Sep 17 00:00:00 2001 From: jasonwshsu Date: Mon, 7 Nov 2022 16:06:46 +0800 Subject: [PATCH] Fix TalkBack shortcut keeps showing enabled when entering into that page. Root Cause: TextUtils.isEmpty() will return true both null and empty case, and write config key into it. Solution: Write config key only if settings key is null. That means no one (developers or users) touch the key ever, so we should respect config key here. Bug: 255912415 Test: make RunSettingsRoboTests ROBOTEST_FILTER=ToggleFeaturePreferenceFragmentTest Change-Id: I092975ea3de8fce78b7ae2a6241a30f6af8125a6 --- .../ToggleFeaturePreferenceFragment.java | 50 ++++++++----------- .../ToggleFeaturePreferenceFragmentTest.java | 28 +++++++++-- 2 files changed, 45 insertions(+), 33 deletions(-) diff --git a/src/com/android/settings/accessibility/ToggleFeaturePreferenceFragment.java b/src/com/android/settings/accessibility/ToggleFeaturePreferenceFragment.java index facf213d9e5..4adb4d85855 100644 --- a/src/com/android/settings/accessibility/ToggleFeaturePreferenceFragment.java +++ b/src/com/android/settings/accessibility/ToggleFeaturePreferenceFragment.java @@ -34,7 +34,6 @@ import android.icu.text.CaseMap; import android.net.Uri; import android.os.Bundle; import android.os.Handler; -import android.os.UserHandle; import android.provider.Settings; import android.service.quicksettings.TileService; import android.text.Html; @@ -62,7 +61,6 @@ import com.android.settings.dashboard.DashboardFragment; import com.android.settings.utils.LocaleUtils; import com.android.settings.widget.SettingsMainSwitchBar; import com.android.settings.widget.SettingsMainSwitchPreference; -import com.android.settingslib.accessibility.AccessibilityUtils; import com.android.settingslib.widget.IllustrationPreference; import com.android.settingslib.widget.OnMainSwitchChangeListener; import com.android.settingslib.widget.TopIntroPreference; @@ -255,7 +253,7 @@ public abstract class ToggleFeaturePreferenceFragment extends DashboardFragment }); } - writeDefaultShortcutTargetServiceToSettingsIfNeeded(getPrefContext()); + writeConfigDefaultAccessibilityServiceIntoShortcutTargetServiceIfNeeded(getContext()); } @Override @@ -796,44 +794,36 @@ public abstract class ToggleFeaturePreferenceFragment extends DashboardFragment } /** - * Setups a configurable default if the setting has never been set. - * - * TODO(b/228562075): Remove this function when correcting the format in config file - * `config_defaultAccessibilityService`. + * Setups {@link com.android.internal.R.string#config_defaultAccessibilityService} into + * {@link Settings.Secure#ACCESSIBILITY_SHORTCUT_TARGET_SERVICE} if that settings key has never + * been set and only write the key when user enter into corresponding page. */ - private void writeDefaultShortcutTargetServiceToSettingsIfNeeded(Context context) { + @VisibleForTesting + void writeConfigDefaultAccessibilityServiceIntoShortcutTargetServiceIfNeeded(Context context) { if (mComponentName == null) { return; } - final ComponentName defaultService = ComponentName.unflattenFromString(context.getString( - com.android.internal.R.string.config_defaultAccessibilityService)); - // write default accessibility service only when user enter into corresponding page. - if (!mComponentName.equals(defaultService)) { + // It might be shortened form (with a leading '.'). Need to unflatten back to ComponentName + // first, or it will encounter errors when getting service from + // `ACCESSIBILITY_SHORTCUT_TARGET_SERVICE`. + final ComponentName configDefaultService = ComponentName.unflattenFromString( + getString(com.android.internal.R.string.config_defaultAccessibilityService)); + + if (!mComponentName.equals(configDefaultService)) { return; } final String targetKey = Settings.Secure.ACCESSIBILITY_SHORTCUT_TARGET_SERVICE; - String targetString = Settings.Secure.getString(context.getContentResolver(), targetKey); - if (!TextUtils.isEmpty(targetString)) { - // The shortcut setting has been set - return; - } + final String targetString = Settings.Secure.getString(context.getContentResolver(), + targetKey); - // AccessibilityManager#getAccessibilityShortcutTargets may not return correct shortcut - // targets during boot. Needs to read settings directly here. - targetString = AccessibilityUtils.getShortcutTargetServiceComponentNameString(context, - UserHandle.myUserId()); - if (TextUtils.isEmpty(targetString)) { - // No configurable default accessibility service - return; - } - - // Only fallback to default accessibility service when setting is never updated. - final ComponentName shortcutName = ComponentName.unflattenFromString(targetString); - if (shortcutName != null) { + // By intentional, we only need to write the config string when the Settings key has never + // been set (== null). Empty string also means someone already wrote it before, so we need + // to respect the value. + if (targetString == null) { Settings.Secure.putString(context.getContentResolver(), targetKey, - shortcutName.flattenToString()); + configDefaultService.flattenToString()); } } diff --git a/tests/robotests/src/com/android/settings/accessibility/ToggleFeaturePreferenceFragmentTest.java b/tests/robotests/src/com/android/settings/accessibility/ToggleFeaturePreferenceFragmentTest.java index a596d93b1e7..9e7ae5d9909 100644 --- a/tests/robotests/src/com/android/settings/accessibility/ToggleFeaturePreferenceFragmentTest.java +++ b/tests/robotests/src/com/android/settings/accessibility/ToggleFeaturePreferenceFragmentTest.java @@ -60,6 +60,7 @@ import org.junit.runner.RunWith; import org.mockito.Answers; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import org.mockito.Spy; import org.robolectric.RobolectricTestRunner; import org.robolectric.annotation.Config; import org.robolectric.shadow.api.Shadow; @@ -91,6 +92,7 @@ public class ToggleFeaturePreferenceFragmentTest { Settings.Secure.ACCESSIBILITY_SHORTCUT_TARGET_SERVICE; private TestToggleFeaturePreferenceFragment mFragment; + @Spy private final Context mContext = ApplicationProvider.getApplicationContext(); @Mock(answer = Answers.RETURNS_DEEP_STUBS) @@ -160,8 +162,10 @@ public class ToggleFeaturePreferenceFragmentTest { @Test public void updateShortcutPreferenceData_hasValueInSettings_assignToVariable() { mFragment.mComponentName = PLACEHOLDER_COMPONENT_NAME; - putStringIntoSettings(SOFTWARE_SHORTCUT_KEY, PLACEHOLDER_COMPONENT_NAME.flattenToString()); - putStringIntoSettings(HARDWARE_SHORTCUT_KEY, PLACEHOLDER_COMPONENT_NAME.flattenToString()); + putSecureStringIntoSettings(SOFTWARE_SHORTCUT_KEY, + PLACEHOLDER_COMPONENT_NAME.flattenToString()); + putSecureStringIntoSettings(HARDWARE_SHORTCUT_KEY, + PLACEHOLDER_COMPONENT_NAME.flattenToString()); mFragment.updateShortcutPreferenceData(); @@ -327,10 +331,28 @@ public class ToggleFeaturePreferenceFragmentTest { assertThat(accessibilityFooterPreference.getOrder()).isEqualTo(Integer.MAX_VALUE - 1); } - private void putStringIntoSettings(String key, String componentName) { + @Test + @Config(shadows = ShadowFragment.class) + public void writeConfigDefaultIfNeeded_sameCNWithFragAndConfig_SameValueInVolumeSettingsKey() { + mFragment.mComponentName = PLACEHOLDER_COMPONENT_NAME; + doReturn(PLACEHOLDER_COMPONENT_NAME.flattenToString()).when(mFragment).getString( + com.android.internal.R.string.config_defaultAccessibilityService); + + mFragment.writeConfigDefaultAccessibilityServiceIntoShortcutTargetServiceIfNeeded(mContext); + + assertThat( + getSecureStringFromSettings(Settings.Secure.ACCESSIBILITY_SHORTCUT_TARGET_SERVICE)) + .isEqualTo(PLACEHOLDER_COMPONENT_NAME.flattenToString()); + } + + private void putSecureStringIntoSettings(String key, String componentName) { Settings.Secure.putString(mContext.getContentResolver(), key, componentName); } + private String getSecureStringFromSettings(String key) { + return Settings.Secure.getString(mContext.getContentResolver(), key); + } + private void putUserShortcutTypeIntoSharedPreference(Context context, PreferredShortcut shortcut) { PreferredShortcuts.saveUserShortcutType(context, shortcut);