diff options
| author | 2024-04-07 15:02:23 -0400 | |
|---|---|---|
| committer | 2024-04-16 17:52:58 +0000 | |
| commit | 65e73e304c3c35c46503098139fadebc0224e333 (patch) | |
| tree | 34a3b6aa81d6bb65da93401106ada480a47f0a6b | |
| parent | afe50c04c1ee12d054f1d5db8c8c580da71ec9a0 (diff) | |
[Mobile] Wrap AccessibilityContentDescriptions in a static getter
The content descriptions are mapped to level by their position in the
PHONE_SIGNAL_STRENGTH array, and thus are susceptible to out of bounds
index exceptions.
This change adds accessor methods to AccessibilityContentDescriptions
so that they can be checked (it's the responsibility of the caller to
check against a 0 return value which will throw if trying to get the
resource).
We also make the content description nullable and handle the resid=0
case.
Test: MobileIconViewModelTest
Bug: 328547883
Flag: NONE
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:6e67f7d49206325e9f0fd0189b8b3a1c5a2094a8)
Merged-In: Ifa55ad8184a5337390d9fa245311b4e9513c0233
Change-Id: Ifa55ad8184a5337390d9fa245311b4e9513c0233
4 files changed, 111 insertions, 8 deletions
diff --git a/packages/SettingsLib/res/values/strings.xml b/packages/SettingsLib/res/values/strings.xml index a4bc235aaa7a..d6c4c8bdab2e 100644 --- a/packages/SettingsLib/res/values/strings.xml +++ b/packages/SettingsLib/res/values/strings.xml @@ -1645,6 +1645,8 @@ <string name="accessibility_phone_two_bars">Phone two bars.</string> <!-- Content description of the phone signal when it is three bars for accessibility (not shown on the screen). [CHAR LIMIT=NONE] --> <string name="accessibility_phone_three_bars">Phone three bars.</string> + <!-- Content description of the phone signal when it is four bars for accessibility (not shown on the screen). [CHAR LIMIT=NONE] --> + <string name="accessibility_phone_four_bars">Phone four bars.</string> <!-- Content description of the phone signal when it is full for accessibility (not shown on the screen). [CHAR LIMIT=NONE] --> <string name="accessibility_phone_signal_full">Phone signal full.</string> diff --git a/packages/SettingsLib/src/com/android/settingslib/AccessibilityContentDescriptions.java b/packages/SettingsLib/src/com/android/settingslib/AccessibilityContentDescriptions.java index ce466dfbf19c..d79d0ba57e96 100644 --- a/packages/SettingsLib/src/com/android/settingslib/AccessibilityContentDescriptions.java +++ b/packages/SettingsLib/src/com/android/settingslib/AccessibilityContentDescriptions.java @@ -33,6 +33,59 @@ public class AccessibilityContentDescriptions { R.string.accessibility_phone_signal_full }; + /** + * @param level int in range [0-4] that describes the signal level + * @return the appropriate content description for that signal strength, or 0 if the param is + * invalid + */ + public static int getDescriptionForLevel(int level) { + if (level > 4 || level < 0) { + return 0; + } + + return PHONE_SIGNAL_STRENGTH[level]; + } + + public static final int[] PHONE_SIGNAL_STRENGTH_INFLATED = { + PHONE_SIGNAL_STRENGTH_NONE, + R.string.accessibility_phone_one_bar, + R.string.accessibility_phone_two_bars, + R.string.accessibility_phone_three_bars, + R.string.accessibility_phone_four_bars, + R.string.accessibility_phone_signal_full + }; + + /** + * @param level int in range [0-5] that describes the inflated signal level + * @return the appropriate content description for that signal strength, or 0 if the param is + * invalid + */ + public static int getDescriptionForInflatedLevel(int level) { + if (level > 5 || level < 0) { + return 0; + } + + return PHONE_SIGNAL_STRENGTH_INFLATED[level]; + } + + /** + * @param level int in range [0-5] that describes the inflated signal level + * @param numberOfLevels one of (5, 6) that describes the default number of levels, or the + * inflated number of levels. The level param should be relative to the + * number of levels. This won't do any inflation. + * @return the appropriate content description for that signal strength, or 0 if the param is + * invalid + */ + public static int getDescriptionForLevel(int level, int numberOfLevels) { + if (numberOfLevels == 5) { + return getDescriptionForLevel(level); + } else if (numberOfLevels == 6) { + return getDescriptionForInflatedLevel(level); + } else { + return 0; + } + } + public static final int[] DATA_CONNECTION_STRENGTH = { R.string.accessibility_no_data, R.string.accessibility_data_one_bar, diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt index eda5c44b5c2f..103b0e3a6f27 100644 --- a/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModel.kt @@ -16,7 +16,7 @@ package com.android.systemui.statusbar.pipeline.mobile.ui.viewmodel -import com.android.settingslib.AccessibilityContentDescriptions.PHONE_SIGNAL_STRENGTH +import com.android.settingslib.AccessibilityContentDescriptions import com.android.systemui.Flags.statusBarStaticInoutIndicators import com.android.systemui.common.shared.model.ContentDescription import com.android.systemui.common.shared.model.Icon @@ -50,7 +50,7 @@ interface MobileIconViewModelCommon { /** True if this view should be visible at all. */ val isVisible: StateFlow<Boolean> val icon: Flow<SignalIconModel> - val contentDescription: Flow<ContentDescription> + val contentDescription: Flow<ContentDescription?> val roaming: Flow<Boolean> /** The RAT icon (LTE, 3G, 5G, etc) to be displayed. Null if we shouldn't show anything */ val networkTypeIcon: Flow<Icon.Resource?> @@ -123,7 +123,7 @@ class MobileIconViewModel( override val icon: Flow<SignalIconModel> = vmProvider.flatMapLatest { it.icon } - override val contentDescription: Flow<ContentDescription> = + override val contentDescription: Flow<ContentDescription?> = vmProvider.flatMapLatest { it.contentDescription } override val roaming: Flow<Boolean> = vmProvider.flatMapLatest { it.roaming } @@ -206,12 +206,26 @@ private class CellularIconViewModel( override val icon: Flow<SignalIconModel> = iconInteractor.signalLevelIcon - override val contentDescription: Flow<ContentDescription> = run { - val initial = ContentDescription.Resource(PHONE_SIGNAL_STRENGTH[0]) + override val contentDescription: Flow<ContentDescription?> = iconInteractor.signalLevelIcon - .map { ContentDescription.Resource(PHONE_SIGNAL_STRENGTH[it.level]) } - .stateIn(scope, SharingStarted.WhileSubscribed(), initial) - } + .map { + // We expect the signal icon to be cellular here since this is the cellular vm + if (it !is SignalIconModel.Cellular) { + null + } else { + val resId = + AccessibilityContentDescriptions.getDescriptionForLevel( + it.level, + it.numberOfLevels + ) + if (resId != 0) { + ContentDescription.Resource(resId) + } else { + null + } + } + } + .stateIn(scope, SharingStarted.WhileSubscribed(), null) private val showNetworkTypeIcon: Flow<Boolean> = combine( diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt index 83d0fe8f9c4b..af49fd1753b6 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/pipeline/mobile/ui/viewmodel/MobileIconViewModelTest.kt @@ -54,6 +54,7 @@ import com.android.systemui.statusbar.policy.data.repository.FakeUserSetupReposi import com.android.systemui.util.CarrierConfigTracker import com.android.systemui.util.mockito.whenever import com.google.common.truth.Truth.assertThat +import com.google.common.truth.Truth.assertWithMessage import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.filterIsInstance import kotlinx.coroutines.flow.launchIn @@ -279,6 +280,39 @@ class MobileIconViewModelTest : SysuiTestCase() { } @Test + fun contentDescription_nonInflated_invalidLevelIsNull() = + testScope.runTest { + val latest by collectLastValue(underTest.contentDescription) + + repository.setAllLevels(-1) + assertThat(latest).isNull() + + repository.setAllLevels(100) + assertThat(latest).isNull() + } + + @Test + fun contentDescription_nonInflated_testABunchOfLevelsForNull() = + testScope.runTest { + val latest by collectLastValue(underTest.contentDescription) + + repository.numberOfLevels.value = 5 + + // -1 and 5 are out of the bounds for non-inflated content descriptions + for (i in -1..5) { + repository.setAllLevels(i) + when (i) { + -1, + 5 -> assertWithMessage("Level $i is expected to be null").that(latest).isNull() + else -> + assertWithMessage("Level $i is expected not to be null") + .that(latest) + .isNotNull() + } + } + } + + @Test fun networkType_dataEnabled_groupIsRepresented() = testScope.runTest { val expected = |