a11y/history: don't change focus when a user clicks on bookmark star icon.

chrome/browser/resources/history/history.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.
 
 <include src="../uber/uber_utils.js">
 <include src="history_focus_manager.js">
 
 ///////////////////////////////////////////////////////////////////////////////
 // Globals:
 /** @const */ var RESULTS_PER_PAGE = 150;
@@ skipping 169 matching lines @@
     var menu = $('action-menu');
     menu.dataset.devicename = self.deviceName;
     menu.dataset.devicetype = self.deviceType;
   };
   domain.textContent = this.domain_;
 
   entryBox.appendChild(time);
 
   var bookmarkSection = createElementWithClassName('div', 'bookmark-section');
   if (this.starred_) {
+    bookmarkSection.title = loadTimeData.getString('removeBookmark');
     bookmarkSection.classList.add('starred');
     bookmarkSection.addEventListener('click', function f(e) {
       recordUmaAction('HistoryPage_BookmarkStarClicked');
       bookmarkSection.classList.remove('starred');
       chrome.send('removeBookmark', [self.url_]);
       bookmarkSection.removeEventListener('click', f);
       e.preventDefault();
     });
   }
   entryBox.appendChild(bookmarkSection);
@@ skipping 84 matching lines @@
   }.bind(this));
 };
 
 /**
  * @param {boolean} isLead Whether this visit is the "lead" visit, i.e. the one
  *     that would be focused if the entry list is tabbed to.
  */
 Visit.prototype.setIsLead = function(isLead) {
   this.domNode_.querySelector('.entry-box').classList.toggle('lead', isLead);
   if (!isLead) {
-    if (this.checkBox)
-      this.checkBox.tabIndex = -1;
-
-    this.titleLink.tabIndex = -1;
-
-    if (this.dropDown)
-      this.dropDown.tabIndex = -1;
+    this.getFocusableControls_().forEach(function(control) {
+      control.tabIndex = -1;
+    });
   }
 };
 
 /**
  * @param {Element} control A control element to focus.
  */
 Visit.prototype.focusControl = function(control) {
-  var controls = [this.titleLink];
-
-  if (this.checkBox)
-    controls.push(this.checkBox);
-
-  if (this.dropDown)
-    controls.push(this.dropDown);
-
+  var controls = this.getFocusableControls_();
   assert(controls.indexOf(control) >= 0);
 
   for (var i = 0; i < controls.length; ++i) {
     controls[i].tabIndex = controls[i] == control ? 0 : -1;
   }
 
   control.focus();
   this.setIsLead(true);
 };
 
 Object.defineProperty(Visit.prototype, 'checkBox', {
   get: function() {
     return this.domNode_.querySelector('input[type=checkbox]');
   },
 });
 
+Object.defineProperty(Visit.prototype, 'bookmarkStar', {
+  get: function() {
+    return this.domNode_.querySelector('.bookmark-section.starred');
+  },
+});
+
 Object.defineProperty(Visit.prototype, 'titleLink', {
   get: function() {
     return this.domNode_.querySelector('.title a');
   },
 });
 
 Object.defineProperty(Visit.prototype, 'dropDown', {
   get: function() {
     return this.domNode_.querySelector('button.drop-down');
   },
@@ skipping 110 matching lines @@
  * Launch a search for more history entries from the same domain.
  * @private
  */
 Visit.prototype.showMoreFromSite_ = function() {
   recordUmaAction('HistoryPage_EntryMenuShowMoreFromSite');
   historyView.setSearch(this.domain_);
   $('search-field').focus();
 };
 
 /**
+ * @return {Array.<Element>} A list of focusable controls.
+ * @private
+ */
+Visit.prototype.getFocusableControls_ = function() {
+  var controls = [];
+
+  if (this.checkBox)
+    controls.push(this.checkBox);
+
+  if (this.bookmarkStar)
+    controls.push(this.bookmarkStar);
+
+  controls.push(this.titleLink);
+
+  if (this.dropDown)
+    controls.push(this.dropDown);
+
+  return controls;
+};
+
+/**
  * @param {Event} e A keydown event to handle.
  * @private
  */
 Visit.prototype.handleKeydown_ = function(e) {
   var keyCode = e.keyCode;
   if (keyCode == 8 || keyCode == 46) {  // Delete or Backspace.
     if (!this.model_.isDeletingVisits())
       this.removeEntryFromHistory_(e);
     return;
   }
 
   var target = e.target;
-  if (target != document.activeElement ||
-      !(keyCode == 37 || keyCode == 39) ||  // Left or right.
-      (keyCode == 37 && target == this.checkBox) ||
-      (keyCode == 39 && target == this.dropDown)) {
+  if (target != document.activeElement || !(keyCode == 37 || keyCode == 39)) {
+    // Handling key code for inactive element or key wasn't left or right.
     return;
   }
 
-  var toFocus;
-  if (e.keyCode == 37)  // Left.
-    toFocus = target == this.dropDown ? this.titleLink : this.checkBox;
-  else  // Right.
-    toFocus = target == this.checkBox ? this.titleLink : this.dropDown;
-
-  this.focusControl(toFocus);
-  e.preventDefault();
+  var controls = this.getFocusableControls_();
+  for (var i = 0; i < controls.length; ++i) {
+    if (controls[i].contains(target)) {
+      var toFocus = e.keyCode == 37 ? controls[i - 1] : controls[i + 1];

[dmazzoni, 2014/07/22 16:59:16]

Add back a comment for 37 == "left" or use a constant

[Dan Beam, 2014/07/22 18:10:06]

Done.

+      if (toFocus) {
+        this.focusControl(toFocus);
+        e.preventDefault();
+      }
+      break;
+    }
+  }
 };
 
 /**
  * Removes a history entry on click or keydown and finds a new entry to focus.
  * @param {Event} e A click or keydown event.
  * @private
  */
 Visit.prototype.removeEntryFromHistory_ = function(e) {
   this.model_.getView().onBeforeRemove(this);
   this.removeFromHistory();
@@ skipping 1121 matching lines @@
 
 /**
  * Swaps focus to |toBeFocused|. Assumes the another visit is currently focused.
  * @param {Visit} visit A visit to focus.
  * @private
  */
 HistoryView.prototype.swapFocusedVisit_ = function(visit) {
   if (!visit)
     return;
 
-  var control;
   var activeVisit = findAncestorByClass(document.activeElement, 'entry').visit;
-  if (document.activeElement == activeVisit.checkBox)
-    control = visit.checkBox;
-  else if (document.activeElement == activeVisit.titleLink)
-    control = visit.titleLink;
-  else if (document.activeElement == activeVisit.dropDown)
-    control = visit.dropDown;
+  var controls = activeVisit.getFocusableControls_();
 
-  visit.focusControl(control);
+  for (var i = 0; i < controls.length; ++i) {
+    var control = controls[i];
+    if (!control.contains(document.activeElement))
+      continue;
+
+    // Try to focus the same type of control if the new visit has it.
+    if (control == activeVisit.checkBox && visit.checkBox) {
+      visit.focusControl(visit.checkBox);
+    } else if (control == activeVisit.bookmarkStar && visit.bookmarkStar) {
+      visit.focusControl(visit.bookmarkStar);
+    } else if (control == activeVisit.titleLink) {
+      visit.focusControl(visit.titleLink);
+    } else if (control == activeVisit.dropDown && visit.dropDown) {
+      visit.focusControl(visit.dropDown);
+    } else {
+      // Otherwise, just focus something that might be in a similar column.
+      var controlsToFocus = visit.getFocusableControls_();
+      var indexToFocus = Math.min(i, controlsToFocus.length - 1);
+      visit.focusControl(controlsToFocus[indexToFocus]);
+    }
+    break;
+  }
+
   activeVisit.setIsLead(false);
 };
 
 /**
  * @param {Event} e A keydown event to handle.
  * @private
  */
 HistoryView.prototype.handleKeydown_ = function(e) {
   // Only handle up or down arrows on the focused element.
   var keyCode = e.keyCode, target = e.target;
@@ skipping 10 matching lines @@
  * @param {Event} e A mousedown event to handle.
  * @private
  */
 HistoryView.prototype.handleMousedown_ = function(e) {
   var target = e.target;
   var entry = findAncestorByClass(target, 'entry');
   if (!entry || !entry.contains(target))
     return;
 
   var visit = entry.visit;
+  if (visit.bookmarkStar && visit.bookmarkStar.contains(target))
+    return;
+
   if (visit.titleLink.contains(target))
     visit.focusControl(visit.titleLink);
   else if (visit.dropDown && visit.dropDown.contains(target))
     visit.focusControl(visit.dropDown);
   else  // Focus the checkbox by default. If no checkbox, focus the title.
     visit.focusControl(visit.checkBox || visit.titleLink);
 
   e.preventDefault();
 };
 
@@ skipping 610 matching lines @@
     historyView.reload();
 }
 
 // Add handlers to HTML elements.
 document.addEventListener('DOMContentLoaded', load);
 
 // This event lets us enable and disable menu items before the menu is shown.
 document.addEventListener('canExecute', function(e) {
   e.canExecute = true;
 });