Add accessibility setting for showing shelf navigation buttons

Bug:1050544

TBR=tommycli@chromium.org
(LGTM given on the cl, but missed +1)

Change-Id: I53c0eae00f0cf080e9123e0b7849da67f6f9fde2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2049083
Commit-Queue: Toni Baržić <tbarzic@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Manu Cornet <manucornet@chromium.org>
Reviewed-by: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#742073}

ash/accessibility/accessibility_controller_impl.cc

@@ -412,6 +412,9 @@ void AccessibilityControllerImpl::RegisterProfilePrefs(
   registry->RegisterBooleanPref(
       prefs::kAccessibilityVirtualKeyboardEnabled, false,
       user_prefs::PrefRegistrySyncable::SYNCABLE_OS_PREF);
+  registry->RegisterBooleanPref(
+      prefs::kAccessibilityTabletModeShelfNavigationButtonsEnabled, false,
+      user_prefs::PrefRegistrySyncable::SYNCABLE_OS_PREF);
   registry->RegisterBooleanPref(
       prefs::kHighContrastAcceleratorDialogHasBeenAccepted, false);
   registry->RegisterBooleanPref(
@@ -910,6 +913,16 @@ bool AccessibilityControllerImpl::IsEnterpriseIconVisibleForVirtualKeyboard() {
       prefs::kAccessibilityVirtualKeyboardEnabled);
 }
 
+void AccessibilityControllerImpl::SetTabletModeShelfNavigationButtonsEnabled(
+    bool enabled) {
+  if (!active_user_prefs_)
+    return;
+
+  active_user_prefs_->SetBoolean(
+      prefs::kAccessibilityTabletModeShelfNavigationButtonsEnabled, enabled);
+  active_user_prefs_->CommitPendingWrite();
+}
+
 void AccessibilityControllerImpl::TriggerAccessibilityAlert(
     AccessibilityAlert alert) {
   if (client_)
@@ -1228,6 +1241,11 @@ void AccessibilityControllerImpl::ObservePrefs(PrefService* prefs) {
       base::BindRepeating(
           &AccessibilityControllerImpl::UpdateVirtualKeyboardFromPref,
           base::Unretained(this)));
+  pref_change_registrar_->Add(
+      prefs::kAccessibilityTabletModeShelfNavigationButtonsEnabled,
+      base::BindRepeating(&AccessibilityControllerImpl::
+                              UpdateTabletModeShelfNavigationButtonsFromPref,
+                          base::Unretained(this)));
 
   // Load current state.
   UpdateAutoclickFromPref();
@@ -1249,6 +1267,7 @@ void AccessibilityControllerImpl::ObservePrefs(PrefService* prefs) {
   UpdateStickyKeysFromPref();
   UpdateSwitchAccessFromPref();
   UpdateVirtualKeyboardFromPref();
+  UpdateTabletModeShelfNavigationButtonsFromPref();
   UpdateShortcutsEnabledFromPref();
 }
 
@@ -1692,6 +1711,20 @@ void AccessibilityControllerImpl::UpdateVirtualKeyboardFromPref() {
   keyboard::SetAccessibilityKeyboardEnabled(enabled);
 }
 
+void AccessibilityControllerImpl::
+    UpdateTabletModeShelfNavigationButtonsFromPref() {
+  DCHECK(active_user_prefs_);
+  const bool enabled = active_user_prefs_->GetBoolean(
+      prefs::kAccessibilityTabletModeShelfNavigationButtonsEnabled);
+
+  if (tablet_mode_shelf_navigation_buttons_enabled_ == enabled)
+    return;
+
+  tablet_mode_shelf_navigation_buttons_enabled_ = enabled;
+
+  NotifyAccessibilityStatusChanged();
+}
+
 base::string16 AccessibilityControllerImpl::GetBatteryDescription() const {
   // Pass battery status as string to callback function.
   return PowerStatus::Get()->GetAccessibleNameString(

ash/accessibility/accessibility_controller_impl.h

@@ -165,6 +165,11 @@ class ASH_EXPORT AccessibilityControllerImpl : public AccessibilityController,
   bool IsVirtualKeyboardSettingVisibleInTray();
   bool IsEnterpriseIconVisibleForVirtualKeyboard();
 
+  void SetTabletModeShelfNavigationButtonsEnabled(bool enabled);
+  bool tablet_mode_shelf_navigation_buttons_enabled() const {
+    return tablet_mode_shelf_navigation_buttons_enabled_;
+  }
+
   bool dictation_active() const { return dictation_active_; }
 
   // Returns true if accessibility shortcuts have been disabled.
@@ -296,6 +301,7 @@ class ASH_EXPORT AccessibilityControllerImpl : public AccessibilityController,
   void UpdateVirtualKeyboardFromPref();
   void UpdateAccessibilityHighlightingFromPrefs();
   void UpdateShortcutsEnabledFromPref();
+  void UpdateTabletModeShelfNavigationButtonsFromPref();
 
   void MaybeCreateSelectToSpeakEventHandler();
   void MaybeCreateSwitchAccessEventHandler();
@@ -326,6 +332,7 @@ class ASH_EXPORT AccessibilityControllerImpl : public AccessibilityController,
   bool virtual_keyboard_enabled_ = false;
   bool dictation_active_ = false;
   bool shortcuts_enabled_ = true;
+  bool tablet_mode_shelf_navigation_buttons_enabled_ = false;
 
   SelectToSpeakState select_to_speak_state_ =
       SelectToSpeakState::kSelectToSpeakStateInactive;

ash/public/cpp/ash_pref_names.cc

@@ -117,6 +117,12 @@ const char kAccessibilitySwitchAccessAutoScanSpeedMs[] =
 // enabled).
 const char kAccessibilitySwitchAccessAutoScanKeyboardSpeedMs[] =
     "settings.a11y.switch_access.auto_scan.keyboard.speed_ms";
+// A boolean pref which, if set, indicates that shelf navigation buttons (home,
+// back and overview button) should be shown in tablet mode. Note that shelf
+// buttons might be shown even if the pref value is false - for example, if
+// spoken feedback, autoclick or switch access are enabled.
+const char kAccessibilityTabletModeShelfNavigationButtonsEnabled[] =
+    "settings.a11y.tablet_mode_shelf_nav_buttons_enabled";
 // A boolean pref which determines whether dictation is enabled.
 const char kAccessibilityDictationEnabled[] = "settings.a11y.dictation";
 // A boolean pref which determines whether the accessibility menu shows

ash/public/cpp/ash_pref_names.h

@@ -45,6 +45,8 @@ ASH_PUBLIC_EXPORT extern const char kAccessibilitySwitchAccessAutoScanEnabled[];
 ASH_PUBLIC_EXPORT extern const char kAccessibilitySwitchAccessAutoScanSpeedMs[];
 ASH_PUBLIC_EXPORT extern const char
     kAccessibilitySwitchAccessAutoScanKeyboardSpeedMs[];
+ASH_PUBLIC_EXPORT extern const char
+    kAccessibilityTabletModeShelfNavigationButtonsEnabled[];
 ASH_PUBLIC_EXPORT extern const char kAccessibilityDictationEnabled[];
 ASH_PUBLIC_EXPORT extern const char kShouldAlwaysShowAccessibilityMenu[];
 

ash/shelf/back_button_unittest.cc

@@ -81,6 +81,7 @@ class BackButtonTest : public AshTestBase,
 };
 
 enum class TestAccessibilityFeature {
+  kTabletModeShelfNavigationButtons,
   kSpokenFeedback,
   kAutoclick,
   kSwitchAccess
@@ -102,6 +103,11 @@ class BackButtonVisibilityWithAccessibilityFeaturesTest
 
   void SetTestA11yFeatureEnabled(bool enabled) {
     switch (GetParam()) {
+      case TestAccessibilityFeature::kTabletModeShelfNavigationButtons:
+        Shell::Get()
+            ->accessibility_controller()
+            ->SetTabletModeShelfNavigationButtonsEnabled(enabled);
+        break;
       case TestAccessibilityFeature::kSpokenFeedback:
         Shell::Get()->accessibility_controller()->SetSpokenFeedbackEnabled(
             enabled, A11Y_NOTIFICATION_NONE);
@@ -252,9 +258,11 @@ TEST_P(BackButtonTest, NoContextMenuOnBackButton) {
 INSTANTIATE_TEST_SUITE_P(
     All,
     BackButtonVisibilityWithAccessibilityFeaturesTest,
-    ::testing::Values(TestAccessibilityFeature::kSpokenFeedback,
-                      TestAccessibilityFeature::kAutoclick,
-                      TestAccessibilityFeature::kSwitchAccess));
+    ::testing::Values(
+        TestAccessibilityFeature::kTabletModeShelfNavigationButtons,
+        TestAccessibilityFeature::kSpokenFeedback,
+        TestAccessibilityFeature::kAutoclick,
+        TestAccessibilityFeature::kSwitchAccess));
 
 TEST_P(BackButtonVisibilityWithAccessibilityFeaturesTest,
        TabletModeSwitchWithA11yFeatureEnabled) {

ash/shelf/home_button_unittest.cc

@@ -117,6 +117,7 @@ class HomeButtonTest
 };
 
 enum class TestAccessibilityFeature {
+  kTabletModeShelfNavigationButtons,
   kSpokenFeedback,
   kAutoclick,
   kSwitchAccess
@@ -138,6 +139,11 @@ class HomeButtonVisibilityWithAccessibilityFeaturesTest
 
   void SetTestA11yFeatureEnabled(bool enabled) {
     switch (GetParam()) {
+      case TestAccessibilityFeature::kTabletModeShelfNavigationButtons:
+        Shell::Get()
+            ->accessibility_controller()
+            ->SetTabletModeShelfNavigationButtonsEnabled(enabled);
+        break;
       case TestAccessibilityFeature::kSpokenFeedback:
         Shell::Get()->accessibility_controller()->SetSpokenFeedbackEnabled(
             enabled, A11Y_NOTIFICATION_NONE);
@@ -579,9 +585,11 @@ TEST_P(HomeButtonTest, ClickOnCornerPixel) {
 INSTANTIATE_TEST_SUITE_P(
     All,
     HomeButtonVisibilityWithAccessibilityFeaturesTest,
-    ::testing::Values(TestAccessibilityFeature::kSpokenFeedback,
-                      TestAccessibilityFeature::kAutoclick,
-                      TestAccessibilityFeature::kSwitchAccess));
+    ::testing::Values(
+        TestAccessibilityFeature::kTabletModeShelfNavigationButtons,
+        TestAccessibilityFeature::kSpokenFeedback,
+        TestAccessibilityFeature::kAutoclick,
+        TestAccessibilityFeature::kSwitchAccess));
 
 TEST_P(HomeButtonVisibilityWithAccessibilityFeaturesTest,
        TabletModeSwitchWithA11yFeatureEnabled) {

ash/shelf/shelf_config.cc

@@ -34,7 +34,9 @@ bool ShelfControlsForcedShownForAccessibility() {
       Shell::Get()->accessibility_controller();
   return accessibility_controller->spoken_feedback_enabled() ||
          accessibility_controller->autoclick_enabled() ||
-         accessibility_controller->switch_access_enabled();
+         accessibility_controller->switch_access_enabled() ||
+         accessibility_controller
+             ->tablet_mode_shelf_navigation_buttons_enabled();
 }
 
 }  // namespace

chrome/app/os_settings_strings.grdp

@@ -92,5 +92,13 @@
   <message name="IDS_OS_SETTINGS_FILES" desc="Name of the settings page which displays file preferences.">
     Files
   </message>
+
+  <!-- Accessibility -->
+    <message name="IDS_SETTINGS_A11Y_TABLET_MODE_SHELF_BUTTONS_LABEL" desc="The name of a setting within accessibility settings that controls whether Chrome OS system shelf navigation buttons (for going back, home, or to overview) should be shown when the device is in tablet mode.">
+      Show navigation buttons
+    </message>
+    <message name="IDS_SETTINGS_A11Y_TABLET_MODE_SHELF_BUTTONS_DESCRIPTION" desc="The description for the setting within accessibility settings that controls whether Chrome OS system shelf navigation buttons should be shown when the device is in tablet mode. The buttons whose visibility the setting controls are the button to go home (to launcher), the button to go back, and the button to go to overview.">
+      Show Launcher, Back, Overview buttons in Shelf in tablet mode. Turned on when ChromeVox (spoken feedback) or Switch Access is enabled.
+    </message>
 </if>
 </grit-part>

chrome/app/os_settings_strings_grdp/IDS_SETTINGS_A11Y_TABLET_MODE_SHELF_BUTTONS_DESCRIPTION.png.sha1

+6adb18106f0beefa3fca77dcba3011c63949b4c1
\ No newline at end of file

chrome/app/os_settings_strings_grdp/IDS_SETTINGS_A11Y_TABLET_MODE_SHELF_BUTTONS_LABEL.png.sha1

+45e1d0ad17eb75101baff6da94a04bed33946b92
\ No newline at end of file

chrome/browser/extensions/api/settings_private/prefs_util.cc

@@ -420,6 +420,9 @@ const PrefsUtil::TypedPrefMap& PrefsUtil::GetWhitelistedKeys() {
       settings_api::PrefType::PREF_TYPE_BOOLEAN;
   (*s_whitelist)[ash::prefs::kAccessibilitySwitchAccessAutoScanSpeedMs] =
       settings_api::PrefType::PREF_TYPE_NUMBER;
+  (*s_whitelist)
+      [ash::prefs::kAccessibilityTabletModeShelfNavigationButtonsEnabled] =
+          settings_api::PrefType::PREF_TYPE_BOOLEAN;
   (*s_whitelist)
       [ash::prefs::kAccessibilitySwitchAccessAutoScanKeyboardSpeedMs] =
           settings_api::PrefType::PREF_TYPE_NUMBER;

chrome/browser/resources/settings/chromeos/os_a11y_page/manage_a11y_page.html

@@ -232,6 +232,23 @@
         sub-label="$i18n{mouseSettingsDescription}"
         role-description="$i18n{subpageArrowRoleDescription}" embedded>
     </cr-link-row>
+    <!-- If shelf navigation buttons are implicitly enabled by other a11y
+        settings (e.g. by spoken feedback), the toggle control should be
+        disabled, and toggled regardless of the actual backing pref value. To
+        handle effective pref value changes, wrap the backing pref in a private
+        property, which changes to a stub pref object when the setting is
+        implicitly enabled. -->
+    <settings-toggle-button
+        id="shelfNavigationButtonsEnabledControl"
+        hidden$="[[!showShelfNavigationButtonsSettings_]]"
+        disabled="[[shelfNavigationButtonsImplicitlyEnabled_]]"
+        pref="[[shelfNavigationButtonsPref_]]"
+        no-set-pref
+        on-settings-boolean-control-change=
+            "updateShelfNavigationButtonsEnabledPref_"
+        label="$i18n{tabletModeShelfNavigationButtonsSettingLabel}"
+        sub-label="$i18n{tabletModeShelfNavigationButtonsSettingDescription}">
+    </settings-toggle-button>
 
     <h2>$i18n{audioAndCaptionsHeading}</h2>
     <cr-link-row id="captionsSubpageButton" class="first"

chrome/browser/resources/settings/chromeos/os_a11y_page/manage_a11y_page.js

@@ -106,6 +106,19 @@ Polymer({
       },
     },
 
+    /**
+     * Whether a setting for enabling shelf navigation buttons in tablet mode
+     * should be displayed in the accessibility settings.
+     * @private
+     */
+    showShelfNavigationButtonsSettings_: {
+      type: Boolean,
+      value() {
+        return loadTimeData.getBoolean(
+            'showTabletModeShelfNavigationButtonsSettings');
+      },
+    },
+
     /** @private */
     isGuest_: {
       type: Boolean,
@@ -126,6 +139,34 @@ Polymer({
 
     /** @private */
     hasTouchpad_: Boolean,
+
+    /**
+     * Boolean indicating whether shelf navigation buttons should implicitly be
+     * enabled in tablet mode - the navigation buttons are implicitly enabled
+     * when spoken feedback, automatic clicks, or switch access are enabled.
+     * The buttons can also be explicitly enabled by a designated a11y setting.
+     * @private
+     */
+    shelfNavigationButtonsImplicitlyEnabled_: {
+      type: Boolean,
+      computed: 'computeShelfNavigationButtonsImplicitlyEnabled_(' +
+          'prefs.settings.accessibility.value,' +
+          'prefs.settings.a11y.autoclick.value,' +
+          'prefs.settings.a11y.switch_access.enabled.value)',
+    },
+
+    /**
+     * The effective pref value that indicates whether shelf navigation buttons
+     * are enabled in tablet mode.
+     * @type {chrome.settingsPrivate.PrefObject}
+     * @private
+     */
+    shelfNavigationButtonsPref_: {
+      type: Object,
+      computed: 'getShelfNavigationButtonsEnabledPref_(' +
+          'shelfNavigationButtonsImplicitlyEnabled_,' +
+          'prefs.settings.a11y.tablet_mode_shelf_nav_buttons_enabled)',
+    },
   },
 
   observers: [
@@ -258,6 +299,65 @@ Polymer({
         /* dynamicParams */ null, /* removeSearch */ true);
   },
 
+  /**
+   * @return {boolean} Whether shelf navigation buttons should implicitly be
+   *     enabled in tablet mode (due to accessibility settings different than
+   *     shelf_navigation_buttons_enabled_in_tablet_mode).
+   * @private
+   */
+  computeShelfNavigationButtonsImplicitlyEnabled_() {
+    /**
+     * Gets the bool pref value for the provided pref key.
+     * @param {string} key
+     * @return {boolean}
+     */
+    const getBoolPrefValue = (key) => {
+      const pref = /** @type {chrome.settingsPrivate.PrefObject} */ (
+          this.get(key, this.prefs));
+      return pref && !!pref.value;
+    };
+
+    return getBoolPrefValue('settings.accessibility') ||
+        getBoolPrefValue('settings.a11y.autoclick') ||
+        getBoolPrefValue('settings.a11y.switch_access.enabled');
+  },
+
+  /**
+   * Calculates the effective value for "shelf navigation buttons enabled in
+   * tablet mode" setting - if the setting is implicitly enabled (by other a11y
+   * settings), this will return a stub pref value.
+   * @private
+   * @return {chrome.settingsPrivate.PrefObject}
+   */
+  getShelfNavigationButtonsEnabledPref_() {
+    if (this.shelfNavigationButtonsImplicitlyEnabled_) {
+      return /** @type {!chrome.settingsPrivate.PrefObject}*/ ({
+        value: true,
+        type: chrome.settingsPrivate.PrefType.BOOLEAN,
+        key: ''
+      });
+    }
+
+    return /** @type {chrome.settingsPrivate.PrefObject} */ (this.get(
+        'settings.a11y.tablet_mode_shelf_nav_buttons_enabled', this.prefs));
+  },
+
+  /**
+   * Handles the <code>tablet_mode_shelf_nav_buttons_enabled</code> setting's
+   * toggle changes. It updates the backing pref value, unless the setting is
+   * implicitly enabled.
+   * @private
+   */
+  updateShelfNavigationButtonsEnabledPref_() {
+    if (this.shelfNavigationButtonsImplicitlyEnabled_) {
+      return;
+    }
+
+    this.set(
+        'prefs.settings.a11y.tablet_mode_shelf_nav_buttons_enabled.value',
+        this.$.shelfNavigationButtonsEnabledControl.checked);
+  },
+
   /** @private */
   onMouseTap_() {
     settings.Router.getInstance().navigateTo(

chrome/browser/ui/webui/settings/chromeos/os_settings_localized_strings_provider.cc

@@ -309,6 +309,10 @@ void AddA11yStrings(content::WebUIDataSource* html_source) {
       {"textToSpeechPreviewVoice", IDS_SETTINGS_TEXT_TO_SPEECH_PREVIEW_VOICE},
       {"textToSpeechPreviewPlay", IDS_SETTINGS_TEXT_TO_SPEECH_PREVIEW_PLAY},
       {"textToSpeechEngines", IDS_SETTINGS_TEXT_TO_SPEECH_ENGINES},
+      {"tabletModeShelfNavigationButtonsSettingLabel",
+       IDS_SETTINGS_A11Y_TABLET_MODE_SHELF_BUTTONS_LABEL},
+      {"tabletModeShelfNavigationButtonsSettingDescription",
+       IDS_SETTINGS_A11Y_TABLET_MODE_SHELF_BUTTONS_DESCRIPTION},
   };
   AddLocalizedStringsBulk(html_source, kLocalizedStrings);
 
@@ -331,6 +335,10 @@ void AddA11yStrings(content::WebUIDataSource* html_source) {
                           base::FeatureList::IsEnabled(
                               ::features::kExperimentalAccessibilityLabels));
 
+  html_source->AddBoolean(
+      "showTabletModeShelfNavigationButtonsSettings",
+      ash::features::IsHideShelfControlsInTabletModeEnabled());
+
   html_source->AddBoolean("enableLiveCaption",
                           base::FeatureList::IsEnabled(media::kLiveCaption));