[Switch Access] Validate settings

Previously, there was a bug in Switch Access where the user could set
two commands to the same key, leading to ambiguity in expected behavior.

This change adds validation, and un-sets any commands previously set to
a key.

Bug: 1056310
Change-Id: I61569cdcdca13a38923db1d34b0577fc02ec1a08
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2075054
Commit-Queue: Anastasia Helfinstein <anastasi@google.com>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#745251}

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

@@ -68,7 +68,7 @@
         <template is="dom-if" route-path="/manageAccessibility/switchAccess">
           <settings-subpage associated-control="[[$$('#subpage-trigger')]]"
               page-title="$i18n{manageSwitchAccessSettings}">
-            <settings-switch-access-subpage prefs="{{prefs.settings.a11y}}">
+            <settings-switch-access-subpage prefs="{{prefs}}">
             </settings-switch-access-subpage>
           </settings-subpage>
         </template>

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

@@ -23,9 +23,8 @@
           $i18n{assignSelectSwitchLabel}
         </div>
         <settings-dropdown-menu label="$i18n{assignSelectSwitchLabel}"
-            pref="{{prefs.switch_access.select.setting}}"
-            menu-options="[[switchAssignOptions_]]"
-            on-settings-control-change="onSelectAssigned_">
+            pref="{{prefs.settings.a11y.switch_access.select.setting}}"
+            menu-options="[[switchAssignOptions_]]">
         </settings-dropdown-menu>
       </div>
       <div class="settings-box continuation list-item">
@@ -33,9 +32,8 @@
           $i18n{assignNextSwitchLabel}
         </div>
         <settings-dropdown-menu label="$i18n{assignNextSwitchLabel}"
-            pref="{{prefs.switch_access.next.setting}}"
-            menu-options="[[switchAssignOptions_]]"
-            on-settings-control-change="onNextAssigned_">
+            pref="{{prefs.settings.a11y.switch_access.next.setting}}"
+            menu-options="[[switchAssignOptions_]]">
         </settings-dropdown-menu>
       </div>
       <div class="settings-box continuation list-item">
@@ -43,26 +41,25 @@
           $i18n{assignPreviousSwitchLabel}
         </div>
         <settings-dropdown-menu label="$i18n{assignPreviousSwitchLabel}"
-            pref="{{prefs.switch_access.previous.setting}}"
-            menu-options="[[switchAssignOptions_]]"
-            on-settings-control-change="onPreviousAssigned_">
+            pref="{{prefs.settings.a11y.switch_access.previous.setting}}"
+            menu-options="[[switchAssignOptions_]]">
         </settings-dropdown-menu>
       </div>
     </div>
     <h2>$i18n{switchAccessAutoScanHeading}</h2>
     <div class="list-frame">
       <settings-toggle-button class="continuation list-item"
-          pref="{{prefs.switch_access.auto_scan.enabled}}"
+          pref="{{prefs.settings.a11y.switch_access.auto_scan.enabled}}"
           label="$i18n{switchAccessAutoScanLabel}">
       </settings-toggle-button>
       <div class="settings-box continuation list-item"
-          hidden$="[[!prefs.switch_access.auto_scan.enabled.value]]">
+          hidden$="[[!prefs.settings.a11y.switch_access.auto_scan.enabled.value]]">
         <div class="start sub-item settings-box-text" id="scanSpeed"
             aria-hidden="true">
           $i18n{switchAccessAutoScanSpeedLabel}
         </div>
         <settings-slider id="scanSpeedSlider"
-            pref="{{prefs.switch_access.auto_scan.speed_ms}}"
+            pref="{{prefs.settings.a11y.switch_access.auto_scan.speed_ms}}"
             ticks="[[autoScanSpeedRangeMs_]]"
             min="[[minScanSpeedMs_]]"
             max="[[maxScanSpeedMs_]]"
@@ -73,13 +70,13 @@
       </div>
       <div class="settings-box continuation list-item"
           hidden$="[[!showKeyboardScanSettings_(
-          prefs.switch_access.auto_scan.enabled.value)]]">
+          prefs.settings.a11y.switch_access.auto_scan.enabled.value)]]">
         <div class="start sub-item settings-box-text" id="keyboardScanSpeed"
             aria-hidden="true">
           $i18n{switchAccessAutoScanKeyboardSpeedLabel}
         </div>
         <settings-slider id="keyboardScanSpeedSlider"
-            pref="{{prefs.switch_access.auto_scan.keyboard.speed_ms}}"
+            pref="{{prefs.settings.a11y.switch_access.auto_scan.keyboard.speed_ms}}"
             ticks="[[autoScanSpeedRangeMs_]]"
             min="[[minScanSpeedMs_]]"
             max="[[maxScanSpeedMs_]]"

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

@@ -20,6 +20,31 @@ const SwitchAccessAssignmentValue = {
   ENTER: 2,
 };
 
+/**
+ * Available commands.
+ * @const
+ */
+const SWITCH_ACCESS_COMMANDS = ['next', 'previous', 'select'];
+
+/**
+ * The portion of the setting name common to all Switch Access preferences.
+ * @const
+ */
+const PREFIX = 'settings.a11y.switch_access.';
+
+/**
+ * The ending of the setting name for all key code preferences.
+ * @const
+ */
+const KEY_CODE_SUFFIX = '.key_codes';
+
+/**
+ * The ending of the setting name for all preferences referring to
+ * Switch Access command settings.
+ * @const
+ */
+const COMMAND_SUFFIX = '.setting';
+
 /** @type {!Array<number>} */
 const AUTO_SCAN_SPEED_RANGE_MS = [
   500,  600,  700,  800,  900,  1000, 1100, 1200, 1300, 1400, 1500, 1600,
@@ -27,6 +52,20 @@ const AUTO_SCAN_SPEED_RANGE_MS = [
   2900, 3000, 3100, 3200, 3300, 3400, 3500, 3600, 3700, 3800, 3900, 4000
 ];
 
+/**
+ * This function extracts the segment of a preference key after the fixed prefix
+ * and returns it. In cases where the preference is Switch Access command
+ * setting preference, it corresponds to the command name.
+ *
+ * @param {!chrome.settingsPrivate.PrefObject} pref
+ * @return {string}
+ */
+function getCommandNameFromCommandPref(pref) {
+  const nameStartIndex = PREFIX.length;
+  const nameEndIndex = pref.key.indexOf('.', nameStartIndex);
+  return pref.key.substring(nameStartIndex, nameEndIndex);
+}
+
 /**
  * @param {!Array<number>} ticksInMs
  * @return {!Array<!cr_slider.SliderTick>}
@@ -118,12 +157,27 @@ Polymer({
     },
   },
 
+  /** @override */
+  created() {
+    chrome.settingsPrivate.onPrefsChanged.addListener((prefs) => {
+      for (const pref of prefs) {
+        if (!pref.key.includes(PREFIX) || !pref.key.includes(COMMAND_SUFFIX)) {
+          continue;
+        }
+        const commandName = getCommandNameFromCommandPref(pref);
+        if (SWITCH_ACCESS_COMMANDS.includes(commandName)) {
+          this.onSwitchAssigned_(pref);
+        }
+      }
+    });
+  },
+
   /**
    * @return {string}
    * @private
    */
   currentSpeed_() {
-    const speed = this.get('prefs.switch_access.auto_scan.speed_ms.value');
+    const speed = this.getPref(PREFIX + 'auto_scan.speed_ms').value;
     if (typeof speed != 'number') {
       return '';
     }
@@ -139,43 +193,48 @@ Polymer({
     const improvedTextInputEnabled = loadTimeData.getBoolean(
         'showExperimentalAccessibilitySwitchAccessImprovedTextInput');
     const autoScanEnabled = /** @type {boolean} */
-        (this.getPref('switch_access.auto_scan.enabled').value);
+        (this.getPref(PREFIX + 'auto_scan.enabled').value);
     return improvedTextInputEnabled && autoScanEnabled;
   },
 
-  /**
-   * @param {string} command
-   */
-  onSwitchAssigned_(command) {
-    const pref = 'prefs.switch_access.' + command;
-    const keyCodeSuffix = '.key_codes.value';
-    const settingSuffix = '.setting.value';
+  /** @param {!chrome.settingsPrivate.PrefObject} newPref */
+  onSwitchAssigned_(newPref) {
+    const command = getCommandNameFromCommandPref(newPref);
+
+    if (newPref.value !== SwitchAccessAssignmentValue.NONE) {
+      // When setting to a value, enforce that no other command can have that
+      // value.
+      for (const val of SWITCH_ACCESS_COMMANDS) {
+        if (val === command) {
+          continue;
+        }
+        if (this.getPref(PREFIX + val + COMMAND_SUFFIX).value ===
+            newPref.value) {
+          chrome.settingsPrivate.setPref(
+              PREFIX + val + COMMAND_SUFFIX, SwitchAccessAssignmentValue.NONE);
+        }
+      }
+    }
 
-    switch (this.get(pref + settingSuffix)) {
+    // Because of complexities with mapping a ListPref to a settings-dropdown,
+    // we instead store two distinct preferences (one for the dropdown selection
+    // and one with the key codes that Switch Access intercepts). The following
+    // code sets the key code preference based on the dropdown preference.
+    switch (newPref.value) {
       case SwitchAccessAssignmentValue.NONE:
-        this.set(pref + keyCodeSuffix, []);
+        chrome.settingsPrivate.setPref(PREFIX + command + KEY_CODE_SUFFIX, []);
         break;
       case SwitchAccessAssignmentValue.SPACE:
-        this.set(pref + keyCodeSuffix, [32]);
+        chrome.settingsPrivate.setPref(
+            PREFIX + command + KEY_CODE_SUFFIX, [32]);
         break;
       case SwitchAccessAssignmentValue.ENTER:
-        this.set(pref + keyCodeSuffix, [13]);
+        chrome.settingsPrivate.setPref(
+            PREFIX + command + KEY_CODE_SUFFIX, [13]);
         break;
     }
   },
 
-  onNextAssigned_() {
-    this.onSwitchAssigned_('next');
-  },
-
-  onPreviousAssigned_() {
-    this.onSwitchAssigned_('previous');
-  },
-
-  onSelectAssigned_() {
-    this.onSwitchAssigned_('select');
-  },
-
   /**
    * @param {number} scanSpeedValueMs
    * @return {string} a string representing the scan speed in seconds.