Enable keyboard shortcuts for chrome://extensions

chrome/browser/resources/extensions/extension_list.js

Index: chrome/browser/resources/extensions/extension_list.js
diff --git a/chrome/browser/resources/extensions/extension_list.js b/chrome/browser/resources/extensions/extension_list.js
index d4fcbf6babcd7bc2401013600cdcb86a2872a8c5..1ff5d98e7aaf25b571bbcb1517bf109c84046515 100644
--- a/chrome/browser/resources/extensions/extension_list.js
+++ b/chrome/browser/resources/extensions/extension_list.js
@@ -65,6 +65,121 @@
  */
 var ExtensionData;
 
+///////////////////////////////////////////////////////////////////////////////
+// ExtensionFocusRow:
+
+/**
+ * Provides an implementation for a single column grid.
+ * @constructor
+ * @extends {cr.ui.FocusRow}
+ */
+function ExtensionFocusRow() {}
+
+/**
+ * Decorates |focusRow| so that it can be treated as a ExtensionFocusRow.
+ * @param {Element} focusRow The element that has all the columns.
+ * @param {Node} boundary Focus events are ignored outside of this node.
+ */
+ExtensionFocusRow.decorate = function(focusRow, boundary) {
+  focusRow.__proto__ = ExtensionFocusRow.prototype;
+  focusRow.decorate(boundary);
+};
+
+ExtensionFocusRow.prototype = {
+  __proto__: cr.ui.FocusRow.prototype,
+
+  /** @override */
+  getEquivalentElement: function(element) {
+    if (this.focusableElements.indexOf(element) > -1)
+      return element;
+
+    // All elements default to another element with the same type.
+    var columnType = element.getAttribute('column-type');
+    var equivalent = this.querySelector('[column-type=' + columnType + ']');
+
+    if (!equivalent || !this.canAddElement_(equivalent)) {
+      var actionLinks = ['options', 'website', 'launch', 'localReload'];
+      if (actionLinks.indexOf(columnType) > -1)
+        equivalent = this.getFirstFocusableByType_(actionLinks);

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

why are we potentially overwriting |equivalent| 4 times after it's set?  should
these all be `else if` more or less?

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

Only one should match. Changed to else if.

+
+      var optionalControls = ['showButton', 'incognito', 'dev-collectErrors',
+                              'allUrls', 'localUrls'];
+      if (optionalControls.indexOf(columnType) > -1)
+        equivalent = this.getFirstFocusableByType_(optionalControls);
+
+      var removeStyleButtons = ['trash', 'enterprise'];
+      if (removeStyleButtons.indexOf(columnType) > -1)
+        equivalent = this.getFirstFocusableByType_(removeStyleButtons);
+
+      var enableControls = ['terminatedReload', 'repair', 'enabled'];
+      if (enableControls.indexOf(columnType) > -1)
+        equivalent = this.getFirstFocusableByType_(enableControls);
+    }
+
+    // Return the first focusable element if no equivalent type is found.
+    return equivalent || this.focusableElements[0];
+  },
+
+  /** Updates the list of focusable elements. */
+  updateFocusableElements: function() {
+    this.focusableElements.length = 0;
+
+    var focusableCandidates = this.querySelectorAll('[column-type]');
+    for (var i = 0; i < focusableCandidates.length; ++i) {
+      var element = focusableCandidates[i];
+      if (this.canAddElement_(element))
+        this.addFocusableElement(element);
+    }
+  },
+
+  /**
+   * Get the first focusable element that matches a list of types.
+   * @param {Array<string>} types An array of types to match from.
+   * @return {?Element} Return the first element that matches a type in |types|.
+   * @private
+   */
+  getFirstFocusableByType_: function(types) {
+    for (var i = 0; i < this.focusableElements.length; ++i) {
+      var element = this.focusableElements[i];
+      if (types.indexOf(element.getAttribute('column-type')) > -1)
+        return element;
+    }
+    return null;
+  },
+
+  /**
+   * @param {Element} element
+   * @return {boolean}
+   * @private
+   */
+  canAddElement_: function(element) {
+    if (!element || element.disabled)
+      return false;
+
+    var developerMode = $('extension-settings').classList.contains('dev-mode');
+    if (this.isDeveloperOption_(element) && !developerMode)
+      return false;
+
+    for (var el = element; el; el = el.parentElement) {
+      if (el.hidden)
+        return false;
+    }
+
+    return true;
+  },
+
+  /**
+   * Returns true if the element should only be shown in developer mode.
+   * @param {Element} element
+   * @return {boolean}
+   * @private
+   */
+  isDeveloperOption_: function(element) {
+    var type = element.getAttribute('column-type');
+    return type.indexOf('dev-') == 0;

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

this can fit in one line as:

  return element.getAttribute('column-type').indexOf('dev-') == 0;

OR

  return /^dev-/.test(element.getAttribute('column-type'));

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

Done.

+  },
+};
+
 cr.define('options', function() {
   'use strict';
 
@@ -96,6 +211,9 @@ cr.define('options', function() {
      */
     optionsShown_: false,
 
+    /** @private {cr.ui.FocusGrid} */

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

nit: !cr.ui.FocusGrid

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

Done.

+    focusGrid_: new cr.ui.FocusGrid(),
+
     /**
      * Necessary to only show the butterbar once.
      * @private {boolean}
@@ -115,10 +233,13 @@ cr.define('options', function() {
     },
 
     /**
-     * Creates all extension items from scratch.
+     * Creates or updates all extension items from scratch.
      * @private
      */
     showExtensionNodes_: function() {
+      // Remove the rows from |focusGrid_| without destroying them.
+      this.focusGrid_.rows.length = 0;
+
       // Any node that is not updated will be removed.
       var seenIds = [];
 
@@ -133,12 +254,18 @@ cr.define('options', function() {
           this.createNode_(extension);
       }, this);
 
-      // Remove extensions that are no longer installed.
+      // Remove extensions that are no longer installed or add them to
+      // |focusGrid_| if they are still installed.
       var nodes = document.querySelectorAll('.extension-list-item-wrapper[id]');
       for (var i = 0; i < nodes.length; ++i) {
         var node = nodes[i];
-        if (seenIds.indexOf(node.id) < 0)
+        if (seenIds.indexOf(node.id) < 0) {
           node.parentElement.removeChild(node);
+          // Unregister the removed node from events.
+          node.destroy();
+        } else {
+          this.focusGrid_.addRow(node);
+        }
       }
 
       var idToHighlight = this.getIdQueryParam_();
@@ -153,6 +280,14 @@ cr.define('options', function() {
       this.classList.toggle('empty-extension-list', noExtensions);
     },
 
+    /** Updates each row's focusable elements without rebuilding the grid. */
+    updateFocusableElements: function() {
+      var nodes = document.querySelectorAll('.extension-list-item-wrapper[id]');

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

nit: nodes -> rows

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

Done.

+      for (var i = 0; i < nodes.length; ++i) {
+        nodes[i].updateFocusableElements();
+      }
+    },
+
     /**
      * Scrolls the page down to the extension node with the given id.
      * @param {string} extensionId The id of the extension to scroll to.
@@ -179,15 +314,17 @@ cr.define('options', function() {
       var template = $('template-collection').querySelector(
           '.extension-list-item-wrapper');
       var node = template.cloneNode(true);
+      ExtensionFocusRow.decorate(node, $('extension-settings-list'));
       node.id = extension.id;
 
       // The 'Show Browser Action' button.
-      this.addListener_('click', node, '.show-button', function(e) {
+      this.addListener_('click', node, '.show-button', 'showButton',
+                        function(e) {
         chrome.send('extensionSettingsShowButton', [extension.id]);
       });
 
       // The 'allow in incognito' checkbox.
-      this.addListener_('change', node, '.incognito-control input',
+      this.addListener_('change', node, '.incognito-control input', 'incognito',
                         function(e) {
         var butterBar = node.querySelector('.butter-bar');
         var checked = e.target.checked;
@@ -204,7 +341,7 @@ cr.define('options', function() {
       // error console is enabled - we can detect this by the existence of the
       // |errorCollectionEnabled| property.
       this.addListener_('change', node, '.error-collection-control input',
-                        function(e) {
+                        'dev-collectErrors', function(e) {
         chrome.send('extensionSettingsEnableErrorCollection',
                     [extension.id, String(e.target.checked)]);
       });
@@ -212,13 +349,15 @@ cr.define('options', function() {
       // The 'allow on all urls' checkbox. This should only be visible if
       // active script restrictions are enabled. If they are not enabled, no
       // extensions should want all urls.
-      this.addListener_('click', node, '.all-urls-control', function(e) {
+      this.addListener_('click', node, '.all-urls-control input', 'allUrls',
+                        function(e) {
         chrome.send('extensionSettingsAllowOnAllUrls',
                     [extension.id, String(e.target.checked)]);
       });
 
       // The 'allow file:// access' checkbox.
-      this.addListener_('click', node, '.file-access-control', function(e) {
+      this.addListener_('click', node, '.file-access-control input',
+                        'localUrls', function(e) {
         chrome.send('extensionSettingsAllowFileAccess',
                     [extension.id, String(e.target.checked)]);
       });
@@ -228,45 +367,54 @@ cr.define('options', function() {
       // footer) - but the actual link opening is done through chrome.send
       // with a preventDefault().
       node.querySelector('.options-link').href = extension.optionsPageHref;
-      this.addListener_('click', node, '.options-link', function(e) {
+      this.addListener_('click', node, '.options-link', 'options', function(e) {
         chrome.send('extensionSettingsOptions', [extension.id]);
         e.preventDefault();
       });
 
-      this.addListener_('click', node, '.options-button', function(e) {
+      this.addListener_('click', node, '.options-button', 'options',
+                        function(e) {
         this.showEmbeddedExtensionOptions_(extension.id, false);
         e.preventDefault();
       }.bind(this));
 
+      // The 'View in Web Store/View Web Site' link.
+      node.querySelector('.site-link').setAttribute('column-type', 'website');
+
       // The 'Permissions' link.
-      this.addListener_('click', node, '.permissions-link', function(e) {
+      this.addListener_('click', node, '.permissions-link', 'details',
+                        function(e) {
         chrome.send('extensionSettingsPermissions', [extension.id]);
         e.preventDefault();
       });
 
       // The 'Reload' link.
-      this.addListener_('click', node, '.reload-link', function(e) {
+      this.addListener_('click', node, '.reload-link', 'localReload',
+                        function(e) {
         chrome.send('extensionSettingsReload', [extension.id]);
         extensionReloadedTimestamp[extension.id] = Date.now();
       });
 
       // The 'Launch' link.
-      this.addListener_('click', node, '.launch-link', function(e) {
+      this.addListener_('click', node, '.launch-link', 'launch', function(e) {
         chrome.send('extensionSettingsLaunch', [extension.id]);
       });
 
       // The 'Reload' terminated link.
-      this.addListener_('click', node, '.terminated-reload-link', function(e) {
+      this.addListener_('click', node, '.terminated-reload-link',
+                        'terminatedReload', function(e) {
         chrome.send('extensionSettingsReload', [extension.id]);
       });
 
       // The 'Repair' corrupted link.
-      this.addListener_('click', node, '.corrupted-repair-button', function(e) {
+      this.addListener_('click', node, '.corrupted-repair-button', 'repair',
+                        function(e) {
         chrome.send('extensionSettingsRepair', [extension.id]);
       });
 
       // The 'Enabled' checkbox.
-      this.addListener_('change', node, '.enable-checkbox input', function(e) {
+      this.addListener_('change', node, '.enable-checkbox input', 'enabled',
+                        function(e) {
         var checked = e.target.checked;
         chrome.send('extensionSettingsEnable', [extension.id, String(checked)]);
 
@@ -282,6 +430,8 @@ cr.define('options', function() {
       var trashTemplate = $('template-collection').querySelector('.trash');
       var trash = trashTemplate.cloneNode(true);
       trash.title = loadTimeData.getString('extensionUninstall');
+      trash.hidden = extension.managedInstall;
+      trash.setAttribute('column-type', 'trash');
       trash.addEventListener('click', function(e) {
         chrome.send('extensionSettingsUninstall', [extension.id]);
       });
@@ -291,7 +441,7 @@ cr.define('options', function() {
 
       // The path, if provided by unpacked extension.
       this.addListener_('click', node, '.load-path a:first-of-type',
-                        function(e) {
+                        'dev-loadPath', function(e) {
         chrome.send('extensionSettingsShowPath', [String(extension.id)]);
         e.preventDefault();
       });
@@ -307,8 +457,9 @@ cr.define('options', function() {
      * @private
      */
     updateNode_: function(extension, node) {
-      node.classList.toggle('inactive-extension',
-                            !extension.enabled || extension.terminated);
+      var isActive = extension.enabled && !extension.terminated;
+
+      node.classList.toggle('inactive-extension', !isActive);
 
       node.classList.remove('policy-controlled', 'may-not-modify',
                             'may-not-remove');
@@ -350,11 +501,12 @@ cr.define('options', function() {
 
       // The 'Show Browser Action' button.
       this.updateVisibility_(node, '.show-button',
-                             extension.enable_show_button);
+                             isActive && extension.enable_show_button);
 
       // The 'allow in incognito' checkbox.
       this.updateVisibility_(node, '.incognito-control',
-                             this.data_.incognitoAvailable, function(item) {
+                             isActive && this.data_.incognitoAvailable,
+                             function(item) {
         var incognito = item.querySelector('input');
         incognito.disabled = !extension.incognitoCanBeEnabled;
         incognito.checked = extension.enabledIncognito;
@@ -368,21 +520,23 @@ cr.define('options', function() {
       // error console is enabled - we can detect this by the existence of the
       // |errorCollectionEnabled| property.
       this.updateVisibility_(node, '.error-collection-control',
-                             extension.wantsErrorCollection, function(item) {
+                             isActive && extension.wantsErrorCollection,
+                             function(item) {
         item.querySelector('input').checked = extension.errorCollectionEnabled;
       });
 
       // The 'allow on all urls' checkbox. This should only be visible if
       // active script restrictions are enabled. If they are not enabled, no
       // extensions should want all urls.
-      this.updateVisibility_(node, '.all-urls-control', extension.showAllUrls,
-                             function(item) {
+      this.updateVisibility_(node, '.all-urls-control',
+                             isActive && extension.showAllUrls, function(item) {
         item.querySelector('input').checked = extension.allowAllUrls;
       });
 
       // The 'allow file:// access' checkbox.
       this.updateVisibility_(node, '.file-access-control',
-                             extension.wantsFileAccess, function(item) {
+                             isActive && extension.wantsFileAccess,
+                             function(item) {
         item.querySelector('input').checked = extension.allowFileAccess;
       });
 
@@ -446,6 +600,8 @@ cr.define('options', function() {
         indicator.setAttribute('textpolicy', textPolicy);
         indicator.image.setAttribute('aria-label', textPolicy);
         controlNode.appendChild(indicator);
+        indicator.querySelector('div').setAttribute('column-type',
+                                                    'enterprise');
       } else if (!needsIndicator && indicator) {
         controlNode.removeChild(indicator);
       }
@@ -540,6 +696,11 @@ cr.define('options', function() {
             item.appendChild(link);
           }
         });
+
+        var allLinks = item.querySelectorAll('a');
+        for (var i = 0; i < allLinks.length; ++i) {
+          allLinks[i].setAttribute('column-type', 'dev-activeViews' + i);
+        }
       });
 
       // The extension warnings (describing runtime issues).
@@ -558,9 +719,9 @@ cr.define('options', function() {
       // both ErrorConsole errors and install warnings.
       // Errors.
       this.updateErrors_(node.querySelector('.manifest-errors'),
-                         extension.manifestErrors);
+                         'dev-manifestErrors', extension.manifestErrors);
       this.updateErrors_(node.querySelector('.runtime-errors'),
-                         extension.runtimeErrors);
+                         'dev-runtimeErrors', extension.runtimeErrors);
 
       // Install warnings.
       this.updateVisibility_(node, '.install-warnings',
@@ -585,6 +746,8 @@ cr.define('options', function() {
           topScroll -= pad / 2;
         setScrollTopForDocument(document, topScroll);
       }
+
+      node.updateFocusableElements();

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

every time I see node#focusRowFunction() my eyes are sad.  if this is a
focusrow, it should be row, not node (which sounds like Node, which does not
have updateFocusableElements).

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

Done.

     },
 
     /**
@@ -592,12 +755,16 @@ cr.define('options', function() {
      * @param {string} type The type of listener to add.
      * @param {Element} node Ancestor of the element specified by |query|.
      * @param {string} query A query to select an element in |node|.
+     * @param {string} columnType A tag used to identify the column when
+     *     changing focus.
      * @param {function(Event)} handler The function that should be called
      *     on click.
      * @private
      */
-    addListener_: function(type, node, query, handler) {
-      node.querySelector(query).addEventListener(type, handler);
+    addListener_: function(type, node, query, columnType, handler) {
+      var element = node.querySelector(query);
+      element.addEventListener(type, handler);
+      element.setAttribute('column-type', columnType);

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

this is an unexpected side effect in "addListener_" IMO.  split this into 2
methods or rename

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

Done.

     },
 
     /**
@@ -632,17 +799,29 @@ cr.define('options', function() {
     /**
      * Updates an element to show a list of errors.
      * @param {Element} panel An element to hold the errors.
+     * @param {string} columnType A tag used to identify the column when
+     *     changing focus.
      * @param {Array<RuntimeError>|undefined} errors The errors to be displayed.
      * @private
      */
-    updateErrors_: function(panel, errors) {
+    updateErrors_: function(panel, columnType, errors) {
       // TODO(hcarmona): Look into updating the ExtensionErrorList rather than
       // rebuilding it every time.
       panel.hidden = !errors || errors.length == 0;
       panel.textContent = '';
       if (!panel.hidden) {
-        panel.appendChild(
-            new extensions.ExtensionErrorList(assertInstanceof(errors, Array)));
+        var errorList =
+            new extensions.ExtensionErrorList(assertInstanceof(errors, Array));
+
+        panel.appendChild(errorList);
+
+        var list = errorList.getListElement();
+        if (list)
+          list.setAttribute('column-type', columnType + 'list');
+
+        var button = errorList.getToggleElement();
+        if (button)
+          button.setAttribute('column-type', columnType + 'button');
       }
     },