diff --git a/chrome/browser/resources/chromeos/switch_access/back_button_manager.js b/chrome/browser/resources/chromeos/switch_access/back_button_manager.js index ef3fc82ade3344..053647457eaae0 100644 --- a/chrome/browser/resources/chromeos/switch_access/back_button_manager.js +++ b/chrome/browser/resources/chromeos/switch_access/back_button_manager.js @@ -22,9 +22,6 @@ class BackButtonManager { /** @private {PanelInterface} */ this.menuPanel_; - - /** @private {chrome.automation.AutomationNode} */ - this.buttonNode_; } /** @@ -66,28 +63,10 @@ class BackButtonManager { } /** - * Returns the button node, if we have found it. - * @return {chrome.automation.AutomationNode} - */ - buttonNode() { - return this.buttonNode_; - } - - /** - * Sets the reference to the menu panel and finds the back button node. + * Sets the reference to the menu panel. * @param {!PanelInterface} menuPanel */ - init(menuPanel, desktop) { + setMenuPanel(menuPanel) { this.menuPanel_ = menuPanel; - this.buttonNode_ = - new AutomationTreeWalker( - desktop, constants.Dir.FORWARD, - {visit: (node) => node.htmlAttributes.id === SAConstants.BACK_ID}) - .next() - .node; - // TODO(anastasi): Determine appropriate event and listen for it, rather - // than setting a timeout. - if (!this.buttonNode_) - setTimeout(this.init.bind(this, menuPanel, desktop), 500); } } diff --git a/chrome/browser/resources/chromeos/switch_access/navigation_manager.js b/chrome/browser/resources/chromeos/switch_access/navigation_manager.js index b7460a5979757e..bdf66991e75edf 100644 --- a/chrome/browser/resources/chromeos/switch_access/navigation_manager.js +++ b/chrome/browser/resources/chromeos/switch_access/navigation_manager.js @@ -60,6 +60,13 @@ class NavigationManager { */ this.scopeStack_ = []; + /** + * Keeps track of when we're visiting the current scope as an actionable + * node. + * @private {boolean} + */ + this.visitingScopeAsActionable_ = false; + /** * Keeps track of if we are currently in a system menu. * @private {boolean} @@ -109,31 +116,36 @@ class NavigationManager { /** * Find the previous interesting node and update |this.node_|. If there is no - * previous node, |this.node_| will be set to the back button. + * previous node, |this.node_| will be set to the youngest descendant in the + * SwitchAccess scope tree to loop again. */ moveBackward() { if (this.menuManager_.moveBackward()) return; - if (this.node_ === this.backButtonManager_.buttonNode()) { - if (SwitchAccessPredicate.isActionable(this.scope_)) - this.setCurrentNode_(this.scope_); - else - this.setCurrentNode_(this.youngestDescendant_(this.scope_)); - return; - } - this.startAtValidNode_(); let treeWalker = new AutomationTreeWalker( this.node_, constants.Dir.BACKWARD, SwitchAccessPredicate.restrictions(this.scope_)); + // Special case: Scope is actionable + if (this.node_ === this.scope_ && this.visitingScopeAsActionable_) { + this.visitingScopeAsActionable_ = false; + this.setCurrentNode_(this.node_); + return; + } + let node = treeWalker.next().node; - if (node === this.scope_) - node = this.backButtonManager_.buttonNode(); + // Special case: Scope is actionable + if (node === this.scope_ && SwitchAccessPredicate.isActionable(node)) { + this.showScopeAsActionable_(); + return; + } + // If treeWalker returns undefined, that means we're at the end of the tree + // and we should start over. if (!node) node = this.youngestDescendant_(this.scope_); @@ -157,35 +169,24 @@ class NavigationManager { this.startAtValidNode_(); - const backButtonNode = this.backButtonManager_.buttonNode(); - if (this.node_ === this.scope_ && backButtonNode) { - this.setCurrentNode_(backButtonNode); - return; - } - - // Replace the back button with the scope to find the following node. - if (this.node_ === backButtonNode) - this.node_ = this.scope_; - let treeWalker = new AutomationTreeWalker( this.node_, constants.Dir.FORWARD, SwitchAccessPredicate.restrictions(this.scope_)); + // Special case: Scope is actionable. + if (this.node_ === this.scope_ && + SwitchAccessPredicate.isActionable(this.node_) && + !this.visitingScopeAsActionable_) { + this.showScopeAsActionable_(); + return; + } + this.visitingScopeAsActionable_ = false; let node = treeWalker.next().node; // If treeWalker returns undefined, that means we're at the end of the tree // and we should start over. - if (!node) { - if (SwitchAccessPredicate.isActionable(this.scope_)) { - node = this.scope_; - } else if (backButtonNode) { - node = backButtonNode; - } else { - this.node_ = this.scope_; - this.moveForward(); - return; - } - } + if (!node) + node = this.scope_; // We can't interact with the desktop node, so skip it. if (node === this.desktop_) { @@ -246,8 +247,11 @@ class NavigationManager { } if (this.node_ === this.scope_) { - this.node_.doDefault(); - return; + // If we're visiting the scope as actionable, perform the default action. + if (this.visitingScopeAsActionable_) { + this.node_.doDefault(); + return; + } } if (SwitchAccessPredicate.isGroup(this.node_, this.scope_)) { @@ -328,7 +332,7 @@ class NavigationManager { * @return {!MenuManager} */ connectMenuPanel(menuPanel) { - this.backButtonManager_.init(menuPanel, this.desktop_); + this.backButtonManager_.setMenuPanel(menuPanel); return this.menuManager_.connectMenuPanel(menuPanel); } @@ -483,17 +487,20 @@ class NavigationManager { return; this.scopeStack_.push(this.scope_); this.scope_ = node; - - // The first node will come immediately after the back button, so we set - // |this.node_| to the back button and call |moveForward|. - const backButtonNode = this.backButtonManager_.buttonNode(); - if (backButtonNode) - this.node_ = backButtonNode; - else - this.node_ = this.scope_; + this.node_ = node; this.moveForward(); } + /** + * Show the current scope as an actionable item. + */ + showScopeAsActionable_() { + this.node_ = this.scope_; + this.visitingScopeAsActionable_ = true; + + this.updateFocusRings_(this.node_.location); + } + /** * Checks if this.node_ is valid. If so, do nothing. * @@ -528,8 +535,8 @@ class NavigationManager { * @private */ updateFocusRings_(opt_focusRect) { - if (this.node_ === this.backButtonManager_.buttonNode()) { - this.backButtonManager_.show(this.scope_); + if (!opt_focusRect && this.node_ === this.scope_) { + this.backButtonManager_.show(this.node_); this.primaryFocusRing_.rects = []; this.scopeFocusRing_.rects = [this.scope_.location]; diff --git a/chrome/browser/resources/chromeos/switch_access/navigation_manager_test.extjs b/chrome/browser/resources/chromeos/switch_access/navigation_manager_test.extjs index a66c0850e50891..e79ca83ea86026 100644 --- a/chrome/browser/resources/chromeos/switch_access/navigation_manager_test.extjs +++ b/chrome/browser/resources/chromeos/switch_access/navigation_manager_test.extjs @@ -29,24 +29,10 @@ function navigateToWebpage(desktop) { switchAccess.moveForward(); } -function navigateToFirstTab(desktop) { - assertTrue(desktop != null); - const node = new AutomationTreeWalker(desktop, constants.Dir.FORWARD, - { visit: (node) => node.role === chrome.automation.RoleType.TAB } - ).next().node; - - assertTrue(node != null); - switchAccess.navigationManager_.node_ = node; -} - function currentNode() { return switchAccess.navigationManager_.node_; } -function backButton() { - return switchAccess.navigationManager_.backButtonManager_.buttonNode(); -} - TEST_F('SwitchAccessNavigationManagerTest', 'SelectButton', function() { const website = `data:text/html;charset=utf-8, @@ -100,10 +86,10 @@ TEST_F('SwitchAccessNavigationManagerTest', 'MoveForward', function() { const button3 = currentNode(); assertEquals('button3', button3.name); - // Moving forwards should take us to the back button. - // TODO(anastasi): Determine why the back button doesn't show up here. - //switchAccess.moveForward(); - //assertEquals(backButton(), currentNode()); + switchAccess.moveForward(); + + // check that the initialScope is the final element. + assertEquals(currentNode(), initialScope); switchAccess.moveForward(); @@ -131,12 +117,12 @@ TEST_F('SwitchAccessNavigationManagerTest', 'MoveBackward', function() { switchAccess.moveBackward(); - // Moving backwards from the first button should take us to the back button. - // TODO(anastasi): Determine why the back button doesn't show up here. - //switchAccess.moveBackward(); - //assertEquals(backButton(), currentNode()); + // Moving backwards from the first button should take us to the initialScope. + assertEquals(currentNode(), initialScope); + + switchAccess.moveBackward(); - // Moving backwards from the back button should take us to the third button. + // Moving backwards from the initialScope should take us to the last button. const button3 = currentNode(); assertEquals('button3', button3.name); @@ -168,62 +154,28 @@ TEST_F('SwitchAccessNavigationManagerTest', 'MoveBackAndForth', function() { const button1 = currentNode(); assertEquals('button1', button1.name); - // Moving backwards from the first button should take us to the back button. - // TODO(anastasi): Determine why the back button doesn't show up here. - //switchAccess.moveBackward(); - //assertEquals(backButton(), currentNode()); + switchAccess.moveBackward(); + + // Moving backwards from the first button should take us to the initialScope. + assertEquals(currentNode(), initialScope); switchAccess.moveBackward(); - // Moving backwards from the back button should take us to the last button. + // Moving backwards from the initialScope should take us to the last button. const button3 = currentNode(); assertEquals('button3', button3.name); - // Moving forwards from the last button should take us to the back button. - // TODO(anastasi): Determine why the back button doesn't show up here. - //switchAccess.moveForward(); - //assertEquals(backButton(), currentNode()); - switchAccess.moveForward(); - assertEquals(currentNode(), button1); - switchAccess.moveForward(); - const button2 = currentNode(); - assertEquals('button2', button2.name); - }); -}); - -TEST_F('SwitchAccessNavigationManagerTest', 'TabNavigation', - function() { - const website = `data:text/html;charset=utf-8, - `; - - this.runWithLoadedTree(website, function(desktop) { - navigateToFirstTab(desktop); - const tabNode = currentNode(); - - // Selecting the tab should enter it, placing our focus on the close button. - switchAccess.selectCurrentNode(); - - const closeButton = currentNode(); - assertEquals('Close', closeButton.name); - assertEquals(chrome.automation.RoleType.BUTTON, closeButton.role); + assertEquals(currentNode(), initialScope); switchAccess.moveForward(); - assertEquals(tabNode, currentNode()); - - // Moving forwards from the tab node should take us to the back button. - // TODO(anastasi): Determine why the back button doesn't show up here. - //switchAccess.moveForward(); - //assertEquals(backButton(), currentNode()); + assertEquals(currentNode(), button1); switchAccess.moveForward(); - assertEquals(closeButton, currentNode()); - // Moving backwards from the close button should take us to the back button. - // TODO(anastasi): Determine why the back button doesn't show up here. - //switchAccess.moveBackward(); - //assertEquals(backButton, currentNode()); + const button2 = currentNode(); + assertEquals('button2', button2.name); }); });