Enable keyboard shortcuts for chrome://extensions

chrome/browser/resources/extensions/extension_error.js

Index: chrome/browser/resources/extensions/extension_error.js
diff --git a/chrome/browser/resources/extensions/extension_error.js b/chrome/browser/resources/extensions/extension_error.js
index 2452b12f8aaa2cade0966ddc34310b4bad681033..82b4d566f8d057f52a2c3e41067cf457dfb7477b 100644
--- a/chrome/browser/resources/extensions/extension_error.js
+++ b/chrome/browser/resources/extensions/extension_error.js
@@ -28,24 +28,35 @@ cr.define('extensions', function() {
   /**
    * Creates a new ExtensionError HTMLElement; this is used to show a
    * notification to the user when an error is caused by an extension.
-   * @param {Object} error The error the element should represent.
+   * @param {RuntimeError} error The error the element should represent.
    * @constructor
-   * @extends {HTMLDivElement}
+   * @extends {cr.ui.FocusRow}
    */
-  function ExtensionError(error) {
+  function ExtensionError(error, boundary) {
     var div = cloneTemplate('extension-error-metadata');
     div.__proto__ = ExtensionError.prototype;
-    div.decorate(error);
+    div.decorate(error, boundary);
     return div;
   }
 
   ExtensionError.prototype = {
-    __proto__: HTMLDivElement.prototype,
+    __proto__: cr.ui.FocusRow.prototype,
+
+    /** @override */
+    getEquivalentElement: function(element) {
+      return assertInstanceof(
+          this.querySelector('.extension-error-view-details'), Element);
+    },
 
     /**
-     * @param {RuntimeError} error
+     * @param {RuntimeError} error The error the element should represent
+     * @param {Element} boundary The boundary for the FocusGrid.
+     * @override
      */
-    decorate: function(error) {
+    decorate: function(error, boundary) {
+      cr.ui.FocusRow.prototype.decorate.call(
+          assertInstanceof(this, cr.ui.FocusRow), boundary);

[Dan Beam, 2015/02/19 19:26:27]

why are you using assertInstanceOf() here?  have we been bitten by this or is
the compiler complaining?

[hcarmona, 2015/02/19 23:42:16]

This was left over from before I updated the @extends and I forgot to remove it.
It's not necessary.

+
       // Add an additional class for the severity level.
       if (error.level == 0)
         this.classList.add('extension-error-severity-info');
@@ -79,6 +90,8 @@ cr.define('extensions', function() {
           extensions.ExtensionErrorOverlay.getInstance().setErrorAndShowOverlay(
               error, extensionUrl);
         });
+
+        this.addFocusableElement(viewDetailsLink);
       }
     },
   };
@@ -109,11 +122,17 @@ cr.define('extensions', function() {
     __proto__: HTMLDivElement.prototype,
 
     decorate: function() {
+      this.focusGrid_ = new cr.ui.FocusGrid();
       this.contents_ = this.querySelector('.extension-error-list-contents');
+      this.contents_.addEventListener('focus', this.focusContents_.bind(this));

[Dan Beam, 2015/02/19 19:26:27]

onFocus_ or onFocusContents_

[hcarmona, 2015/02/19 23:42:17]

Done.

+      this.contents_.addEventListener('focusin',
+                                      this.focusinContents_.bind(this));

[Dan Beam, 2015/02/19 19:26:27]

onFocusin_ or onFocusinContents_

[hcarmona, 2015/02/19 23:42:16]

Done.

       this.errors_.forEach(function(error) {
         if (idIsValid(error.extensionId)) {
+          var focusRow = new ExtensionError(error, this.contents_);
           this.contents_.appendChild(document.createElement('li')).appendChild(
-              new ExtensionError(error));
+              focusRow);
+          this.focusGrid_.addRow(focusRow);
         }
       }, this);
 
@@ -123,6 +142,70 @@ cr.define('extensions', function() {
     },
 
     /**
+     * @return {?Element} The "Show more" element or null if it's hidden.
+     */
+    getToggleElement: function() {
+      var more = this.querySelector(
+          '.extension-error-list-show-more [is="action-link"]');

[Dan Beam, 2015/02/19 19:26:28]

return this.querySelector(
    '.extension-error-list-show-more [is="action-link"]:not([hidden])');

[hcarmona, 2015/02/19 23:42:17]

Done.

+      if (!more.hidden)
+        return more;
+      return null;
+    },
+
+    /**
+     * @return {Element} The boundary for the FocusGrid.

[Dan Beam, 2015/02/19 19:26:28]

nit: 1-line -> /** ... */

[hcarmona, 2015/02/19 23:42:16]

Done.

+     */
+    getListElement: function() {

[Dan Beam, 2015/02/19 19:26:28]

getFocusGridBoundary?

[hcarmona, 2015/02/19 23:42:16]

Updated doc. Only this class needs to know it's the boundary of the grid.
Classes outside only need to know it's the element with a list of errors.

+      return this.contents_;
+    },
+
+    /**
+     * This element should not be focusable once this element or an element
+     * inside is it is focused. This is necessary to allow tabbing out of the
+     * grid in reverse.

[Dan Beam, 2015/02/19 19:26:28]

rewrite doc comment without using "this" as much (no idea what you're talking
about without reading whole file)

[hcarmona, 2015/02/19 23:42:17]

Done.

+     * @private
+     */
+    focusinContents_: function() {
+      this.contents_.tabIndex = -1;
+    },
+
+    /**
+     * Focus the first focusable row when tabbing into the grid for the
+     * first time.
+     * @private
+     */
+    focusContents_: function() {
+      var activeRow = this.contents_.querySelector('.focus-row-active');
+      var toggleButton = this.getToggleElement();
+
+      if (!toggleButton || toggleButton.isShowingAll) {
+        if (activeRow)

[Dan Beam, 2015/02/19 19:26:27]

activeRow = activeRow ||
    this.contents_.querySelector('.extension-error-metadata');
activeRow.getEquivalentElement(null).focus();

[hcarmona, 2015/02/19 23:42:17]

Done.

+          activeRow.getEquivalentElement(null).focus();
+        else {

[Dan Beam, 2015/02/19 19:26:28]

conditionals can either have 0 curlies or ALL curlies.  add more here

[hcarmona, 2015/02/19 23:42:17]

Done.

+          activeRow = this.contents_.querySelector('.extension-error-metadata');

[Dan Beam, 2015/02/19 19:26:28]

why doesn't focusgrid have a way of accessing rows?

[hcarmona, 2015/02/19 23:42:16]

It does! :D And using that simplifies this code significantly. Done.

+          activeRow.getEquivalentElement(null).focus();
+        }
+      } else {
+        var firstVisibleChildIndex = this.contents_.children.length -
+                                     ExtensionErrorList.MAX_ERRORS_TO_SHOW_;
+
+        for (var i = firstVisibleChildIndex; i < this.contents_.children.length;
+            ++i) {
+          var child = this.contents_.children[i].querySelector(
+              '.extension-error-metadata');
+          if (child == activeRow) {
+            activeRow.getEquivalentElement(null).focus();
+            return;
+          }
+        }
+
+        activeRow = this.contents_.children[firstVisibleChildIndex].
+          querySelector('.extension-error-metadata');

[Dan Beam, 2015/02/19 19:26:28]

wrong indent

[hcarmona, 2015/02/19 23:42:16]

Done.

+        activeRow.getEquivalentElement(null).focus();
+      }
+    },
+
+    /**
      * Initialize the "Show More" link for the error list. If there are more
      * than |MAX_ERRORS_TO_SHOW_| errors in the list.
      * @private
@@ -144,6 +227,9 @@ cr.define('extensions', function() {
       });
 
       link.addEventListener('click', function(e) {
+        // Needs to be enabled in case the focused row is now hidden.
+        this.contents_.tabIndex = 0;
+
         link.isShowingAll = !link.isShowingAll;
 
         var message = link.isShowingAll ? 'extensionErrorsShowFewer' :