Make downloads list keyboard shortcuts more consistent.

chrome/browser/resources/history/history.js

Index: chrome/browser/resources/history/history.js
diff --git a/chrome/browser/resources/history/history.js b/chrome/browser/resources/history/history.js
index a71592de4ee5d70587681b99c7b513c00db04f3a..fe3b580e6a19865fea81720c52b148dc63c75d2b 100644
--- a/chrome/browser/resources/history/history.js
+++ b/chrome/browser/resources/history/history.js
@@ -250,6 +250,8 @@ Visit.prototype.getResultDOM = function(propertyBag) {
       e.preventDefault();
     }.bind(this));
   }
+  if (focusless)
+    bookmarkSection.tabIndex = -1;
   entryBox.appendChild(bookmarkSection);
 
   var visitEntryWrapper = /** @type {HTMLElement} */(
@@ -895,22 +897,43 @@ function HistoryFocusObserver() {}
 HistoryFocusObserver.prototype = {
   /** @override */
   onActivate: function(row) {
-    this.getActiveRowElement_(row).classList.add('active');
+    row.getRowElement().classList.add('active');
   },
 
   /** @override */
   onDeactivate: function(row) {
-    this.getActiveRowElement_(row).classList.remove('active');
+    row.getRowElement().classList.remove('active');
   },
 
   /**
-   * @param {cr.ui.FocusRow} row The row to find an element for.
+   * @param {Element} item The row item to find a a row element for.
    * @return {Element} |row|'s "active" element.
-   * @private
+   * @override
    */
-  getActiveRowElement_: function(row) {
-    return findAncestorByClass(row.items[0], 'entry') ||
-           findAncestorByClass(row.items[0], 'site-domain-wrapper');
+  getRowElement: function(item) {
+    return findAncestorByClass(item, 'entry') ||
+           findAncestorByClass(item, 'site-domain-wrapper');
+  },
+
+  /** @override */
+  onElementIdMiss: function(row, expectedId) {
+    // Clickable titles.
+    if (expectedId == 'domain')
+      return 'title';
+    if (expectedId == 'title')
+      return 'domain';
+
+    // Select the title or the domain if there is no bookmarked star.
+    if (expectedId == 'star') {
+      if (row.elementIds.indexOf('title') != -1)
+        return 'title';
+      return 'domain';
+    }
+
+    if (expectedId == 'menu')
+      return row.elementIds[row.elementIds.length - 1];
+
+    return null;
   },
 };
 
@@ -1204,7 +1227,7 @@ HistoryView.prototype.onBeforeRemove = function(visit) {
   var row = this.focusGrid_.rows[pos.row + 1] ||
             this.focusGrid_.rows[pos.row - 1];
   if (row)
-    row.focusIndex(Math.min(pos.col, row.items.length - 1));
+    row.setFocusId(pos.elementId);
 };
 
 /** @param {Visit} visit The visit about to be unstarred. */
@@ -1214,7 +1237,7 @@ HistoryView.prototype.onBeforeUnstarred = function(visit) {
 
   var pos = this.focusGrid_.getPositionForTarget(document.activeElement);
   var row = this.focusGrid_.rows[pos.row];
-  row.focusIndex(Math.min(pos.col + 1, row.items.length - 1));
+  row.setFocusId(pos.elementId);
 };
 
 /** @param {Visit} visit The visit that was just unstarred. */
@@ -1674,26 +1697,44 @@ var focusGridRowSelector = [
   '.site-domain-wrapper'
 ].join(', ');
 
-var focusGridColumnSelector = [
-  '.entry-box input',
-  '.bookmark-section.starred',
-  '.title a',
-  '.drop-down',
-  '.domain-checkbox',
-  '[is="action-link"]',
-].join(', ');
+/**
+ * Add a column to |focusRow| if it exists in |row|.
+ * @param {Element} row The row that should contain the element.
+ * @param {cr.ui.FocusRow} focusRow The focus row to add the element to.
+ * @param {string} query A query to select the appropriate element in |row|.
+ * @param {string} elementId The elementId to use in |focusRow|.
+ * @private
+ */
+HistoryView.prototype.addFocusRow_ = function(row, focusRow, query, elementId) {
+  var element = row.querySelectorAll(query);
+
+  // There should only be 0 or 1 matches.
+  if (element.length == 1)
+    focusRow.setFocusableElementId(element[0], elementId);
+  else
+    assert(element.length == 0);
+};
 
 /** @private */
 HistoryView.prototype.updateFocusGrid_ = function() {
   var rows = this.resultDiv_.querySelectorAll(focusGridRowSelector);
-  var grid = [];
+  this.focusGrid_.destroy();
 
   for (var i = 0; i < rows.length; ++i) {
     assert(rows[i].parentNode);
-    grid.push(rows[i].querySelectorAll(focusGridColumnSelector));
-  }
+    var focusRow = this.focusGrid_.createRow();
+
+    // These 2 buttons are mutually exclusive, but visually the same.

[dmazzoni, 2015/01/15 19:38:19]

I don't quite understand this comment.

[hcarmona, 2015/01/16 21:39:06]

Done.

+    this.addFocusRow_(rows[i], focusRow, '.entry-box input', 'checkbox');
+    this.addFocusRow_(rows[i], focusRow, '.domain-checkbox', 'checkbox');
 
-  this.focusGrid_.setGrid(grid);
+    this.addFocusRow_(rows[i], focusRow, '.bookmark-section.starred', 'star');
+    this.addFocusRow_(rows[i], focusRow, '[is="action-link"]', 'domain');
+    this.addFocusRow_(rows[i], focusRow, '.title a', 'title');
+    this.addFocusRow_(rows[i], focusRow, '.drop-down', 'menu');
+
+    this.focusGrid_.addRow(focusRow);
+  }
 };
 
 /**