Fix bug where focus could be assigned to the panel itself in Select-to-speak.

Panel is no longer focusable, only its child buttons. STS JS code now calls focus() on button instead of panel itself to re-focus panel.

AX-Relnotes: Fix bug where pressing Search key + S twice in a row without text selection while panel is visible would focus panel but then remove focus from panel button.

Bug: 1166733
Change-Id: I45958dabbe3ee4ba21ddee26f6e00a3a75d0f464
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2629658
Commit-Queue: Joel Riley <joelriley@google.com>
Reviewed-by: Katie Dektar <katie@chromium.org>
Cr-Commit-Position: refs/heads/master@{#843808}

ash/system/accessibility/select_to_speak_menu_bubble_controller.cc

@@ -54,7 +54,6 @@ void SelectToSpeakMenuBubbleController::Show(const gfx::Rect& anchor,
     bubble_view_ = new TrayBubbleView(init_params);
     bubble_view_->SetArrow(views::BubbleBorder::TOP_LEFT);
     bubble_view_->SetCanActivate(true);
-    bubble_view_->SetFocusBehavior(ActionableView::FocusBehavior::ALWAYS);
 
     menu_view_ = new SelectToSpeakMenuView(this);
     menu_view_->SetBorder(

chrome/browser/resources/chromeos/accessibility/select_to_speak/select_to_speak.js

@@ -107,8 +107,11 @@ export class SelectToSpeak {
     /** @private {chrome.automation.AutomationNode} */
     this.desktop_;
 
-    /** @private {?chrome.automation.AutomationNode} */
-    this.panel_ = null;
+    /**
+     * Button in the floating panel, useful for restoring focus to the panel.
+     * @private {?chrome.automation.AutomationNode}
+     */
+    this.panelButton_ = null;
 
     /** @private {number|undefined} */
     this.intervalRef_;
@@ -376,7 +379,7 @@ export class SelectToSpeak {
         windowParent.children.length === 1 &&
         windowParent.children[0].className ===
             SELECT_TO_SPEAK_MENU_CLASS_NAME) {
-      this.panel_ = windowParent;
+      this.panelButton_ = focusedNode;
     }
   }
 
@@ -607,11 +610,13 @@ export class SelectToSpeak {
   focusPanel_() {
     // Used cached panel node if possible to avoid expensive desktop.find().
     // Note: Checking role attribute to see if node is still valid.
-    if (this.panel_ && this.panel_.role) {
-      this.panel_.focus();
+    if (this.panelButton_ && this.panelButton_.role) {
+      // The panel itself isn't focusable, so set focus to most recently
+      // focused panel button.
+      this.panelButton_.focus();
       return;
     }
-    this.panel_ = null;
+    this.panelButton_ = null;
 
     // Fallback to more expensive method of finding panel.
     const menuView = this.desktop_.find(
@@ -619,8 +624,8 @@ export class SelectToSpeak {
     if (menuView !== null && menuView.parent &&
         menuView.parent.className === TRAY_BUBBLE_VIEW_CLASS_NAME) {
       // The menu view's parent is the TrayBubbleView can can be assigned focus.
-      this.panel_ = menuView.parent;
-      this.panel_.focus();
+      this.panelButton_ = menuView.find({role: RoleType.TOGGLE_BUTTON});
+      this.panelButton_.focus();
     }
   }
 
@@ -1955,9 +1960,6 @@ export class SelectToSpeak {
     if (!node) {
       return false;
     }
-    if (node === this.panel_) {
-      return true;
-    }
 
     // Determine if the node is part of the floating panel or the reading speed
     // selection bubble.

chrome/browser/resources/chromeos/accessibility/select_to_speak/select_to_speak_navigation_control_test.js

@@ -20,6 +20,7 @@ SelectToSpeakNavigationControlTest = class extends SelectToSpeakE2ETest {
     var runTest = this.deferRunTest(WhenTestDone.EXPECT);
 
     window.EventType = chrome.automation.EventType;
+    window.RoleType = chrome.automation.RoleType;
     window.SelectToSpeakState = chrome.accessibilityPrivate.SelectToSpeakState;
 
     (async function() {
@@ -27,8 +28,6 @@ SelectToSpeakNavigationControlTest = class extends SelectToSpeakE2ETest {
       window.selectToSpeak = module.selectToSpeak;
 
       module = await import('/select_to_speak/select_to_speak.js');
-      window.SELECT_TO_SPEAK_TRAY_CLASS_NAME =
-          module.SELECT_TO_SPEAK_TRAY_CLASS_NAME;
 
       module = await import('/select_to_speak/select_to_speak_constants.js');
       window.SelectToSpeakConstants = module.SelectToSpeakConstants;
@@ -56,6 +55,28 @@ SelectToSpeakNavigationControlTest = class extends SelectToSpeakE2ETest {
     </script>
     <body onload="doSelection()">${bodyHtml}</body>`;
   }
+
+  isNodeWithinPanel(node) {
+    const windowParent =
+        AutomationUtil.getFirstAncestorWithRole(node, RoleType.WINDOW);
+    return windowParent.className === 'TrayBubbleView' &&
+        windowParent.children.length === 1 &&
+        windowParent.children[0].className === 'SelectToSpeakMenuView';
+  }
+
+  waitForPanelFocus(root, callback) {
+    callback = this.newCallback(callback);
+    const focusCallback = () => {
+      chrome.automation.getFocus((node) => {
+        if (!this.isNodeWithinPanel(node)) {
+          return;
+        }
+        root.removeEventListener(EventType.FOCUS, focusCallback);
+        callback(node);
+      });
+    };
+    root.addEventListener(EventType.FOCUS, focusCallback);
+  }
 };
 
 TEST_F(
@@ -746,4 +767,63 @@ TEST_F(
               assertEquals(selectToSpeak.state_, SelectToSpeakState.SPEAKING);
             });
           });
+    });
+
+TEST_F(
+    'SelectToSpeakNavigationControlTest', 'SetsInitialFocusToPanel',
+    function() {
+      const bodyHtml = '<p id="p1">Sample text</p>';
+      this.runWithLoadedTree(
+          this.generateHtmlWithSelectedElement('p1', bodyHtml), (root) => {
+            const desktop = root.parent.root;
+
+            // Wait for button in STS panel to be focused.
+            // Test will fail if panel is never focused.
+            this.waitForPanelFocus(desktop, () => {});
+
+            // Trigger STS, which will initially set focus to the panel.
+            this.triggerReadSelectedText();
+          });
+    });
+
+TEST_F(
+    'SelectToSpeakNavigationControlTest', 'KeyboardShortcutKeepsFocusInPanel',
+    function() {
+      const bodyHtml = '<p id="p1">Sample text</p>';
+      this.runWithLoadedTree(
+          this.generateHtmlWithSelectedElement('p1', bodyHtml), (root) => {
+            const desktop = root.parent.root;
+
+            // Wait for button within STS panel is focused.
+            this.waitForPanelFocus(desktop, () => {
+              // Remove text selection.
+              const textNode = this.findTextNode(root, 'Sample text');
+              chrome.automation.setDocumentSelection({
+                anchorObject: textNode,
+                anchorOffset: 0,
+                focusObject: textNode,
+                focusOffset: 0
+              });
+
+              // Perform Search key + S, which should restore focus to
+              // panel.
+              selectToSpeak.fireMockKeyDownEvent(
+                  {keyCode: SelectToSpeakConstants.SEARCH_KEY_CODE});
+              selectToSpeak.fireMockKeyDownEvent(
+                  {keyCode: SelectToSpeakConstants.READ_SELECTION_KEY_CODE});
+              selectToSpeak.fireMockKeyUpEvent(
+                  {keyCode: SelectToSpeakConstants.READ_SELECTION_KEY_CODE});
+              selectToSpeak.fireMockKeyUpEvent(
+                  {keyCode: SelectToSpeakConstants.SEARCH_KEY_CODE});
+
+              // Verify focus is still on button within panel.
+              chrome.automation.getFocus(this.newCallback((focusedNode) => {
+                assertEquals(focusedNode.role, RoleType.TOGGLE_BUTTON);
+                assertTrue(this.isNodeWithinPanel(focusedNode));
+              }));
+            });
+
+            // Trigger STS, which will initially set focus to the panel.
+            this.triggerReadSelectedText();
+          });
     });
\ No newline at end of file