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 a 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,

[Ivan Korotkov, 2012/08/03 01:00:23]

Don't put them into prototype: that makes them shared across all List objects.
Instead, you can assign them in constructor or on the first usage at all.

[dmazzoni, 2012/08/03 22:59:01]

Moved to decorate(), or did you have something else in mind?

[Ivan Korotkov, 2012/08/03 23:19:37]

decorate is OK

+
+    /**
      * 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 208 matching lines @@
       var length = this.dataModel ? this.dataModel.length : 0;
       this.selectionModel = new ListSelectionModel(length);
 
       this.addEventListener('dblclick', this.handleDoubleClickOrTap_);
       this.addEventListener('mousedown', this.handlePointerDownUp_);
       this.addEventListener('mouseup', this.handlePointerDownUp_);
       this.addEventListener('keydown', this.handleKeyDown);
       this.addEventListener('focus', this.handleElementFocus_, true);
       this.addEventListener('blur', this.handleElementBlur_, true);
       this.addEventListener('scroll', this.handleScroll.bind(this));
-      this.setAttribute('role', 'listbox');
+      this.setAttribute('role', 'list');
 
       this.touchHandler_ = new cr.ui.TouchHandler(this);
       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 230 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)
-          listItem.selected = change.selected;
+          if (listItem) {
+            listItem.selected = change.selected;
+            if (change.selected)
+              this.setAttribute('aria-activedescendant', listItem.id);

[Ivan Korotkov, 2012/08/03 01:00:23]

Indentation.

[dmazzoni, 2012/08/03 22:59:01]

Done.

+          }
       }, 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_++;
       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
   };
 });