Commit e410a714 authored by Luciano Pacheco's avatar Luciano Pacheco Committed by Commit Bot

Fix a11y on navigation/directory tree

This CL fixes the keyboard navigation when using screen reader/
Chromevox.

The main fix is to force focus on the cr.ui.Tree/DirectoryTree element
when navigating with Chromevox keyboard shortcut "Search+right arrow",
before the focus was staying on "body" element so Tree couldn't handle
the keyboard events to expand/collapse directories.

It's a series of small fixes.

1. Fix text read by Chromevox when navigating on directories, it was
reading all sub-directories names, instead of only the current focused
directory's name, fixed by adding |id| to each entries' label element,
to make existing |aria-labelledby| attribute work for screen reader.
This was broken because we overwrite the |innerHTML| from
|cr.ui.TreeItem|, losing the |id|.

2. Listen to |click| event on DirectoryTree and forces the focus to it
when the focus/activeElement is the "body" element. This happens when
Chromevox issues the |click| event when user press "Search+right arrow".

3. Replace |cr.ui.Tree|'s |role| attribute from "group" to "navigation",
which is a "landmark" type which is recommended [1]. This required to
change |cr.ui.Tree| to not overwrite existing |role| attribute.


[1] - https://developers.google.com/web/fundamentals/accessibility/how-to-review#take_advantage_of_headings_and_landmarks

Bug: 755278
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: If33be610e1d28f0a69205ee148783328a367fe80
Reviewed-on: https://chromium-review.googlesource.com/985772
Commit-Queue: Luciano Pacheco (SYD) <lucmult@chromium.org>
Reviewed-by: default avatarNaoki Fukino <fukino@chromium.org>
Reviewed-by: default avatarDemetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: default avatarDavid Tseng <dtseng@chromium.org>
Reviewed-by: default avatarSasha Morrissey <sashab@chromium.org>
Cr-Commit-Position: refs/heads/master@{#553917}
parent a0af0684
......@@ -106,12 +106,16 @@ var MENU_TREE_ITEM_INNER_HTML =
*/
function DirectoryItem(label, tree) {
var item = /** @type {DirectoryItem} */ (new cr.ui.TreeItem());
// Get the original label id defined by TreeItem, before overwriting
// prototype.
var labelId = item.labelElement.id;
item.__proto__ = DirectoryItem.prototype;
item.parentTree_ = tree;
item.directoryModel_ = tree.directoryModel;
item.fileFilter_ = tree.directoryModel.getFileFilter();
item.innerHTML = TREE_ITEM_INNER_HTML;
item.labelElement.id = labelId;
item.addEventListener('expand', item.onExpand_.bind(item), false);
// Listen for collapse because for the delayed expansion case all
......@@ -858,6 +862,9 @@ DriveVolumeItem.prototype.selectByEntry = function(entry) {
*/
function ShortcutItem(modelItem, tree) {
var item = /** @type {ShortcutItem} */ (new cr.ui.TreeItem());
// Get the original label id defined by TreeItem, before overwriting
// prototype.
var labelId = item.labelElement.id;
item.__proto__ = ShortcutItem.prototype;
item.parentTree_ = tree;
......@@ -865,6 +872,7 @@ function ShortcutItem(modelItem, tree) {
item.modelItem_ = modelItem;
item.innerHTML = TREE_ITEM_INNER_HTML;
item.labelElement.id = labelId;
var icon = item.querySelector('.icon');
icon.classList.add('item-icon');
......@@ -979,11 +987,15 @@ ShortcutItem.prototype.activate = function() {
*/
function MenuItem(modelItem, tree) {
var item = new cr.ui.TreeItem();
// Get the original label id defined by TreeItem, before overwriting
// prototype.
var labelId = item.labelElement.id;
item.__proto__ = MenuItem.prototype;
item.parentTree_ = tree;
item.modelItem_ = modelItem;
item.innerHTML = MENU_TREE_ITEM_INNER_HTML;
item.labelElement.id = labelId;
item.label = modelItem.label;
item.menuButton_ = /** @type {!cr.ui.MenuButton} */(queryRequiredElement(
......@@ -1054,12 +1066,16 @@ MenuItem.prototype.activate = function() {
*/
function RecentItem(modelItem, tree) {
var item = new cr.ui.TreeItem();
// Get the original label id defined by TreeItem, before overwriting
// prototype.
var labelId = item.labelElement.id;
item.__proto__ = RecentItem.prototype;
item.parentTree_ = tree;
item.modelItem_ = modelItem;
item.dirEntry_ = modelItem.entry;
item.innerHTML = TREE_ITEM_INNER_HTML;
item.labelElement.id = labelId;
item.label = modelItem.label;
var icon = queryRequiredElement('.icon', item);
......@@ -1370,6 +1386,15 @@ DirectoryTree.prototype.decorateDirectoryTree = function(
'entries-changed',
this.onEntriesChanged_.bind(this));
this.addEventListener('click', (event) => {
// Chromevox triggers |click| without switching focus, we force the focus
// here so we can handle further keyboard/mouse events to expand/collapse
// directories.
if (document.activeElement === document.body) {
this.focus();
}
});
this.privateOnDirectoryChangedBound_ =
this.onDirectoryContentChanged_.bind(this);
chrome.fileManagerPrivate.onDirectoryChanged.addListener(
......
......@@ -421,7 +421,7 @@
<div class="dialog-container">
<div class="dialog-navigation-list">
<div class="dialog-navigation-list-contents">
<tree id="directory-tree" tabindex="21"></tree>
<tree id="directory-tree" role="navigation" tabindex="21"></tree>
</div>
<div class="dialog-navigation-list-footer">
<div id="progress-center" hidden>
......
......@@ -39,14 +39,12 @@ test.selectFirstListItem = function() {
*/
test.createNewFolder = function(initialEntrySet) {
var maxListItemId = test.maxListItemId();
return Promise.resolve()
.then(() => {
// Push Ctrl + E.
assertTrue(
test.fakeKeyDown('#file-list', 'e', 'U+0045', true, false, false));
// Force focus to file list.
document.querySelector('#file-list:not([hidden])').focus();
// Press Ctrl+E to create a new folder.
assertTrue(test.fakeKeyDown('#file-list', 'e', 'U+0045', true, false, false));
// Wait for rename text field.
return test.waitForElement('li[renaming] input.rename');
})
return test.waitForElement('li[renaming] input.rename')
.then(() => {
var elements =
document.querySelectorAll('div.detail-table > list > li[selected]');
......
......@@ -40,6 +40,11 @@ function copyBetweenVolumes(targetFile,
chrome.test.assertTrue(result);
remoteCall.waitForFiles(appId, srcContents).then(this.next);
},
// Set focus on the file list.
function() {
remoteCall.callRemoteTestUtil(
'focus', appId, ['#file-list:not([hidden])'], this.next);
},
// Select the source file.
function() {
remoteCall.callRemoteTestUtil(
......
......@@ -58,6 +58,7 @@ cr.define('cr.ui', function() {
this.addEventListener('dblclick', this.handleDblClick);
this.addEventListener('keydown', this.handleKeyDown);
if (!this.hasAttribute('role'))
this.setAttribute('role', 'group');
},
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment