Make extension error overlay remotely keyboard accessible

chrome/browser/resources/extensions/extension_error.js

 // Copyright 2013 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.
 
 cr.define('extensions', function() {
   'use strict';
 
   /**
    * Clone a template within the extension error template collection.
    * @param {string} templateName The class name of the template to clone.
@@ skipping 41 matching lines @@
     div.__proto__ = ExtensionError.prototype;
     div.decorateWithError_(error, boundary);
     return div;
   }
 
   ExtensionError.prototype = {
     __proto__: cr.ui.FocusRow.prototype,
 
     /** @override */
     getEquivalentElement: function(element) {
-      if (element.classList.contains('extension-error-metadata'))
-        return this;
-      if (element.classList.contains('error-delete-button')) {
-        return /** @type {!HTMLElement} */ (this.querySelector(
-            '.error-delete-button'));
-      }
-      assertNotReached();
-      return element;
+      return element.classList.contains('error-delete-button') ?
+          this.deleteButton_ : this.messageSpan_;

[Dan Beam, 2015/08/11 02:46:54]

returning a default within |this| seems like a better practice

     },
 
     /**
      * @param {(RuntimeError|ManifestError)} error The error the element should
      *     represent.
      * @param {Element} boundary The boundary for the FocusGrid.
      * @private
      */
     decorateWithError_: function(error, boundary) {
       this.decorate(boundary);
@@ skipping 24 matching lines @@
         this.classList.add('extension-error-severity-warning');
       }
 
       var iconNode = document.createElement('img');
       iconNode.className = 'extension-error-icon';
       // TODO(hcarmona): Populate alt text with a proper description since this
       // icon conveys the severity of the error. (info, warning, fatal).
       iconNode.alt = '';
       this.insertBefore(iconNode, this.firstChild);
 
-      var messageSpan = this.querySelector('.extension-error-message');
-      messageSpan.textContent = error.message;
+      this.messageSpan_ = this.querySelector('.extension-error-message');
+      this.messageSpan_.textContent = error.message;
 
-      var deleteButton = this.querySelector('.error-delete-button');
-      deleteButton.addEventListener('click', function(e) {

[Dan Beam, 2015/08/11 02:46:54]

^ this never worked for keyboard events

+      this.deleteButton_ = this.querySelector('.error-delete-button');
+      this.deleteButton_.addEventListener('click', function(e) {
         this.dispatchEvent(
             new CustomEvent('deleteExtensionError',
                             {bubbles: true, detail: this.error}));
       }.bind(this));
 
       this.addEventListener('click', function(e) {
-        if (e.target != deleteButton)
+        if (e.target != this.deleteButton_)
           this.requestActive_();
       }.bind(this));
       this.addEventListener('keydown', function(e) {
-        if (e.keyIdentifier == 'Enter' && e.target != deleteButton)
+        if (e.keyIdentifier == 'Enter' && e.target != this.deleteButton_)
           this.requestActive_();
       });
 
-      this.addFocusableElement(this);
-      this.addFocusableElement(deleteButton);
+      this.addFocusableElement(this.messageSpan_);
+      this.addFocusableElement(this.deleteButton_);
     },
 
     /**
      * Bubble up an event to request to become active.
      * @private
      */
     requestActive_: function() {
       this.dispatchEvent(
           new CustomEvent('highlightExtensionError',
                           {bubbles: true, detail: this.error}));
@@ skipping 27 matching lines @@
       /**
        * @private {!Array<(ManifestError|RuntimeError)>}
        */
       this.errors_ = [];
 
       this.focusGrid_ = new cr.ui.FocusGrid();
       this.listContents_ = this.querySelector('.extension-error-list-contents');
 
       errors.forEach(this.addError_, this);
 
+      this.focusGrid_.ensureRowActive();

[Dan Beam, 2015/08/11 02:46:53]

this was never called initially

+
       this.addEventListener('highlightExtensionError', function(e) {
         this.setActiveErrorNode_(e.target);
       });
       this.addEventListener('deleteExtensionError', function(e) {
         this.removeError_(e.detail);
       });
 
       this.querySelector('#extension-error-list-clear').addEventListener(
           'click', function(e) {
         this.clear(true);
@@ skipping 51 matching lines @@
           break;
       }
       assert(index != this.errors_.length);
       var errorList = this.querySelector('.extension-error-list-contents');
 
       var wasActive =
           this.activeError_ && this.activeError_.error.id == error.id;
 
       this.errors_.splice(index, 1);
       var listElement = errorList.children[index];
+
+      var focusRow = assertInstanceof(listElement, ExtensionError);

[Dan Beam, 2015/08/11 02:46:54]

nodes were removed from the DOM but not from |this.focusGrid_|

+      this.focusGrid_.removeRow(focusRow);
+      this.focusGrid_.ensureRowActive();
+
+      // TODO(dbeam): in a world where this UI is actually used, we should
+      // probably move the focus before removing |listElement|.
       listElement.parentNode.removeChild(listElement);
 
       if (wasActive) {
         index = Math.min(index, this.errors_.length - 1);
         this.setActiveError(index);  // Gracefully handles the -1 case.
       }
 
       chrome.developerPrivate.deleteExtensionErrors({
         extensionId: error.extensionId,
         errorIds: [error.id]
@@ skipping 90 matching lines @@
       this.dispatchEvent(
           new CustomEvent('activeExtensionErrorChanged',
                           {bubbles: true, detail: node ? node.error : null}));
     },
   };
 
   return {
     ExtensionErrorList: ExtensionErrorList
   };
 });