From 267251aae38f5476fcc28da531230b430ee76082 Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Mon, 10 Feb 2025 05:50:02 +0000 Subject: [PATCH 1/3] feat(A11yFeedback): Add FeedbackManager for Accessibility page This change introduces a new FeedbackManager class to identify and determine the availability of user feedback. This API will be used to check if feedback is available for a user to provide. Bug: 393980229 Test: atest FeedbackManagerTest Flag: com.android.server.accessibility.enable_low_vision_generic_feedback Change-Id: I577aa626c18f5c449dc3970d1324cc4940d20639 --- .../accessibility/FeedbackManager.java | 95 +++++++++++++ .../accessibility/FeedbackManagerTest.java | 128 ++++++++++++++++++ 2 files changed, 223 insertions(+) create mode 100644 src/com/android/settings/accessibility/FeedbackManager.java create mode 100644 tests/robotests/src/com/android/settings/accessibility/FeedbackManagerTest.java diff --git a/src/com/android/settings/accessibility/FeedbackManager.java b/src/com/android/settings/accessibility/FeedbackManager.java new file mode 100644 index 00000000000..d765797e1db --- /dev/null +++ b/src/com/android/settings/accessibility/FeedbackManager.java @@ -0,0 +1,95 @@ +/* + * Copyright (C) 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.settings.accessibility; + +import android.app.Activity; +import android.content.Intent; +import android.text.TextUtils; + +import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; + +import com.android.server.accessibility.Flags; +import com.android.settingslib.DeviceInfoUtils; + +import java.lang.ref.WeakReference; + +/** + * Manages the feedback flow. This class is responsible for checking feedback availability and + * sending feedback. Uses a WeakReference to the Activity to prevent memory leaks. + */ +public class FeedbackManager { + + // TODO(b/393980229): Add a feature provider for Pixel overlay to expose the feedback bucket ID + static final String ACCESSIBILITY_FEEDBACK_REQUEST_BUCKET_ID = + "com.google.android.settings.intelligence.ACCESSIBILITY_FEEDBACK_REQUEST"; + static final String CATEGORY_TAG = "category_tag"; + private static final int FEEDBACK_INTENT_RESULT_CODE = 0; + + private final WeakReference mActivityWeakReference; + @Nullable private final String mFeedbackReporterPackage; + @Nullable private final String mCategoryTag; + + /** + * Constructs a new FeedbackManager. + * + * @param activity The activity context. A WeakReference is used to prevent memory leaks. + */ + public FeedbackManager(@Nullable Activity activity) { + this(activity, DeviceInfoUtils.getFeedbackReporterPackage(activity)); + } + + @VisibleForTesting + public FeedbackManager(@Nullable Activity activity, @Nullable String feedbackReporterPackage) { + this.mActivityWeakReference = new WeakReference<>(activity); + this.mFeedbackReporterPackage = feedbackReporterPackage; + this.mCategoryTag = ACCESSIBILITY_FEEDBACK_REQUEST_BUCKET_ID; + } + + /** + * Checks if feedback is available on the device. + * + * @return {@code true} if feedback is available, {@code false} otherwise. + */ + public boolean isAvailable() { + if (!Flags.enableLowVisionGenericFeedback()) { + return false; + } + + return !TextUtils.isEmpty(mFeedbackReporterPackage) && mActivityWeakReference.get() != null; + } + + /** + * Sends feedback using the available feedback reporter. This will start the feedback + * activity. It is the responsibility of the calling activity to handle the result + * code {@link #FEEDBACK_INTENT_RESULT_CODE} if necessary. + * + * @return {@code true} if the feedback intent was successfully started, {@code false} + * otherwise. + */ + public boolean sendFeedback() { + Activity activity = mActivityWeakReference.get(); + if (!isAvailable() || activity == null) { + return false; + } + + final Intent intent = new Intent(Intent.ACTION_BUG_REPORT); + intent.setPackage(mFeedbackReporterPackage); + intent.putExtra(CATEGORY_TAG, mCategoryTag); + activity.startActivityForResult(intent, FEEDBACK_INTENT_RESULT_CODE); + return true; + } +} diff --git a/tests/robotests/src/com/android/settings/accessibility/FeedbackManagerTest.java b/tests/robotests/src/com/android/settings/accessibility/FeedbackManagerTest.java new file mode 100644 index 00000000000..aca144e2465 --- /dev/null +++ b/tests/robotests/src/com/android/settings/accessibility/FeedbackManagerTest.java @@ -0,0 +1,128 @@ +/* + * Copyright (C) 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.settings.accessibility; + +import static com.android.settings.accessibility.FeedbackManager.ACCESSIBILITY_FEEDBACK_REQUEST_BUCKET_ID; +import static com.android.settings.accessibility.FeedbackManager.CATEGORY_TAG; + +import static com.google.common.truth.Truth.assertThat; + +import android.app.Activity; +import android.content.Intent; +import android.os.Bundle; +import android.platform.test.annotations.DisableFlags; +import android.platform.test.annotations.EnableFlags; +import android.platform.test.flag.junit.SetFlagsRule; + +import com.android.server.accessibility.Flags; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.robolectric.Robolectric; +import org.robolectric.RobolectricTestRunner; +import org.robolectric.Shadows; + +/** Tests for {@link FeedbackManager}. */ +@RunWith(RobolectricTestRunner.class) +public class FeedbackManagerTest { + + @Rule + public final SetFlagsRule mSetFlagsRule = new SetFlagsRule(); + + private static final String FEEDBACK_PACKAGE = "test.feedback.package"; + + private Activity mActivity; + + @Before + public void setUp() { + mActivity = Robolectric.buildActivity(Activity.class).create().get(); + } + + @Test + @EnableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) + public void isAvailable_enableLowVisionGenericFeedbackWithPackageAndActivity_returnsTrue() { + FeedbackManager feedbackManager = new FeedbackManager(mActivity, FEEDBACK_PACKAGE); + + assertThat(feedbackManager.isAvailable()).isTrue(); + } + + @Test + @DisableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) + public void isAvailable_disableLowVisionGenericFeedback_returnsFalse() { + FeedbackManager feedbackManager = new FeedbackManager(mActivity, FEEDBACK_PACKAGE); + + assertThat(feedbackManager.isAvailable()).isFalse(); + } + + @Test + @EnableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) + public void isAvailable_withNullPackage_returnsFalse() { + FeedbackManager feedbackManager = new FeedbackManager(mActivity, null); + + assertThat(feedbackManager.isAvailable()).isFalse(); + } + + @Test + @EnableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) + public void isAvailable_withNullActivity_returnsFalse() { + FeedbackManager feedbackManager = new FeedbackManager(null, FEEDBACK_PACKAGE); + + assertThat(feedbackManager.isAvailable()).isFalse(); + } + + @Test + @EnableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) + public void sendFeedback_enableLowVisionGenericFeedbackWithPackageAndActivity_success() { + FeedbackManager feedbackManager = new FeedbackManager(mActivity, FEEDBACK_PACKAGE); + + assertThat(feedbackManager.sendFeedback()).isTrue(); + + Intent startedIntent = Shadows.shadowOf(mActivity).getNextStartedActivity(); + assertThat(startedIntent).isNotNull(); + assertThat(startedIntent.getAction()).isEqualTo(Intent.ACTION_BUG_REPORT); + assertThat(startedIntent.getPackage()).isEqualTo(FEEDBACK_PACKAGE); + Bundle extras = startedIntent.getExtras(); + assertThat(extras).isNotNull(); + assertThat(extras.getString(CATEGORY_TAG)).isEqualTo( + ACCESSIBILITY_FEEDBACK_REQUEST_BUCKET_ID); + } + + @Test + @DisableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) + public void sendFeedback_disableLowVisionGenericFeedback_returnsFalse() { + FeedbackManager feedbackManager = new FeedbackManager(mActivity, FEEDBACK_PACKAGE); + + assertThat(feedbackManager.sendFeedback()).isFalse(); + } + + @Test + @EnableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) + public void sendFeedback_withNullPackage_returnsFalse() { + FeedbackManager feedbackManager = new FeedbackManager(mActivity, null); + + assertThat(feedbackManager.sendFeedback()).isFalse(); + } + + @Test + @EnableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) + public void sendFeedback_withNullActivity_returnsFalse() { + FeedbackManager feedbackManager = new FeedbackManager(null, FEEDBACK_PACKAGE); + + assertThat(feedbackManager.sendFeedback()).isFalse(); + } +} From c86e2f347244290d4f982780064ddf08ef6869a4 Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Thu, 13 Feb 2025 09:08:30 +0000 Subject: [PATCH 2/3] feat(A11yFeedback): Add feedback entry for Accessibility page This entry point allows users to access in the action bar. Visibility is controlled by the aconfig and FeedbackManager#isAvailable Bug: 393981463 Test: atest AccessibilitySettingsTest Flag: com.android.server.accessibility.enable_low_vision_generic_feedback Change-Id: I8c219b8220b5839121d14959fe526e6200afeecb --- res/values/strings.xml | 2 + .../accessibility/AccessibilitySettings.java | 40 ++++++++++- .../AccessibilitySettingsTest.java | 71 ++++++++++++++++++- 3 files changed, 110 insertions(+), 3 deletions(-) diff --git a/res/values/strings.xml b/res/values/strings.xml index 39e0dbdf4e8..c6ed89a22f9 100644 --- a/res/values/strings.xml +++ b/res/values/strings.xml @@ -5157,6 +5157,8 @@ Help improve by taking a survey No surveys available + + Send feedback Downloaded apps diff --git a/src/com/android/settings/accessibility/AccessibilitySettings.java b/src/com/android/settings/accessibility/AccessibilitySettings.java index c0341cc5949..57eb4d5fba4 100644 --- a/src/com/android/settings/accessibility/AccessibilitySettings.java +++ b/src/com/android/settings/accessibility/AccessibilitySettings.java @@ -30,6 +30,9 @@ import android.os.UserHandle; import android.provider.Settings; import android.text.TextUtils; import android.util.ArrayMap; +import android.view.Menu; +import android.view.MenuInflater; +import android.view.MenuItem; import android.view.accessibility.AccessibilityManager; import androidx.annotation.NonNull; @@ -101,6 +104,8 @@ public class AccessibilitySettings extends DashboardFragment implements // presentation. private static final long DELAY_UPDATE_SERVICES_MILLIS = 1000; + static final int MENU_ID_SEND_FEEDBACK = 0; + private final Handler mHandler = new Handler(); private final Runnable mUpdateRunnable = new Runnable() { @@ -143,8 +148,9 @@ public class AccessibilitySettings extends DashboardFragment implements } }; - @VisibleForTesting - AccessibilitySettingsContentObserver mSettingsContentObserver; + private AccessibilitySettingsContentObserver mSettingsContentObserver; + + private FeedbackManager mFeedbackManager; private final Map mCategoryToPrefCategoryMap = new ArrayMap<>(); @@ -245,6 +251,24 @@ public class AccessibilitySettings extends DashboardFragment implements super.onDestroy(); } + @Override + public void onCreateOptionsMenu(@NonNull Menu menu, @NonNull MenuInflater inflater) { + if (getFeedbackManager().isAvailable()) { + menu.add(Menu.NONE, MENU_ID_SEND_FEEDBACK, Menu.NONE, + getPrefContext().getText(R.string.accessibility_send_feedback_title)); + } + super.onCreateOptionsMenu(menu, inflater); + } + + @Override + public boolean onOptionsItemSelected(@NonNull MenuItem item) { + if (item.getItemId() == MENU_ID_SEND_FEEDBACK) { + getFeedbackManager().sendFeedback(); + return true; + } + return super.onOptionsItemSelected(item); + } + @Override protected int getPreferenceScreenResId() { return R.xml.accessibility_settings; @@ -255,6 +279,18 @@ public class AccessibilitySettings extends DashboardFragment implements return TAG; } + @VisibleForTesting + void setFeedbackManager(FeedbackManager feedbackManager) { + this.mFeedbackManager = feedbackManager; + } + + private FeedbackManager getFeedbackManager() { + if (mFeedbackManager == null) { + mFeedbackManager = new FeedbackManager(getActivity()); + } + return mFeedbackManager; + } + /** * Returns the summary for the current state of this accessibilityService. * diff --git a/tests/robotests/src/com/android/settings/accessibility/AccessibilitySettingsTest.java b/tests/robotests/src/com/android/settings/accessibility/AccessibilitySettingsTest.java index 91d7d91c833..13168658055 100644 --- a/tests/robotests/src/com/android/settings/accessibility/AccessibilitySettingsTest.java +++ b/tests/robotests/src/com/android/settings/accessibility/AccessibilitySettingsTest.java @@ -21,7 +21,11 @@ import static com.android.internal.accessibility.common.ShortcutConstants.UserSh import static com.google.common.truth.Truth.assertThat; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.robolectric.Shadows.shadowOf; @@ -43,6 +47,8 @@ import android.platform.test.annotations.DisableFlags; import android.platform.test.annotations.EnableFlags; import android.platform.test.flag.junit.SetFlagsRule; import android.provider.Settings; +import android.view.Menu; +import android.view.MenuItem; import android.view.accessibility.AccessibilityManager; import androidx.fragment.app.Fragment; @@ -113,7 +119,8 @@ public class AccessibilitySettingsTest { @Rule public final MockitoRule mocks = MockitoJUnit.rule(); - @Rule public final SetFlagsRule mSetFlagsRule = new SetFlagsRule(); + @Rule + public final SetFlagsRule mSetFlagsRule = new SetFlagsRule(); private final Context mContext = ApplicationProvider.getApplicationContext(); @Spy private final AccessibilityServiceInfo mServiceInfo = getMockAccessibilityServiceInfo( @@ -121,7 +128,13 @@ public class AccessibilitySettingsTest { private ShadowAccessibilityManager mShadowAccessibilityManager; @Mock private LocalBluetoothManager mLocalBluetoothManager; + @Mock + private Menu mMenu; + @Mock + private MenuItem mMenuItem; + private ActivityController mActivityController; + private AccessibilitySettings mFragment; @Before @@ -438,6 +451,62 @@ public class AccessibilitySettingsTest { } + @Test + @EnableFlags(com.android.server.accessibility.Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) + public void onCreateOptionsMenu_enableLowVisionGenericFeedback_shouldAddSendFeedbackMenu() { + setupFragment(); + mFragment.setFeedbackManager(new FeedbackManager(mFragment.getActivity(), PACKAGE_NAME)); + when(mMenu.add(anyInt(), anyInt(), anyInt(), anyInt())).thenReturn(mMenuItem); + + mFragment.onCreateOptionsMenu(mMenu, /* inflater= */ null); + + verify(mMenu).add(anyInt(), eq(AccessibilitySettings.MENU_ID_SEND_FEEDBACK), + anyInt(), eq(mContext.getText(R.string.accessibility_send_feedback_title))); + } + + @Test + @DisableFlags(com.android.server.accessibility.Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) + public void onCreateOptionsMenu_disableLowVisionGenericFeedback_shouldNotAddSendFeedbackMenu() { + setupFragment(); + mFragment.setFeedbackManager(new FeedbackManager(mFragment.getActivity(), PACKAGE_NAME)); + when(mMenu.add(anyInt(), anyInt(), anyInt(), anyInt())).thenReturn(mMenuItem); + + mFragment.onCreateOptionsMenu(mMenu, /* inflater= */ null); + + verify(mMenu, never()).add(anyInt(), eq(AccessibilitySettings.MENU_ID_SEND_FEEDBACK), + anyInt(), eq(mContext.getText(R.string.accessibility_send_feedback_title))); + } + + @Test + @EnableFlags(com.android.server.accessibility.Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) + public void onOptionsItemSelected_enableLowVisionGenericFeedback_shouldStartSendFeedback() { + setupFragment(); + mFragment.setFeedbackManager(new FeedbackManager(mFragment.getActivity(), PACKAGE_NAME)); + when(mMenu.add(anyInt(), anyInt(), anyInt(), anyInt())).thenReturn(mMenuItem); + mFragment.onCreateOptionsMenu(mMenu, /* inflater= */ null); + when(mMenuItem.getItemId()).thenReturn(AccessibilitySettings.MENU_ID_SEND_FEEDBACK); + + mFragment.onOptionsItemSelected(mMenuItem); + + Intent startedIntent = shadowOf(mFragment.getActivity()).getNextStartedActivity(); + assertThat(startedIntent).isNotNull(); + } + + @Test + @DisableFlags(com.android.server.accessibility.Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) + public void onOptionsItemSelected_disableLowVisionGenericFeedback_shouldNotStartSendFeedback() { + setupFragment(); + mFragment.setFeedbackManager(new FeedbackManager(mFragment.getActivity(), PACKAGE_NAME)); + when(mMenu.add(anyInt(), anyInt(), anyInt(), anyInt())).thenReturn(mMenuItem); + mFragment.onCreateOptionsMenu(mMenu, /* inflater= */ null); + when(mMenuItem.getItemId()).thenReturn(AccessibilitySettings.MENU_ID_SEND_FEEDBACK); + + mFragment.onOptionsItemSelected(mMenuItem); + + Intent startedIntent = shadowOf(mFragment.getActivity()).getNextStartedActivity(); + assertThat(startedIntent).isNull(); + } + @Test public void testAccessibilityMenuInSystem_IncludedInInteractionControl() { mShadowAccessibilityManager.setInstalledAccessibilityServiceList( From 6b916e6f2959beece531ae74a54e99648829c467 Mon Sep 17 00:00:00 2001 From: Menghan Li Date: Fri, 14 Feb 2025 03:01:41 +0000 Subject: [PATCH 3/3] feat(A11yFeedback): Pixel overlay to expose the feedback bucket ID This change introduces a feature provider for Pixel overlays, allowing customization of the feedback bucket ID on Android. Bug: 393980229 Test: Manual testing for Pixel and non-Pixel overlay in real device Test: atest AccessibilitySettingsTest FeedbackManagerTest Flag: com.android.server.accessibility.enable_low_vision_generic_feedback Change-Id: Ieeb1dba4de5c13a275e66781621cbfcca7219a5e --- .../AccessibilityFeedbackFeatureProvider.java | 35 +++++++++++ ...essibilityFeedbackFeatureProviderImpl.java | 31 ++++++++++ .../accessibility/FeedbackManager.java | 43 ++++++++++---- .../settings/overlay/FeatureFactory.kt | 6 ++ .../settings/overlay/FeatureFactoryImpl.kt | 5 ++ .../AccessibilitySettingsTest.java | 13 ++-- .../accessibility/FeedbackManagerTest.java | 59 +++++++++++++------ .../testutils/FakeFeatureFactory.java | 8 ++- .../settings/testutils/FakeFeatureFactory.kt | 3 + .../testutils/FakeFeatureFactory.java | 7 +++ 10 files changed, 178 insertions(+), 32 deletions(-) create mode 100644 src/com/android/settings/accessibility/AccessibilityFeedbackFeatureProvider.java create mode 100644 src/com/android/settings/accessibility/AccessibilityFeedbackFeatureProviderImpl.java diff --git a/src/com/android/settings/accessibility/AccessibilityFeedbackFeatureProvider.java b/src/com/android/settings/accessibility/AccessibilityFeedbackFeatureProvider.java new file mode 100644 index 00000000000..018bd2e2ec1 --- /dev/null +++ b/src/com/android/settings/accessibility/AccessibilityFeedbackFeatureProvider.java @@ -0,0 +1,35 @@ +/* + * Copyright (C) 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.settings.accessibility; + +import android.content.ComponentName; + +import androidx.annotation.Nullable; + +/** + * Provider for Accessibility feedback related features. + */ +public interface AccessibilityFeedbackFeatureProvider { + + /** + * Returns value according to the {@code componentName}. + * + * @param componentName the component name of the downloaded service or activity + * @return Feedback bucket ID + */ + @Nullable + String getCategory(@Nullable ComponentName componentName); +} diff --git a/src/com/android/settings/accessibility/AccessibilityFeedbackFeatureProviderImpl.java b/src/com/android/settings/accessibility/AccessibilityFeedbackFeatureProviderImpl.java new file mode 100644 index 00000000000..917c5f64146 --- /dev/null +++ b/src/com/android/settings/accessibility/AccessibilityFeedbackFeatureProviderImpl.java @@ -0,0 +1,31 @@ +/* + * Copyright (C) 2025 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.settings.accessibility; + +import android.content.ComponentName; + +import androidx.annotation.Nullable; + +/** Default implementation of {@link AccessibilityFeedbackFeatureProvider}. */ +public class AccessibilityFeedbackFeatureProviderImpl implements + AccessibilityFeedbackFeatureProvider{ + + @Override + @Nullable + public String getCategory(@Nullable ComponentName componentName) { + return ""; + } +} diff --git a/src/com/android/settings/accessibility/FeedbackManager.java b/src/com/android/settings/accessibility/FeedbackManager.java index d765797e1db..52aefd22d31 100644 --- a/src/com/android/settings/accessibility/FeedbackManager.java +++ b/src/com/android/settings/accessibility/FeedbackManager.java @@ -16,6 +16,7 @@ package com.android.settings.accessibility; import android.app.Activity; +import android.content.ComponentName; import android.content.Intent; import android.text.TextUtils; @@ -23,6 +24,7 @@ import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; import com.android.server.accessibility.Flags; +import com.android.settings.overlay.FeatureFactory; import com.android.settingslib.DeviceInfoUtils; import java.lang.ref.WeakReference; @@ -33,14 +35,11 @@ import java.lang.ref.WeakReference; */ public class FeedbackManager { - // TODO(b/393980229): Add a feature provider for Pixel overlay to expose the feedback bucket ID - static final String ACCESSIBILITY_FEEDBACK_REQUEST_BUCKET_ID = - "com.google.android.settings.intelligence.ACCESSIBILITY_FEEDBACK_REQUEST"; static final String CATEGORY_TAG = "category_tag"; private static final int FEEDBACK_INTENT_RESULT_CODE = 0; private final WeakReference mActivityWeakReference; - @Nullable private final String mFeedbackReporterPackage; + @Nullable private final String mReporterPackage; @Nullable private final String mCategoryTag; /** @@ -49,14 +48,36 @@ public class FeedbackManager { * @param activity The activity context. A WeakReference is used to prevent memory leaks. */ public FeedbackManager(@Nullable Activity activity) { - this(activity, DeviceInfoUtils.getFeedbackReporterPackage(activity)); + this(activity, /* componentName= */ null); } + /** + * Constructs a new FeedbackManager. + * + * @param activity The activity context. A WeakReference is used to prevent memory leaks. + * @param componentName The component name associated with the feedback. + */ + public FeedbackManager(@Nullable Activity activity, @Nullable ComponentName componentName) { + this(activity, + DeviceInfoUtils.getFeedbackReporterPackage(activity), + FeatureFactory.getFeatureFactory() + .getAccessibilityFeedbackFeatureProvider() + .getCategory(componentName)); + } + + /** + * Constructs a new FeedbackManager. This constructor is visible for testing. + * + * @param activity The activity context. A WeakReference is used to prevent memory leaks. + * @param reporterPackage The package name of the feedback reporter. + * @param category The feedback bucket ID. + */ @VisibleForTesting - public FeedbackManager(@Nullable Activity activity, @Nullable String feedbackReporterPackage) { + public FeedbackManager(@Nullable Activity activity, @Nullable String reporterPackage, + @Nullable String category) { this.mActivityWeakReference = new WeakReference<>(activity); - this.mFeedbackReporterPackage = feedbackReporterPackage; - this.mCategoryTag = ACCESSIBILITY_FEEDBACK_REQUEST_BUCKET_ID; + this.mReporterPackage = reporterPackage; + this.mCategoryTag = category; } /** @@ -69,7 +90,9 @@ public class FeedbackManager { return false; } - return !TextUtils.isEmpty(mFeedbackReporterPackage) && mActivityWeakReference.get() != null; + return !TextUtils.isEmpty(mReporterPackage) + && !TextUtils.isEmpty(mCategoryTag) + && mActivityWeakReference.get() != null; } /** @@ -87,7 +110,7 @@ public class FeedbackManager { } final Intent intent = new Intent(Intent.ACTION_BUG_REPORT); - intent.setPackage(mFeedbackReporterPackage); + intent.setPackage(mReporterPackage); intent.putExtra(CATEGORY_TAG, mCategoryTag); activity.startActivityForResult(intent, FEEDBACK_INTENT_RESULT_CODE); return true; diff --git a/src/com/android/settings/overlay/FeatureFactory.kt b/src/com/android/settings/overlay/FeatureFactory.kt index 6a419dd5fe0..46aa19b0d05 100644 --- a/src/com/android/settings/overlay/FeatureFactory.kt +++ b/src/com/android/settings/overlay/FeatureFactory.kt @@ -16,6 +16,7 @@ package com.android.settings.overlay import android.content.Context +import com.android.settings.accessibility.AccessibilityFeedbackFeatureProvider import com.android.settings.accessibility.AccessibilityMetricsFeatureProvider import com.android.settings.accessibility.AccessibilitySearchFeatureProvider import com.android.settings.accounts.AccountFeatureProvider @@ -133,6 +134,11 @@ abstract class FeatureFactory { */ abstract val securitySettingsFeatureProvider: SecuritySettingsFeatureProvider + /** + * Retrieves implementation for Accessibility feedback category feature. + */ + abstract val accessibilityFeedbackFeatureProvider: AccessibilityFeedbackFeatureProvider + /** * Retrieves implementation for Accessibility search index feature. */ diff --git a/src/com/android/settings/overlay/FeatureFactoryImpl.kt b/src/com/android/settings/overlay/FeatureFactoryImpl.kt index 22d3f1b6a4d..08abf2bd466 100644 --- a/src/com/android/settings/overlay/FeatureFactoryImpl.kt +++ b/src/com/android/settings/overlay/FeatureFactoryImpl.kt @@ -20,6 +20,8 @@ import android.content.Context import android.net.ConnectivityManager import android.net.VpnManager import android.os.UserManager +import com.android.settings.accessibility.AccessibilityFeedbackFeatureProvider +import com.android.settings.accessibility.AccessibilityFeedbackFeatureProviderImpl import com.android.settings.accessibility.AccessibilityMetricsFeatureProvider import com.android.settings.accessibility.AccessibilityMetricsFeatureProviderImpl import com.android.settings.accessibility.AccessibilitySearchFeatureProvider @@ -165,6 +167,9 @@ open class FeatureFactoryImpl : FeatureFactory() { SecuritySettingsFeatureProviderImpl() } + override val accessibilityFeedbackFeatureProvider: AccessibilityFeedbackFeatureProvider + by lazy { AccessibilityFeedbackFeatureProviderImpl() } + override val accessibilitySearchFeatureProvider: AccessibilitySearchFeatureProvider by lazy { AccessibilitySearchFeatureProviderImpl() } diff --git a/tests/robotests/src/com/android/settings/accessibility/AccessibilitySettingsTest.java b/tests/robotests/src/com/android/settings/accessibility/AccessibilitySettingsTest.java index 13168658055..e590a80b27d 100644 --- a/tests/robotests/src/com/android/settings/accessibility/AccessibilitySettingsTest.java +++ b/tests/robotests/src/com/android/settings/accessibility/AccessibilitySettingsTest.java @@ -113,6 +113,7 @@ public class AccessibilitySettingsTest { private static final String EMPTY_STRING = ""; private static final String DEFAULT_SUMMARY = "default summary"; private static final String DEFAULT_DESCRIPTION = "default description"; + private static final String DEFAULT_CATEGORY = "default category"; private static final String DEFAULT_LABEL = "default label"; private static final Boolean SERVICE_ENABLED = true; private static final Boolean SERVICE_DISABLED = false; @@ -455,7 +456,8 @@ public class AccessibilitySettingsTest { @EnableFlags(com.android.server.accessibility.Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) public void onCreateOptionsMenu_enableLowVisionGenericFeedback_shouldAddSendFeedbackMenu() { setupFragment(); - mFragment.setFeedbackManager(new FeedbackManager(mFragment.getActivity(), PACKAGE_NAME)); + mFragment.setFeedbackManager( + new FeedbackManager(mFragment.getActivity(), PACKAGE_NAME, DEFAULT_CATEGORY)); when(mMenu.add(anyInt(), anyInt(), anyInt(), anyInt())).thenReturn(mMenuItem); mFragment.onCreateOptionsMenu(mMenu, /* inflater= */ null); @@ -468,7 +470,8 @@ public class AccessibilitySettingsTest { @DisableFlags(com.android.server.accessibility.Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) public void onCreateOptionsMenu_disableLowVisionGenericFeedback_shouldNotAddSendFeedbackMenu() { setupFragment(); - mFragment.setFeedbackManager(new FeedbackManager(mFragment.getActivity(), PACKAGE_NAME)); + mFragment.setFeedbackManager( + new FeedbackManager(mFragment.getActivity(), PACKAGE_NAME, DEFAULT_CATEGORY)); when(mMenu.add(anyInt(), anyInt(), anyInt(), anyInt())).thenReturn(mMenuItem); mFragment.onCreateOptionsMenu(mMenu, /* inflater= */ null); @@ -481,7 +484,8 @@ public class AccessibilitySettingsTest { @EnableFlags(com.android.server.accessibility.Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) public void onOptionsItemSelected_enableLowVisionGenericFeedback_shouldStartSendFeedback() { setupFragment(); - mFragment.setFeedbackManager(new FeedbackManager(mFragment.getActivity(), PACKAGE_NAME)); + mFragment.setFeedbackManager( + new FeedbackManager(mFragment.getActivity(), PACKAGE_NAME, DEFAULT_CATEGORY)); when(mMenu.add(anyInt(), anyInt(), anyInt(), anyInt())).thenReturn(mMenuItem); mFragment.onCreateOptionsMenu(mMenu, /* inflater= */ null); when(mMenuItem.getItemId()).thenReturn(AccessibilitySettings.MENU_ID_SEND_FEEDBACK); @@ -496,7 +500,8 @@ public class AccessibilitySettingsTest { @DisableFlags(com.android.server.accessibility.Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) public void onOptionsItemSelected_disableLowVisionGenericFeedback_shouldNotStartSendFeedback() { setupFragment(); - mFragment.setFeedbackManager(new FeedbackManager(mFragment.getActivity(), PACKAGE_NAME)); + mFragment.setFeedbackManager( + new FeedbackManager(mFragment.getActivity(), PACKAGE_NAME, DEFAULT_CATEGORY)); when(mMenu.add(anyInt(), anyInt(), anyInt(), anyInt())).thenReturn(mMenuItem); mFragment.onCreateOptionsMenu(mMenu, /* inflater= */ null); when(mMenuItem.getItemId()).thenReturn(AccessibilitySettings.MENU_ID_SEND_FEEDBACK); diff --git a/tests/robotests/src/com/android/settings/accessibility/FeedbackManagerTest.java b/tests/robotests/src/com/android/settings/accessibility/FeedbackManagerTest.java index aca144e2465..299c020991f 100644 --- a/tests/robotests/src/com/android/settings/accessibility/FeedbackManagerTest.java +++ b/tests/robotests/src/com/android/settings/accessibility/FeedbackManagerTest.java @@ -15,7 +15,6 @@ */ package com.android.settings.accessibility; -import static com.android.settings.accessibility.FeedbackManager.ACCESSIBILITY_FEEDBACK_REQUEST_BUCKET_ID; import static com.android.settings.accessibility.FeedbackManager.CATEGORY_TAG; import static com.google.common.truth.Truth.assertThat; @@ -44,7 +43,8 @@ public class FeedbackManagerTest { @Rule public final SetFlagsRule mSetFlagsRule = new SetFlagsRule(); - private static final String FEEDBACK_PACKAGE = "test.feedback.package"; + private static final String PACKAGE_NAME = "test.feedback.package"; + private static final String DEFAULT_CATEGORY = "default category"; private Activity mActivity; @@ -55,8 +55,9 @@ public class FeedbackManagerTest { @Test @EnableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) - public void isAvailable_enableLowVisionGenericFeedbackWithPackageAndActivity_returnsTrue() { - FeedbackManager feedbackManager = new FeedbackManager(mActivity, FEEDBACK_PACKAGE); + public void isAvailable_enableLowVisionGenericFeedbackWithValidParams_returnsTrue() { + FeedbackManager feedbackManager = + new FeedbackManager(mActivity, PACKAGE_NAME, DEFAULT_CATEGORY); assertThat(feedbackManager.isAvailable()).isTrue(); } @@ -64,15 +65,26 @@ public class FeedbackManagerTest { @Test @DisableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) public void isAvailable_disableLowVisionGenericFeedback_returnsFalse() { - FeedbackManager feedbackManager = new FeedbackManager(mActivity, FEEDBACK_PACKAGE); + FeedbackManager feedbackManager = + new FeedbackManager(mActivity, PACKAGE_NAME, DEFAULT_CATEGORY); assertThat(feedbackManager.isAvailable()).isFalse(); } @Test @EnableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) - public void isAvailable_withNullPackage_returnsFalse() { - FeedbackManager feedbackManager = new FeedbackManager(mActivity, null); + public void isAvailable_withNullCategory_returnsFalse() { + FeedbackManager feedbackManager = + new FeedbackManager(mActivity, PACKAGE_NAME, /* category= */ null); + + assertThat(feedbackManager.isAvailable()).isFalse(); + } + + @Test + @EnableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) + public void isAvailable_withNullReporterPackage_returnsFalse() { + FeedbackManager feedbackManager = + new FeedbackManager(mActivity, /* reporterPackage= */ null, DEFAULT_CATEGORY); assertThat(feedbackManager.isAvailable()).isFalse(); } @@ -80,40 +92,52 @@ public class FeedbackManagerTest { @Test @EnableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) public void isAvailable_withNullActivity_returnsFalse() { - FeedbackManager feedbackManager = new FeedbackManager(null, FEEDBACK_PACKAGE); + FeedbackManager feedbackManager = + new FeedbackManager(/* activity= */ null, PACKAGE_NAME, DEFAULT_CATEGORY); assertThat(feedbackManager.isAvailable()).isFalse(); } @Test @EnableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) - public void sendFeedback_enableLowVisionGenericFeedbackWithPackageAndActivity_success() { - FeedbackManager feedbackManager = new FeedbackManager(mActivity, FEEDBACK_PACKAGE); + public void sendFeedback_enableLowVisionGenericFeedbackWithValidParams_success() { + FeedbackManager feedbackManager = + new FeedbackManager(mActivity, PACKAGE_NAME, DEFAULT_CATEGORY); assertThat(feedbackManager.sendFeedback()).isTrue(); Intent startedIntent = Shadows.shadowOf(mActivity).getNextStartedActivity(); assertThat(startedIntent).isNotNull(); assertThat(startedIntent.getAction()).isEqualTo(Intent.ACTION_BUG_REPORT); - assertThat(startedIntent.getPackage()).isEqualTo(FEEDBACK_PACKAGE); + assertThat(startedIntent.getPackage()).isEqualTo(PACKAGE_NAME); Bundle extras = startedIntent.getExtras(); assertThat(extras).isNotNull(); - assertThat(extras.getString(CATEGORY_TAG)).isEqualTo( - ACCESSIBILITY_FEEDBACK_REQUEST_BUCKET_ID); + assertThat(extras.getString(CATEGORY_TAG)).isEqualTo(DEFAULT_CATEGORY); } @Test @DisableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) public void sendFeedback_disableLowVisionGenericFeedback_returnsFalse() { - FeedbackManager feedbackManager = new FeedbackManager(mActivity, FEEDBACK_PACKAGE); + FeedbackManager feedbackManager = + new FeedbackManager(mActivity, PACKAGE_NAME, DEFAULT_CATEGORY); assertThat(feedbackManager.sendFeedback()).isFalse(); } @Test @EnableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) - public void sendFeedback_withNullPackage_returnsFalse() { - FeedbackManager feedbackManager = new FeedbackManager(mActivity, null); + public void sendFeedback_withNullCategory_returnsFalse() { + FeedbackManager feedbackManager = + new FeedbackManager(mActivity, PACKAGE_NAME, /* category= */ null); + + assertThat(feedbackManager.sendFeedback()).isFalse(); + } + + @Test + @EnableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) + public void sendFeedback_withNullReporterPackage_returnsFalse() { + FeedbackManager feedbackManager = + new FeedbackManager(mActivity, /* reporterPackage= */ null, DEFAULT_CATEGORY); assertThat(feedbackManager.sendFeedback()).isFalse(); } @@ -121,7 +145,8 @@ public class FeedbackManagerTest { @Test @EnableFlags(Flags.FLAG_ENABLE_LOW_VISION_GENERIC_FEEDBACK) public void sendFeedback_withNullActivity_returnsFalse() { - FeedbackManager feedbackManager = new FeedbackManager(null, FEEDBACK_PACKAGE); + FeedbackManager feedbackManager = + new FeedbackManager(/* activity= */ null, PACKAGE_NAME, DEFAULT_CATEGORY); assertThat(feedbackManager.sendFeedback()).isFalse(); } diff --git a/tests/robotests/testutils/com/android/settings/testutils/FakeFeatureFactory.java b/tests/robotests/testutils/com/android/settings/testutils/FakeFeatureFactory.java index cfa6357affd..c5d4c36228a 100644 --- a/tests/robotests/testutils/com/android/settings/testutils/FakeFeatureFactory.java +++ b/tests/robotests/testutils/com/android/settings/testutils/FakeFeatureFactory.java @@ -19,6 +19,7 @@ import static org.mockito.Mockito.mock; import android.content.Context; +import com.android.settings.accessibility.AccessibilityFeedbackFeatureProvider; import com.android.settings.accessibility.AccessibilityMetricsFeatureProvider; import com.android.settings.accessibility.AccessibilitySearchFeatureProvider; import com.android.settings.accounts.AccountFeatureProvider; @@ -103,6 +104,7 @@ public class FakeFeatureFactory extends FeatureFactory { public PrivateSpaceLoginFeatureProvider mPrivateSpaceLoginFeatureProvider; public DisplayFeatureProvider mDisplayFeatureProvider; public SyncAcrossDevicesFeatureProvider mSyncAcrossDevicesFeatureProvider; + public AccessibilityFeedbackFeatureProvider mAccessibilityFeedbackFeatureProvider; /** * Call this in {@code @Before} method of the test class to use fake factory. @@ -340,5 +342,9 @@ public class FakeFeatureFactory extends FeatureFactory { public SyncAcrossDevicesFeatureProvider getSyncAcrossDevicesFeatureProvider() { return mSyncAcrossDevicesFeatureProvider; } -} + @Override + public AccessibilityFeedbackFeatureProvider getAccessibilityFeedbackFeatureProvider() { + return mAccessibilityFeedbackFeatureProvider; + } +} diff --git a/tests/spa_unit/src/com/android/settings/testutils/FakeFeatureFactory.kt b/tests/spa_unit/src/com/android/settings/testutils/FakeFeatureFactory.kt index 37a6abe88f1..56dd444b474 100644 --- a/tests/spa_unit/src/com/android/settings/testutils/FakeFeatureFactory.kt +++ b/tests/spa_unit/src/com/android/settings/testutils/FakeFeatureFactory.kt @@ -17,6 +17,7 @@ package com.android.settings.testutils import android.content.Context +import com.android.settings.accessibility.AccessibilityFeedbackFeatureProvider import com.android.settings.accessibility.AccessibilityMetricsFeatureProvider import com.android.settings.accessibility.AccessibilitySearchFeatureProvider import com.android.settings.accounts.AccountFeatureProvider @@ -125,6 +126,8 @@ class FakeFeatureFactory : FeatureFactory() { get() = TODO("Not yet implemented") override val securitySettingsFeatureProvider: SecuritySettingsFeatureProvider get() = TODO("Not yet implemented") + override val accessibilityFeedbackFeatureProvider: AccessibilityFeedbackFeatureProvider + get() = TODO("Not yet implemented") override val accessibilitySearchFeatureProvider: AccessibilitySearchFeatureProvider get() = TODO("Not yet implemented") override val accessibilityMetricsFeatureProvider: AccessibilityMetricsFeatureProvider diff --git a/tests/unit/src/com/android/settings/testutils/FakeFeatureFactory.java b/tests/unit/src/com/android/settings/testutils/FakeFeatureFactory.java index 46316d8626d..d77d7a4ff01 100644 --- a/tests/unit/src/com/android/settings/testutils/FakeFeatureFactory.java +++ b/tests/unit/src/com/android/settings/testutils/FakeFeatureFactory.java @@ -19,6 +19,7 @@ import static org.mockito.Mockito.mock; import android.content.Context; +import com.android.settings.accessibility.AccessibilityFeedbackFeatureProvider; import com.android.settings.accessibility.AccessibilityMetricsFeatureProvider; import com.android.settings.accessibility.AccessibilitySearchFeatureProvider; import com.android.settings.accounts.AccountFeatureProvider; @@ -102,6 +103,7 @@ public class FakeFeatureFactory extends FeatureFactory { public PrivateSpaceLoginFeatureProvider mPrivateSpaceLoginFeatureProvider; public DisplayFeatureProvider mDisplayFeatureProvider; public SyncAcrossDevicesFeatureProvider mSyncAcrossDevicesFeatureProvider; + public AccessibilityFeedbackFeatureProvider mAccessibilityFeedbackFeatureProvider; /** Call this in {@code @Before} method of the test class to use fake factory. */ public static FakeFeatureFactory setupForTest() { @@ -341,4 +343,9 @@ public class FakeFeatureFactory extends FeatureFactory { public SyncAcrossDevicesFeatureProvider getSyncAcrossDevicesFeatureProvider() { return mSyncAcrossDevicesFeatureProvider; } + + @Override + public AccessibilityFeedbackFeatureProvider getAccessibilityFeedbackFeatureProvider() { + return mAccessibilityFeedbackFeatureProvider; + } }