Skip to content

Commit

Permalink
Bug 1873048 - Update AccessibilityUtils to include toolbar's scrollbu…
Browse files Browse the repository at this point in the history
…ttons in the isKeyboardFocusableBrowserToolbarButton check. r=Jamie,reusable-components-reviewers,tgiles

Include `scrollbutton-down` and `scrollbutton-up` toolbarbuttons in the `isKeyboardFocusableBrowserToolbarButton` check to avoid failing [isKeyboardFocusable a11y_check](https://searchfox.org/mozilla-central/source/testing/mochitest/tests/SimpleTest/AccessibilityUtils.js#450) for them. These toolbar buttons are not expected to be focusable with keyboard because the keyboard-only user would actually scroll tabs left/right in the toolbar while trying to navigate to them which would make them redundant in the focus order.

Depends on D197335

Differential Revision: https://phabricator.services.mozilla.com/D197713
  • Loading branch information
anna-yeddi committed Jan 29, 2024
1 parent 6894f9a commit 2da17be
Show file tree
Hide file tree
Showing 3 changed files with 16 additions and 7 deletions.
2 changes: 1 addition & 1 deletion browser/base/content/test/tabs/browser.toml
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ support-files = [
]

["browser_overflowScroll.js"]
fail-if = ["a11y_checks"] # Bug 1854233 scrollbutton-down/up may not be focusable and/or labeled
fail-if = ["a11y_checks"] # Bugs 1854233 and 1873049 scrollbutton-down/up are not labeled
skip-if = [
"win11_2009", # Bug 1797751
]
Expand Down
17 changes: 13 additions & 4 deletions testing/mochitest/tests/SimpleTest/AccessibilityUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -233,18 +233,27 @@ this.AccessibilityUtils = (function () {
if (!node || !node.ownerGlobal) {
return false;
}
const toolbar = node.closest("toolbar");
const toolbar =
node.closest("toolbar") ||
node.flattenedTreeParentNode.closest("toolbar");
if (!toolbar || toolbar.getAttribute("keyNav") != "true") {
return false;
}
// The Go button in the Url Bar is an example of a purposefully
// non-focusable image toolbar button that provides an mouse/touch-only
// control for the search query submission, while a keyboard user could
// press `Enter` to do it. We do not want to create an extra tab stop for
// such controls and the markup would include `keyNav="false"` to flag it.
// press `Enter` to do it. Similarly, two scroll buttons that appear when
// toolbar is overflowing, and keyboard-only users would actually scroll
// tabs in the toolbar while trying to navigate to these controls. When
// toolbarbuttons are redundant for keyboard users, we do not want to
// create an extra tab stop for such controls, thus we are expecting the
// button markup to include `keyNav="false"` attribute to flag it.
if (node.getAttribute("keyNav") == "false") {
const ariaRoles = getAriaRoles(accessible);
return ariaRoles.includes("button");
return (
ariaRoles.includes("button") ||
accessible.role == Ci.nsIAccessibleRole.ROLE_PUSHBUTTON
);
}
return node.ownerGlobal.ToolbarKeyboardNavigator._isButton(node);
}
Expand Down
4 changes: 2 additions & 2 deletions toolkit/content/widgets/arrowscrollbox.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@
return `
<html:link rel="stylesheet" href="chrome://global/skin/toolbarbutton.css"/>
<html:link rel="stylesheet" href="chrome://global/skin/arrowscrollbox.css"/>
<toolbarbutton id="scrollbutton-up" part="scrollbutton-up"/>
<toolbarbutton id="scrollbutton-up" part="scrollbutton-up" keyNav="false"/>
<spacer part="overflow-start-indicator"/>
<box class="scrollbox-clip" part="scrollbox-clip" flex="1">
<scrollbox part="scrollbox" flex="1">
<html:slot/>
</scrollbox>
</box>
<spacer part="overflow-end-indicator"/>
<toolbarbutton id="scrollbutton-down" part="scrollbutton-down"/>
<toolbarbutton id="scrollbutton-down" part="scrollbutton-down" keyNav="false"/>
`;
}

Expand Down

0 comments on commit 2da17be

Please sign in to comment.