Bug 1335055 - improving accessibility of a tree component (keyboard and semantics). r=Honza
authorYura Zenevich <yzenevich@mozilla.com>
Thu, 16 Mar 2017 22:58:18 -0400
changeset 348203 b1d8962e07f92f7608c0b6da9067eab71c2a4514
parent 348202 12ce302b8f9be8fe2b8782c0b0a85dc2919f7563
child 348204 36a2230fbad6a80e477ae9669ef5e0ac5414fcf9
push id39092
push userkwierso@gmail.com
push dateFri, 17 Mar 2017 18:14:05 +0000
treeherderautoland@88576fd417e7 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersHonza
bugs1335055
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1335055 - improving accessibility of a tree component (keyboard and semantics). r=Honza MozReview-Commit-ID: 1ffA4VjuyUZ
devtools/client/shared/components/test/mochitest/head.js
devtools/client/shared/components/test/mochitest/test_tree_01.html
devtools/client/shared/components/test/mochitest/test_tree_02.html
devtools/client/shared/components/test/mochitest/test_tree_03.html
devtools/client/shared/components/test/mochitest/test_tree_04.html
devtools/client/shared/components/test/mochitest/test_tree_05.html
devtools/client/shared/components/test/mochitest/test_tree_06.html
devtools/client/shared/components/tree.js
--- a/devtools/client/shared/components/test/mochitest/head.js
+++ b/devtools/client/shared/components/test/mochitest/head.js
@@ -76,16 +76,33 @@ var TEST_TREE_INTERFACE = {
 
 function isRenderedTree(actual, expectedDescription, msg) {
   const expected = expectedDescription.map(x => x + "\n").join("");
   dumpn(`Expected tree:\n${expected}`);
   dumpn(`Actual tree:\n${actual}`);
   is(actual, expected, msg);
 }
 
+function isAccessibleTree(tree, options = {}) {
+  const treeNode = tree.refs.tree;
+  is(treeNode.getAttribute("tabindex"), "0", "Tab index is set");
+  is(treeNode.getAttribute("role"), "tree", "Tree semantics is present");
+  if (options.hasActiveDescendant) {
+    ok(treeNode.hasAttribute("aria-activedescendant"),
+       "Tree has an active descendant set");
+  }
+
+  const treeNodes = [...treeNode.querySelectorAll(".tree-node")];
+  for (let node of treeNodes) {
+    ok(node.id, "TreeNode has an id");
+    is(node.getAttribute("role"), "treeitem", "Tree item semantics is present");
+    ok(node.hasAttribute("aria-level"), "Aria level attribute is set");
+  }
+}
+
 // Encoding of the following tree/forest:
 //
 // A
 // |-- B
 // |   |-- E
 // |   |   |-- K
 // |   |   `-- L
 // |   |-- F
--- a/devtools/client/shared/components/test/mochitest/test_tree_01.html
+++ b/devtools/client/shared/components/test/mochitest/test_tree_01.html
@@ -26,16 +26,17 @@ window.onload = Task.async(function* () 
     ok(React, "Should get React");
     ok(Tree, "Should get Tree");
 
     const t = Tree(TEST_TREE_INTERFACE);
     ok(t, "Should be able to create Tree instances");
 
     const tree = ReactDOM.render(t, window.document.body);
     ok(tree, "Should be able to mount Tree instances");
+    isAccessibleTree(tree);
 
     TEST_TREE.expanded = new Set("ABCDEFGHIJKLMNO".split(""));
     yield forceRender(tree);
 
     isRenderedTree(document.body.textContent, [
       "A:false",
       "-B:false",
       "--E:false",
--- a/devtools/client/shared/components/test/mochitest/test_tree_02.html
+++ b/devtools/client/shared/components/test/mochitest/test_tree_02.html
@@ -19,16 +19,17 @@ Test that collapsed subtrees aren't rend
 window.onload = Task.async(function* () {
   try {
     let ReactDOM = browserRequire("devtools/client/shared/vendor/react-dom");
     let React = browserRequire("devtools/client/shared/vendor/react");
     let Tree = React.createFactory(browserRequire("devtools/client/shared/components/tree"));
 
     const tree = ReactDOM.render(Tree(TEST_TREE_INTERFACE), window.document.body);
 
+    isAccessibleTree(tree);
     TEST_TREE.expanded = new Set("MNO".split(""));
     yield forceRender(tree);
 
     isRenderedTree(document.body.textContent, [
       "A:false",
       "M:false",
       "-N:false",
       "--O:false",
--- a/devtools/client/shared/components/test/mochitest/test_tree_03.html
+++ b/devtools/client/shared/components/test/mochitest/test_tree_03.html
@@ -21,16 +21,17 @@ window.onload = Task.async(function* () 
     let ReactDOM = browserRequire("devtools/client/shared/vendor/react-dom");
     let React = browserRequire("devtools/client/shared/vendor/react");
     let Tree = React.createFactory(browserRequire("devtools/client/shared/components/tree"));
 
     const tree = ReactDOM.render(Tree(Object.assign({}, TEST_TREE_INTERFACE, {
       autoExpandDepth: 1
     })), window.document.body);
 
+    isAccessibleTree(tree);
     isRenderedTree(document.body.textContent, [
       "A:false",
       "-B:false",
       "-C:false",
       "-D:false",
       "M:false",
       "-N:false",
     ], "Tree should be auto expanded one level");
--- a/devtools/client/shared/components/test/mochitest/test_tree_04.html
+++ b/devtools/client/shared/components/test/mochitest/test_tree_04.html
@@ -37,16 +37,17 @@ window.onload = Task.async(function* () 
 
     TEST_TREE.expanded = new Set("ABCDEFGHIJKLMNO".split(""));
 
     yield setState(tree, {
       height: 3 * ITEM_HEIGHT,
       scroll: 1 * ITEM_HEIGHT
     });
 
+    isAccessibleTree(tree);
     isRenderedTree(document.body.textContent, [
       "A:false",
       "-B:false",
       "--E:false",
       "---K:false",
       "---L:false",
     ], "Tree should show the 2nd, 3rd, and 4th items + buffer of 1 item at each end");
 
@@ -54,16 +55,17 @@ window.onload = Task.async(function* () 
     is(spacers.top, 0, "Top spacer has the correct height");
     is(spacers.bottom, 10 * ITEM_HEIGHT, "Bottom spacer has the correct height");
 
     yield setState(tree, {
       height: 2 * ITEM_HEIGHT,
       scroll: 3 * ITEM_HEIGHT
     });
 
+    isAccessibleTree(tree);
     isRenderedTree(document.body.textContent, [
       "--E:false",
       "---K:false",
       "---L:false",
       "--F:false",
     ], "Tree should show the 4th and 5th item + buffer of 1 item at each end");
 
     spacers = getSpacerHeights();
--- a/devtools/client/shared/components/test/mochitest/test_tree_05.html
+++ b/devtools/client/shared/components/test/mochitest/test_tree_05.html
@@ -30,19 +30,21 @@ window.onload = Task.async(function* () 
         { onFocus: x => renderTree({ focused: x }) },
         props
       );
       return ReactDOM.render(Tree(treeProps), window.document.body);
     }
 
     const tree = renderTree();
 
+    isAccessibleTree(tree);
     TEST_TREE.expanded = new Set("ABCDEFGHIJKLMNO".split(""));
 
     renderTree({ focused: "G" });
+    isAccessibleTree(tree, { hasActiveDescendant: true });
 
     isRenderedTree(document.body.textContent, [
       "A:false",
       "-B:false",
       "--E:false",
       "---K:false",
       "---L:false",
       "--F:false",
--- a/devtools/client/shared/components/test/mochitest/test_tree_06.html
+++ b/devtools/client/shared/components/test/mochitest/test_tree_06.html
@@ -29,16 +29,17 @@ window.onload = Task.async(function* () 
         { onFocus: x => renderTree({ focused: x }) },
         props
       );
       return ReactDOM.render(Tree(treeProps), window.document.body);
     }
 
     const tree = renderTree();
 
+    isAccessibleTree(tree);
     TEST_TREE.expanded = new Set("ABCDEFGHIJKLMNO".split(""));
 
     // UP ----------------------------------------------------------------------
 
     info("Up to the previous sibling.");
     renderTree({ focused: "L" });
     Simulate.keyDown(document.querySelector(".tree"), { key: "ArrowUp" });
     yield forceRender(tree);
--- a/devtools/client/shared/components/tree.js
+++ b/devtools/client/shared/components/tree.js
@@ -193,16 +193,24 @@ module.exports = createClass({
     focused: PropTypes.any,
 
     // Handle when a new item is focused.
     onFocus: PropTypes.func,
 
     // The depth to which we should automatically expand new items.
     autoExpandDepth: PropTypes.number,
 
+    // Note: the two properties below are mutually exclusive. Only one of the
+    // label properties is necessary.
+    // ID of an element whose textual content serves as an accessible label for
+    // a tree.
+    labelledby: PropTypes.string,
+    // Accessibility label for a tree widget.
+    label: PropTypes.string,
+
     // Optional event handlers for when items are expanded or collapsed. Useful
     // for dispatching redux events and updating application state, maybe lazily
     // loading subtrees from a worker, etc.
     //
     // Type:
     //     onExpand(item: Item)
     //     onCollapse(item: Item)
     //
@@ -550,75 +558,103 @@ module.exports = createClass({
     const traversal = this._dfsFromRoots();
 
     // 'begin' and 'end' are the index of the first (at least partially) visible item
     // and the index after the last (at least partially) visible item, respectively.
     // `NUMBER_OF_OFFSCREEN_ITEMS` is removed from `begin` and added to `end` so that
     // the top and bottom of the page are filled with the `NUMBER_OF_OFFSCREEN_ITEMS`
     // previous and next items respectively, which helps the user to see fewer empty
     // gaps when scrolling quickly.
-    const { itemHeight } = this.props;
+    const { itemHeight, focused } = this.props;
     const { scroll, height } = this.state;
     const begin = Math.max(((scroll / itemHeight) | 0) - NUMBER_OF_OFFSCREEN_ITEMS, 0);
     const end = Math.ceil((scroll + height) / itemHeight) + NUMBER_OF_OFFSCREEN_ITEMS;
     const toRender = traversal.slice(begin, end);
     const topSpacerHeight = begin * itemHeight;
     const bottomSpacerHeight = Math.max(traversal.length - end, 0) * itemHeight;
 
     const nodes = [
       dom.div({
         key: "top-spacer",
+        role: "presentation",
         style: {
           padding: 0,
           margin: 0,
           height: topSpacerHeight + "px"
         }
       })
     ];
 
     for (let i = 0; i < toRender.length; i++) {
       const index = begin + i;
       const first = index == 0;
       const last = index == traversal.length - 1;
       const { item, depth } = toRender[i];
+      const key = this.props.getKey(item);
       nodes.push(TreeNode({
-        key: this.props.getKey(item),
+        key,
         index,
         first,
         last,
         item,
         depth,
+        id: key,
         renderItem: this.props.renderItem,
-        focused: this.props.focused === item,
+        focused: focused === item,
         expanded: this.props.isExpanded(item),
         hasChildren: !!this.props.getChildren(item).length,
         onExpand: this._onExpand,
         onCollapse: this._onCollapse,
-        onFocus: () => this._focus(begin + i, item),
-        onFocusedNodeUnmount: () => this.refs.tree && this.refs.tree.focus(),
+        onClick: () => this._focus(begin + i, item),
+        // Focus on the previous node if focused node is unmounted.
+        onFocusedNodeUnmount: () => this._focusPrevNode(),
       }));
     }
 
     nodes.push(dom.div({
       key: "bottom-spacer",
+      role: "presentation",
       style: {
         padding: 0,
         margin: 0,
         height: bottomSpacerHeight + "px"
       }
     }));
 
     return dom.div(
       {
         className: "tree",
         ref: "tree",
+        role: "tree",
+        tabIndex: "0",
         onKeyDown: this._onKeyDown,
         onKeyPress: this._preventArrowKeyScrolling,
         onKeyUp: this._preventArrowKeyScrolling,
         onScroll: this._onScroll,
+        onFocus: ({nativeEvent}) => {
+          if (focused || !nativeEvent || !this.refs.tree) {
+            return;
+          }
+
+          let { explicitOriginalTarget } = nativeEvent;
+          // Only set default focus to the first tree node if the focus came
+          // from outside the tree (e.g. by tabbing to the tree from other
+          // external elements).
+          if (explicitOriginalTarget !== this.refs.tree &&
+              !this.refs.tree.contains(explicitOriginalTarget)) {
+            this._focus(begin, toRender[0].item);
+          }
+        },
+        onClick: () => {
+          // Focus should always remain on the tree container itself.
+          this.refs.tree.focus();
+        },
+        "aria-label": this.props.label,
+        "aria-labelledby": this.props.labelledby,
+        "aria-activedescendant": focused && this.props.getKey(focused),
         style: {
           padding: 0,
           margin: 0
         }
       },
       nodes
     );
   }
@@ -664,74 +700,40 @@ const ArrowExpander = createFactory(crea
     }
 
     return dom.div(attrs);
   }
 }));
 
 const TreeNode = createFactory(createClass({
   propTypes: {
+    id: PropTypes.any.isRequired,
     focused: PropTypes.bool.isRequired,
     onFocusedNodeUnmount: PropTypes.func,
     item: PropTypes.any.isRequired,
     expanded: PropTypes.bool.isRequired,
     hasChildren: PropTypes.bool.isRequired,
     onExpand: PropTypes.func.isRequired,
     index: PropTypes.number.isRequired,
     first: PropTypes.bool,
     last: PropTypes.bool,
-    onFocus: PropTypes.func,
-    onBlur: PropTypes.func,
+    onClick: PropTypes.func,
     onCollapse: PropTypes.func.isRequired,
     depth: PropTypes.number.isRequired,
     renderItem: PropTypes.func.isRequired,
   },
 
-  componentDidMount() {
-    if (this.props.focused) {
-      this.refs.button.focus();
-    }
-  },
-
-  componentDidUpdate() {
+  componentWillUnmount() {
     if (this.props.focused) {
-      this.refs.button.focus();
-    }
-  },
-
-  componentWillUnmount() {
-    // If this node is being destroyed and has focus, transfer the focus manually
-    // to the parent tree component. Otherwise, the focus will get lost and keyboard
-    // navigation in the tree will stop working. This is a workaround for a XUL bug.
-    // See bugs 1259228 and 1152441 for details.
-    // DE-XUL: Remove this hack once all usages are only in HTML documents.
-    if (this.props.focused) {
-      this.refs.button.blur();
       if (this.props.onFocusedNodeUnmount) {
         this.props.onFocusedNodeUnmount();
       }
     }
   },
 
-  _buttonAttrs: {
-    ref: "button",
-    style: {
-      opacity: 0,
-      width: "0 !important",
-      height: "0 !important",
-      padding: "0 !important",
-      outline: "none",
-      MozAppearance: "none",
-      // XXX: Despite resetting all of the above properties (and margin), the
-      // button still ends up with ~79px width, so we set a large negative
-      // margin to completely hide it.
-      MozMarginStart: "-1000px !important",
-    }
-  },
-
   render() {
     const arrow = ArrowExpander({
       item: this.props.item,
       expanded: this.props.expanded,
       visible: this.props.hasChildren,
       onExpand: this.props.onExpand,
       onCollapse: this.props.onCollapse,
     });
@@ -742,39 +744,45 @@ const TreeNode = createFactory(createCla
     }
     if (this.props.first) {
       classList.push("tree-node-first");
     }
     if (this.props.last) {
       classList.push("tree-node-last");
     }
 
+    let ariaExpanded;
+    if (this.props.hasChildren) {
+      ariaExpanded = false;
+    }
+    if (this.props.expanded) {
+      ariaExpanded = true;
+    }
+
     return dom.div(
       {
+        id: this.props.id,
         className: classList.join(" "),
-        onFocus: this.props.onFocus,
-        onClick: this.props.onFocus,
-        onBlur: this.props.onBlur,
+        role: "treeitem",
+        "aria-level": this.props.depth,
+        onClick: this.props.onClick,
+        "aria-expanded": ariaExpanded,
         "data-expanded": this.props.expanded ? "" : undefined,
         "data-depth": this.props.depth,
         style: {
           padding: 0,
           margin: 0
         }
       },
 
       this.props.renderItem(this.props.item,
                             this.props.depth,
                             this.props.focused,
                             arrow,
                             this.props.expanded),
-
-      // XXX: OSX won't focus/blur regular elements even if you set tabindex
-      // unless there is an input/button child.
-      dom.button(this._buttonAttrs)
     );
   }
 }));
 
 /**
  * Create a function that calls the given function `fn` only once per animation
  * frame.
  *