Improve accessibility of user image selection screen.

chrome/browser/resources/shared/js/cr/ui/list.js

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // require: array_data_model.js
 // require: list_selection_model.js
 // require: list_selection_controller.js
 // require: list_item.js
 
 /**
@@ skipping 73 matching lines @@
     remainingSpace_: true,
 
     /**
      * Function used to create grid items.
      * @type {function(): !ListItem}
      * @private
      */
     itemConstructor_: cr.ui.ListItem,
 
     /**
+     * The prefix to use when giving each item an unique id.
+     * @type {string}
+     * @private
+     */
+    uniqueIdPrefix_: 'list',
+
+    /**
+     * The next id suffix to use when giving each item an unique id.
+     * @type {number}
+     * @private
+     */
+    nextUniqueIdSuffix_: 0,
+
+    /**
      * Function used to create grid items.
      * @type {function(): !ListItem}
      */
     get itemConstructor() {
       return this.itemConstructor_;
     },
     set itemConstructor(func) {
       if (func != this.itemConstructor_) {
         this.itemConstructor_ = func;
         this.cachedItems_ = {};
@@ skipping 221 matching lines @@
       this.addEventListener(cr.ui.TouchHandler.EventType.TOUCH_START,
                             this.handlePointerDownUp_);
       this.addEventListener(cr.ui.TouchHandler.EventType.TOUCH_END,
                             this.handlePointerDownUp_);
       this.addEventListener(cr.ui.TouchHandler.EventType.TAP,
                             this.handleDoubleClickOrTap_);
       this.touchHandler_.enable(false, false);
       // Make list focusable
       if (!this.hasAttribute('tabindex'))
         this.tabIndex = 0;
+
+      // Try to get an unique id prefix from the id of this element or the
+      // nearest ancestor with an id.
+      var element = this;
+      while (element && !element.id)
+        element = element.parentElement;
+      if (element && element.id)
+        this.uniqueIdPrefix_ = element.id;
     },
 
     /**
      * @param {ListItem=} item The list item to measure.
      * @return {number} The height of the given item. If the fixed height on CSS
      * is set by 'px', uses that value as height. Otherwise, measures the size.
      * @private
      */
     measureItemHeight_: function(item) {
       var height = item.style.height;
@@ skipping 221 matching lines @@
 
     /**
      * Callback from the selection model. We dispatch {@code change} events
      * when the selection changes.
      * @param {!cr.Event} e Event with change info.
      * @private
      */
     handleOnChange_: function(ce) {
       ce.changes.forEach(function(change) {
         var listItem = this.getListItemByIndex(change.index);
-        if (listItem)
+        if (listItem) {
           listItem.selected = change.selected;
+          if (change.selected)
+            this.setAttribute('aria-activedescendant', listItem.id);
+        }
       }, this);
 
       cr.dispatchSimpleEvent(this, 'change');
     },
 
     /**
      * Handles a change of the lead item from the selection model.
      * @param {Event} pe The property change event.
      * @private
      */
@@ skipping 212 matching lines @@
     },
 
     /**
      * Creates a new list item.
      * @param {*} value The value to use for the item.
      * @return {!ListItem} The newly created list item.
      */
     createItem: function(value) {
       var item = new this.itemConstructor_(value);
       item.label = value;
+      item.id = this.uniqueIdPrefix_ + '-' + this.nextUniqueIdSuffix_;

[Ivan Korotkov, 2012/05/04 17:02:27]

Please make assigning unique IDs to each element optional, so that this doesn't
affect other lists.

[dmazzoni, 2012/05/04 17:48:00]

I don't want to make it optional, then we'll have to explicitly enable it to
make any other list accessible. This change should improve accessibility for
other lists.

The unique id prefix is derived from the id of the nearest ancestor, so it's
extremely unlikely two lists will end up with the same id. If they did, it's
unlikely it'd break anything other than accessibility.

The only other option I see is to make nextUniqueIdSuffix a class variable (i.e.
a global) so that ids are never shared between list items.

[Ivan Korotkov, 2012/05/04 22:03:33]

Ok, I guess these IDs won't hurt.

+      this.nextUniqueIdSuffix_++;
+
       if (typeof item.decorate == 'function')
         item.decorate();
       return item;
     },
 
     /**
      * Creates the selection controller to use internally.
      * @param {cr.ui.ListSelectionModel} sm The underlying selection model.
      * @return {!cr.ui.ListSelectionController} The newly created selection
      *     controller.
@@ skipping 436 matching lines @@
    * because list items can contain controls that can be focused, and for some
    * purposes (e.g., styling), the list can still be conceptually focused at
    * that point even though it doesn't actually have the page focus.
    */
   cr.defineProperty(List, 'hasElementFocus', cr.PropertyKind.BOOL_ATTR);
 
   return {
     List: List
   };
 });