[Switch Access] Clarify meaning of dashed focus ring

After feedback from leberly@ that the significance of the dashed focus
rings used by Switch Access is unclear, we are changing that behavior.

Per a discussion with lpalmaro@ and jasonwan@, the new behavior is to
show a dashed focus ring around the "next" focus when selecting the
element has a primary effect of changing the focus (i.e. selecting the
back button or entering a group), and no dashed ring otherwise.

The previous behavior was to always show the dashed ring as the current
group's location.

Bug: 996392
Change-Id: I5f13df27f7a57818cd0cca9f118fe46581009071
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1848352
Commit-Queue: Anastasia Helfinstein <anastasi@google.com>
Reviewed-by: Chris Hall <chrishall@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705273}

chrome/browser/resources/chromeos/switch_access/focus_ring_manager.js

@@ -25,12 +25,6 @@ class FocusRingManager {
      */
     this.colorPattern_ = /^#[0-9A-F]{3,8}$/i;
 
-    /**
-     * Keeps track of the scope node currently being focused.
-     * @private {chrome.automation.AutomationNode}
-     */
-    this.currentScope_;
-
     /**
      * A reference to the Switch Access object.
      * @private {!SwitchAccessInterface}
@@ -58,8 +52,8 @@ class FocusRingManager {
       color: color,
       secondaryColor: SAConstants.Focus.SECONDARY_COLOR
     });
-    this.rings_.set(SAConstants.Focus.ID.SCOPE, {
-      id: SAConstants.Focus.ID.SCOPE,
+    this.rings_.set(SAConstants.Focus.ID.NEXT, {
+      id: SAConstants.Focus.ID.NEXT,
       rects: [],
       type: chrome.accessibilityPrivate.FocusType.DASHED,
       color: color,
@@ -88,50 +82,58 @@ class FocusRingManager {
   }
 
   /**
-   * Sets the primary and scope focus rings to be around the given nodes.
-   * Saves the scope for future comparison.
+   * Sets the primary and next focus rings based on the current primary and
+   *     group nodes used for navigation.
    * @param {!chrome.automation.AutomationNode} primary
-   * @param {!chrome.automation.AutomationNode} scope
+   * @param {!chrome.automation.AutomationNode} group
    */
-  setFocusNodes(primary, scope) {
+  setFocusNodes(primary, group) {
     if (this.rings_.size === 0) return;
 
-    const focusRect = primary.location;
-
-    // If the scope element has not changed, we want to use the previously
-    // calculated rect as the current scope rect.
-    let scopeRect = scope.location;
-    const currentScopeRects = this.rings_.get(SAConstants.Focus.ID.SCOPE).rects;
-    if (currentScopeRects.length && scope === this.currentScope_)
-      scopeRect = currentScopeRects[0];
-    this.currentScope_ = scope;
-
     if (primary === this.backButtonManager_.backButtonNode()) {
-      this.backButtonManager_.show(scopeRect);
+      this.backButtonManager_.show(group.location);
 
       this.rings_.get(SAConstants.Focus.ID.PRIMARY).rects = [];
-      this.rings_.get(SAConstants.Focus.ID.SCOPE).rects = [scopeRect];
+      // Clear the dashed ring between transitions, as the animation is
+      // distracting.
+      this.rings_.get(SAConstants.Focus.ID.NEXT).rects = [];
+      this.updateFocusRings_();
+
+      this.rings_.get(SAConstants.Focus.ID.NEXT).rects = [group.location];
       this.updateFocusRings_();
       return;
     }
     this.backButtonManager_.hide();
 
-    // If the current element is not the back button, the scope rect should
-    // expand to contain the focus rect.
-    scopeRect = RectHelper.expandToFitWithPadding(
-        SAConstants.Focus.SCOPE_BUFFER, scopeRect, focusRect);
+    // If the primary node is a group, show its first child as the "next" focus.
+    if (SwitchAccessPredicate.isGroup(primary, group)) {
+      const firstChild = new AutomationTreeWalker(
+                             primary, constants.Dir.FORWARD,
+                             SwitchAccessPredicate.restrictions(primary))
+                             .next()
+                             .node;
+
+      // Clear the dashed ring between transitions, as the animation is
+      // distracting.
+      this.rings_.get(SAConstants.Focus.ID.NEXT).rects = [];
+      this.updateFocusRings_();
 
+      let focusRect = primary.location;
+      if (firstChild && firstChild.location) {
+        // If the current element is not the back button, the focus rect should
+        // expand to contain the child rect.
+        focusRect = RectHelper.expandToFitWithPadding(
+            SAConstants.Focus.GROUP_BUFFER, focusRect, firstChild.location);
+        this.rings_.get(SAConstants.Focus.ID.NEXT).rects =
+            [firstChild.location];
+      }
       this.rings_.get(SAConstants.Focus.ID.PRIMARY).rects = [focusRect];
-    this.rings_.get(SAConstants.Focus.ID.SCOPE).rects = [scopeRect];
       this.updateFocusRings_();
+      return;
     }
 
-  /**
-   * Clears the focus ring with the given ID.
-   * @param {!SAConstants.Focus.ID} id
-   */
-  clearRing(id) {
-    this.rings_.get(id).rects = [];
+    this.rings_.get(SAConstants.Focus.ID.PRIMARY).rects = [primary.location];
+    this.rings_.get(SAConstants.Focus.ID.NEXT).rects = [];
     this.updateFocusRings_();
   }
 
@@ -143,18 +145,6 @@ class FocusRingManager {
     this.updateFocusRings_();
   }
 
-  /**
-   * Sets the indicated focus ring to highlight the given rects.
-   * @param {!SAConstants.Focus.ID} id
-   * @param {!Array<chrome.accessibilityPrivate.ScreenRect>} rects
-   */
-  setRing(id, rects) {
-    if (this.rings_.size === 0) return;
-
-    this.rings_.get(id).rects = rects;
-    this.updateFocusRings_();
-  }
-
   /**
    * Updates all focus rings to reflect new location, color, style, or other
    * changes.

chrome/browser/resources/chromeos/switch_access/switch_access_constants.js

@@ -48,20 +48,20 @@ SAConstants.Focus.CLASS = 'focus';
  * @enum {string}
  */
 SAConstants.Focus.ID = {
-  // The ID for the user's current focus.
+  // The ID for the ring showing the user's current focus.
   PRIMARY: 'primary',
-  // The ID for the group containing the user's focus.
-  SCOPE: 'scope',
+  // The ID for the ring showing the next focus.
+  NEXT: 'next',
   // The ID for the area where text is being input.
   TEXT: 'text'
 };
 
 /**
- * The buffer (in dip) between the primary focus ring and the scope focus ring.
+ * The buffer (in dip) between a child's focus ring and its parent's focus ring.
  * @type {number}
  * @const
  */
-SAConstants.Focus.SCOPE_BUFFER = 2;
+SAConstants.Focus.GROUP_BUFFER = 2;
 
 /**
  * The inner color of the focus rings.
@@ -155,7 +155,7 @@ SAConstants.MenuAction = {
   SCROLL_RIGHT: chrome.automation.ActionType.SCROLL_RIGHT,
   // Scroll the current element (or its ancestor) up.
   SCROLL_UP: chrome.automation.ActionType.SCROLL_UP,
-  // Either perform the default action or enter a new scope, as applicable.
+  // Either perform the default action or enter a new group, as applicable.
   SELECT: 'select',
   // Open and jump to the Switch Access settings.
   SETTINGS: 'settings',

chrome/browser/resources/chromeos/switch_access/text_input_manager.js

@@ -17,7 +17,7 @@ class TextInputManager {
   }
 
   /**
-   * Enters the keyboard and draws the text input focus ring.
+   * Enters the keyboard.
    * @param {!chrome.automation.AutomationNode} node
    */
   enterKeyboard(node) {
@@ -25,17 +25,13 @@ class TextInputManager {
       return false;
 
     this.node_ = node;
-    this.navigationManager_.focusRingManager.setRing(
-        SAConstants.Focus.ID.TEXT, [this.node_.location]);
     return true;
   }
 
-  /** Resets the focus ring and the focus in |navigationManager_|. */
+  /** Resets the focus in |navigationManager_|. */
   returnToTextFocus() {
     if (!this.node_)
       return;
-    this.navigationManager_.focusRingManager.clearRing(
-        SAConstants.Focus.ID.TEXT);
     this.navigationManager_.exitCurrentScope(this.node_);
     this.node_ = null;
   }