From 3cba1dc1c58018111ec9bebf00387db3d4dd3ece Mon Sep 17 00:00:00 2001 From: menghanli Date: Wed, 18 Mar 2020 20:54:27 +0800 Subject: [PATCH] Refines volume key shortcut confirm dialog strings - Provide more clear content for single and multiple services are enabled. - Avoid non-a11y users accidentally turning shortcut on without reading this carefully. Hence we put "don't turn on" as the primary action. Bug: 155249323 Bug: 138582063 Test: atest AccessibilityShortcutControllerTest Change-Id: I1c391bb0516bcebfbf6161b94cc0c0b80e0bb72f --- .../AccessibilityShortcutController.java | 45 +++++++++++-- .../dialog/AccessibilityTarget.java | 2 +- .../dialog/AccessibilityTargetHelper.java | 66 +++++++++++++++---- .../util/AccessibilityUtils.java | 3 +- .../accessibility/util/ShortcutUtils.java | 3 +- core/res/res/values/strings.xml | 6 -- core/res/res/values/symbols.xml | 11 ++-- .../AccessibilityShortcutControllerTest.java | 28 +++++--- 8 files changed, 124 insertions(+), 40 deletions(-) diff --git a/core/java/com/android/internal/accessibility/AccessibilityShortcutController.java b/core/java/com/android/internal/accessibility/AccessibilityShortcutController.java index be66d0c238cc6..1aae92d02aa74 100644 --- a/core/java/com/android/internal/accessibility/AccessibilityShortcutController.java +++ b/core/java/com/android/internal/accessibility/AccessibilityShortcutController.java @@ -19,6 +19,7 @@ package com.android.internal.accessibility; import static android.view.WindowManager.LayoutParams.TYPE_KEYGUARD_DIALOG; import static android.view.accessibility.AccessibilityManager.ACCESSIBILITY_SHORTCUT_KEY; +import static com.android.internal.accessibility.dialog.AccessibilityTargetHelper.getTargets; import static com.android.internal.util.ArrayUtils.convertToLongArray; import android.accessibilityservice.AccessibilityServiceInfo; @@ -52,6 +53,7 @@ import android.view.accessibility.AccessibilityManager; import android.widget.Toast; import com.android.internal.R; +import com.android.internal.accessibility.dialog.AccessibilityTarget; import com.android.internal.util.function.pooled.PooledLambda; import java.lang.annotation.Retention; @@ -264,16 +266,21 @@ public class AccessibilityShortcutController { } private AlertDialog createShortcutWarningDialog(int userId) { - final String warningMessage = mContext.getString( - R.string.accessibility_shortcut_toogle_warning); + List targets = getTargets(mContext, ACCESSIBILITY_SHORTCUT_KEY); + if (targets.size() == 0) { + return null; + } + + // Avoid non-a11y users accidentally turning shortcut on without reading this carefully. + // Put "don't turn on" as the primary action. final AlertDialog alertDialog = mFrameworkObjectProvider.getAlertDialogBuilder( // Use SystemUI context so we pick up any theme set in a vendor overlay mFrameworkObjectProvider.getSystemUiContext()) - .setTitle(R.string.accessibility_shortcut_warning_dialog_title) - .setMessage(warningMessage) + .setTitle(getShortcutWarningTitle(targets)) + .setMessage(getShortcutWarningMessage(targets)) .setCancelable(false) - .setPositiveButton(R.string.leave_accessibility_shortcut_on, null) - .setNegativeButton(R.string.disable_accessibility_shortcut, + .setNegativeButton(R.string.accessibility_shortcut_on, null) + .setPositiveButton(R.string.accessibility_shortcut_off, (DialogInterface d, int which) -> { Settings.Secure.putStringForUser(mContext.getContentResolver(), Settings.Secure.ACCESSIBILITY_SHORTCUT_TARGET_SERVICE, "", @@ -294,6 +301,32 @@ public class AccessibilityShortcutController { return alertDialog; } + private String getShortcutWarningTitle(List targets) { + if (targets.size() == 1) { + return mContext.getString( + R.string.accessibility_shortcut_single_service_warning_title, + targets.get(0).getLabel()); + } + return mContext.getString( + R.string.accessibility_shortcut_multiple_service_warning_title); + } + + private String getShortcutWarningMessage(List targets) { + if (targets.size() == 1) { + return mContext.getString( + R.string.accessibility_shortcut_single_service_warning, + targets.get(0).getLabel()); + } + + final StringBuilder sb = new StringBuilder(); + for (AccessibilityTarget target : targets) { + sb.append(mContext.getString(R.string.accessibility_shortcut_multiple_service_list, + target.getLabel())); + } + return mContext.getString(R.string.accessibility_shortcut_multiple_service_warning, + sb.toString()); + } + private AccessibilityServiceInfo getInfoForTargetService() { final ComponentName targetComponentName = getShortcutTargetComponentName(); if (targetComponentName == null) { diff --git a/core/java/com/android/internal/accessibility/dialog/AccessibilityTarget.java b/core/java/com/android/internal/accessibility/dialog/AccessibilityTarget.java index 37871d0b5a104..d75659372a07e 100644 --- a/core/java/com/android/internal/accessibility/dialog/AccessibilityTarget.java +++ b/core/java/com/android/internal/accessibility/dialog/AccessibilityTarget.java @@ -38,7 +38,7 @@ import com.android.internal.accessibility.dialog.TargetAdapter.ViewHolder; * Abstract base class for creating various target related to accessibility service, * accessibility activity, and white listing feature. */ -abstract class AccessibilityTarget implements TargetOperations, OnTargetSelectedListener, +public abstract class AccessibilityTarget implements TargetOperations, OnTargetSelectedListener, OnTargetCheckedChangeListener { private Context mContext; @ShortcutType diff --git a/core/java/com/android/internal/accessibility/dialog/AccessibilityTargetHelper.java b/core/java/com/android/internal/accessibility/dialog/AccessibilityTargetHelper.java index f63cbe0dcd9e8..60a102adcf7ae 100644 --- a/core/java/com/android/internal/accessibility/dialog/AccessibilityTargetHelper.java +++ b/core/java/com/android/internal/accessibility/dialog/AccessibilityTargetHelper.java @@ -53,19 +53,61 @@ import java.util.Locale; /** * Collection of utilities for accessibility target. */ -final class AccessibilityTargetHelper { +public final class AccessibilityTargetHelper { private AccessibilityTargetHelper() {} - static List getTargets(Context context, + /** + * Returns list of {@link AccessibilityTarget} of assigned accessibility shortcuts from + * {@link AccessibilityManager#getAccessibilityShortcutTargets} including accessibility + * feature's package name, component id, etc. + * + * @param context The context of the application. + * @param shortcutType The shortcut type. + * @return The list of {@link AccessibilityTarget}. + * @hide + */ + public static List getTargets(Context context, @ShortcutType int shortcutType) { - final List targets = getInstalledTargets(context, shortcutType); - final AccessibilityManager ams = context.getSystemService(AccessibilityManager.class); - final List requiredTargets = ams.getAccessibilityShortcutTargets(shortcutType); - targets.removeIf(target -> !requiredTargets.contains(target.getId())); + // List all accessibility target + final List installedTargets = getInstalledTargets(context, + shortcutType); - return targets; + // List accessibility shortcut target + final AccessibilityManager am = (AccessibilityManager) context.getSystemService( + Context.ACCESSIBILITY_SERVICE); + final List assignedTargets = am.getAccessibilityShortcutTargets(shortcutType); + + // Get the list of accessibility shortcut target in all accessibility target + final List results = new ArrayList<>(); + for (String assignedTarget : assignedTargets) { + for (AccessibilityTarget installedTarget : installedTargets) { + if (!MAGNIFICATION_CONTROLLER_NAME.contentEquals(assignedTarget)) { + final ComponentName assignedTargetComponentName = + ComponentName.unflattenFromString(assignedTarget); + final ComponentName targetComponentName = ComponentName.unflattenFromString( + installedTarget.getId()); + if (assignedTargetComponentName.equals(targetComponentName)) { + results.add(installedTarget); + continue; + } + } + if (assignedTarget.contentEquals(installedTarget.getId())) { + results.add(installedTarget); + } + } + } + return results; } + /** + * Returns list of {@link AccessibilityTarget} of the installed accessibility service, + * accessibility activity, and white listing feature including accessibility feature's package + * name, component id, etc. + * + * @param context The context of the application. + * @param shortcutType The shortcut type. + * @return The list of {@link AccessibilityTarget}. + */ static List getInstalledTargets(Context context, @ShortcutType int shortcutType) { final List targets = new ArrayList<>(); @@ -110,9 +152,10 @@ final class AccessibilityTargetHelper { private static List getAccessibilityServiceTargets(Context context, @ShortcutType int shortcutType) { - final AccessibilityManager ams = context.getSystemService(AccessibilityManager.class); + final AccessibilityManager am = (AccessibilityManager) context.getSystemService( + Context.ACCESSIBILITY_SERVICE); final List installedServices = - ams.getInstalledAccessibilityServiceList(); + am.getInstalledAccessibilityServiceList(); if (installedServices == null) { return Collections.emptyList(); } @@ -136,9 +179,10 @@ final class AccessibilityTargetHelper { private static List getAccessibilityActivityTargets(Context context, @ShortcutType int shortcutType) { - final AccessibilityManager ams = context.getSystemService(AccessibilityManager.class); + final AccessibilityManager am = (AccessibilityManager) context.getSystemService( + Context.ACCESSIBILITY_SERVICE); final List installedServices = - ams.getInstalledAccessibilityShortcutListAsUser(context, + am.getInstalledAccessibilityShortcutListAsUser(context, ActivityManager.getCurrentUser()); if (installedServices == null) { return Collections.emptyList(); diff --git a/core/java/com/android/internal/accessibility/util/AccessibilityUtils.java b/core/java/com/android/internal/accessibility/util/AccessibilityUtils.java index e50b010d691ad..9ee0b0ea18910 100644 --- a/core/java/com/android/internal/accessibility/util/AccessibilityUtils.java +++ b/core/java/com/android/internal/accessibility/util/AccessibilityUtils.java @@ -142,7 +142,8 @@ public final class AccessibilityUtils { */ public static boolean isAccessibilityServiceEnabled(Context context, @NonNull String componentId) { - final AccessibilityManager am = context.getSystemService(AccessibilityManager.class); + final AccessibilityManager am = (AccessibilityManager) context.getSystemService( + Context.ACCESSIBILITY_SERVICE); final List enabledServices = am.getEnabledAccessibilityServiceList(AccessibilityServiceInfo.FEEDBACK_ALL_MASK); diff --git a/core/java/com/android/internal/accessibility/util/ShortcutUtils.java b/core/java/com/android/internal/accessibility/util/ShortcutUtils.java index 100422f5660df..31ccb6c32babd 100644 --- a/core/java/com/android/internal/accessibility/util/ShortcutUtils.java +++ b/core/java/com/android/internal/accessibility/util/ShortcutUtils.java @@ -137,7 +137,8 @@ public final class ShortcutUtils { */ public static boolean isShortcutContained(Context context, @ShortcutType int shortcutType, @NonNull String componentId) { - final AccessibilityManager am = context.getSystemService(AccessibilityManager.class); + final AccessibilityManager am = (AccessibilityManager) context.getSystemService( + Context.ACCESSIBILITY_SERVICE); final List requiredTargets = am.getAccessibilityShortcutTargets(shortcutType); return requiredTargets.contains(componentId); } diff --git a/core/res/res/values/strings.xml b/core/res/res/values/strings.xml index 35a7857b839f2..f3f3d47df4a7c 100644 --- a/core/res/res/values/strings.xml +++ b/core/res/res/values/strings.xml @@ -4390,12 +4390,6 @@ \t• %1$s\n - - Turn on TalkBack? - - - Holding down both volume keys for a few seconds turns on TalkBack, a screen reader that is helpful for people who are blind or have low vision. TalkBack completely changes how your device works.\n\nYou can change this shortcut to another feature in Settings > Accessibility. - Turn on %1$s? diff --git a/core/res/res/values/symbols.xml b/core/res/res/values/symbols.xml index 9a7f07f14cb1d..3b6beec5cbcd6 100644 --- a/core/res/res/values/symbols.xml +++ b/core/res/res/values/symbols.xml @@ -3217,12 +3217,15 @@ - - + + + + + + + - - diff --git a/core/tests/coretests/src/com/android/internal/accessibility/AccessibilityShortcutControllerTest.java b/core/tests/coretests/src/com/android/internal/accessibility/AccessibilityShortcutControllerTest.java index b21504c73772d..c17c36eba2dc5 100644 --- a/core/tests/coretests/src/com/android/internal/accessibility/AccessibilityShortcutControllerTest.java +++ b/core/tests/coretests/src/com/android/internal/accessibility/AccessibilityShortcutControllerTest.java @@ -93,6 +93,7 @@ import java.util.Set; @RunWith(AndroidJUnit4.class) public class AccessibilityShortcutControllerTest { private static final String SERVICE_NAME_STRING = "fake.package/fake.service.name"; + private static final CharSequence PACKAGE_NAME_STRING = "Service name"; private static final String SERVICE_NAME_SUMMARY = "Summary"; private static final long VIBRATOR_PATTERN_1 = 100L; private static final long VIBRATOR_PATTERN_2 = 150L; @@ -150,6 +151,8 @@ public class AccessibilityShortcutControllerTest { new AccessibilityManager(mHandler, mAccessibilityManagerService, 0); when(mFrameworkObjectProvider.getAccessibilityManagerInstance(mContext)) .thenReturn(accessibilityManager); + when(mContext.getSystemService(Context.ACCESSIBILITY_SERVICE)) + .thenReturn(accessibilityManager); when(mFrameworkObjectProvider.getAlertDialogBuilder(mContext)) .thenReturn(mAlertDialogBuilder); when(mFrameworkObjectProvider.makeToastFromText(eq(mContext), anyObject(), anyInt())) @@ -166,13 +169,13 @@ public class AccessibilityShortcutControllerTest { ResolveInfo resolveInfo = mock(ResolveInfo.class); resolveInfo.serviceInfo = mock(ServiceInfo.class); resolveInfo.serviceInfo.applicationInfo = mApplicationInfo; - when(resolveInfo.loadLabel(anyObject())).thenReturn("Service name"); + when(resolveInfo.loadLabel(anyObject())).thenReturn(PACKAGE_NAME_STRING); when(mServiceInfo.getResolveInfo()).thenReturn(resolveInfo); when(mServiceInfo.getComponentName()) .thenReturn(ComponentName.unflattenFromString(SERVICE_NAME_STRING)); when(mServiceInfo.loadSummary(any())).thenReturn(SERVICE_NAME_SUMMARY); - when(mAlertDialogBuilder.setTitle(anyInt())).thenReturn(mAlertDialogBuilder); + when(mAlertDialogBuilder.setTitle(anyObject())).thenReturn(mAlertDialogBuilder); when(mAlertDialogBuilder.setCancelable(anyBoolean())).thenReturn(mAlertDialogBuilder); when(mAlertDialogBuilder.setMessage(anyObject())).thenReturn(mAlertDialogBuilder); when(mAlertDialogBuilder.setPositiveButton(anyInt(), anyObject())) @@ -324,7 +327,8 @@ public class AccessibilityShortcutControllerTest { assertEquals(1, Settings.Secure.getInt( mContentResolver, ACCESSIBILITY_SHORTCUT_DIALOG_SHOWN, 0)); - verify(mResources).getString(R.string.accessibility_shortcut_toogle_warning); + verify(mResources).getString( + R.string.accessibility_shortcut_single_service_warning_title, PACKAGE_NAME_STRING); verify(mAlertDialog).show(); verify(mAccessibilityManagerService, atLeastOnce()).getInstalledAccessibilityServiceList( anyInt()); @@ -376,16 +380,20 @@ public class AccessibilityShortcutControllerTest { ArgumentCaptor captor = ArgumentCaptor.forClass(DialogInterface.OnClickListener.class); - verify(mAlertDialogBuilder).setNegativeButton(eq(R.string.disable_accessibility_shortcut), + verify(mAlertDialogBuilder).setPositiveButton(eq(R.string.accessibility_shortcut_off), captor.capture()); - // Call the button callback - captor.getValue().onClick(null, 0); + // Call the button callback, if one exists + if (captor.getValue() != null) { + captor.getValue().onClick(null, 0); + } assertTrue(TextUtils.isEmpty( Settings.Secure.getString(mContentResolver, ACCESSIBILITY_SHORTCUT_TARGET_SERVICE))); + assertEquals(0, Settings.Secure.getInt( + mContentResolver, ACCESSIBILITY_SHORTCUT_DIALOG_SHOWN)); } @Test - public void testClickingLeaveOnButtonInDialog_shouldLeaveShortcutReady() throws Exception { + public void testClickingTurnOnButtonInDialog_shouldLeaveShortcutReady() throws Exception { configureShortcutEnabled(ENABLED_EXCEPT_LOCK_SCREEN); configureValidShortcutService(); Settings.Secure.putInt(mContentResolver, ACCESSIBILITY_SHORTCUT_DIALOG_SHOWN, 0); @@ -393,8 +401,8 @@ public class AccessibilityShortcutControllerTest { ArgumentCaptor captor = ArgumentCaptor.forClass(DialogInterface.OnClickListener.class); - verify(mAlertDialogBuilder).setPositiveButton(eq(R.string.leave_accessibility_shortcut_on), - captor.capture()); + verify(mAlertDialogBuilder).setNegativeButton(eq(R.string.accessibility_shortcut_on), + captor.capture()); // Call the button callback, if one exists if (captor.getValue() != null) { captor.getValue().onClick(null, 0); @@ -402,7 +410,7 @@ public class AccessibilityShortcutControllerTest { assertEquals(SERVICE_NAME_STRING, Settings.Secure.getString(mContentResolver, ACCESSIBILITY_SHORTCUT_TARGET_SERVICE)); assertEquals(1, Settings.Secure.getInt( - mContentResolver, ACCESSIBILITY_SHORTCUT_DIALOG_SHOWN)); + mContentResolver, ACCESSIBILITY_SHORTCUT_DIALOG_SHOWN)); } @Test