history: fix more focus oddness.

Various things (cr.ui.MenuButtons, checkboxes) call .preventDefault()
while handling 'mousedown' events which messes with focus. Account for this.

R=arv@chromium.org
BUG=424475

Review URL: https://codereview.chromium.org/685783003

Cr-Commit-Position: refs/heads/master@{#302699}

chrome/browser/resources/history/history.js

@@ -212,7 +212,7 @@ Visit.prototype.getResultDOM = function(propertyBag) {
     if (!isMobileVersion()) {
       // Clicking anywhere in the entryBox will check/uncheck the checkbox.
       entryBox.setAttribute('for', checkbox.id);
-      entryBox.addEventListener('mousedown', entryBoxMousedown);
+      entryBox.addEventListener('mousedown', this.handleMousedown_.bind(this));
       entryBox.addEventListener('click', entryBoxClick);
       entryBox.addEventListener('keydown', this.handleKeydown_.bind(this));
     }
@@ -498,6 +498,20 @@ Visit.prototype.handleKeydown_ = function(e) {
     this.removeEntryFromHistory_(e);
 };
 
+/**
+ * @param {Event} event A mousedown event.
+ * @private
+ */
+Visit.prototype.handleMousedown_ = function(event) {
+  // Prevent text selection when shift-clicking to select multiple entries.
+  if (event.shiftKey) {
+    event.preventDefault();
+
+    if (this.model_.getView().isInFocusGrid(event.target))
+      event.target.focus();
+  }
+};
+
 /**
  * Removes a history entry on click or keydown and finds a new entry to focus.
  * @param {Event} e A click or keydown event.
@@ -897,6 +911,28 @@ HistoryFocusObserver.prototype = {
   },
 };
 
+///////////////////////////////////////////////////////////////////////////////
+// HistoryFocusGrid:
+
+/**
+ * @param {Node=} opt_boundary
+ * @param {cr.ui.FocusRow.Observer=} opt_observer
+ * @constructor
+ * @extends {cr.ui.FocusGrid}
+ */
+function HistoryFocusGrid(opt_boundary, opt_observer) {
+  cr.ui.FocusGrid.apply(this, arguments);
+}
+
+HistoryFocusGrid.prototype = {
+  __proto__: cr.ui.FocusGrid.prototype,
+
+  /** @override */
+  onMousedown: function(row, e) {
+    return !!findAncestorByClass(e.target, 'menu-button');
+  },
+};
+
 ///////////////////////////////////////////////////////////////////////////////
 // HistoryView:
 
@@ -911,8 +947,8 @@ function HistoryView(model) {
   this.editButtonTd_ = $('edit-button');
   this.editingControlsDiv_ = $('editing-controls');
   this.resultDiv_ = $('results-display');
-  this.focusGrid_ = new cr.ui.FocusGrid(this.resultDiv_,
-                                        new HistoryFocusObserver);
+  this.focusGrid_ = new HistoryFocusGrid(this.resultDiv_,
+                                         new HistoryFocusObserver);
   this.pageDiv_ = $('results-pagination');
   this.model_ = model;
   this.pageIndex_ = 0;
@@ -1254,6 +1290,14 @@ HistoryView.prototype.positionNotificationBar = function() {
   }
 };
 
+/**
+ * @param {Element} el An element to look for.
+ * @return {boolean} Whether |el| is in |this.focusGrid_|.
+ */
+HistoryView.prototype.isInFocusGrid = function(el) {
+  return !!this.focusGrid_.getPositionForTarget(el);
+};
+
 // HistoryView, private: ------------------------------------------------------
 
 /**
@@ -2163,12 +2207,6 @@ function updateParentCheckbox(checkbox) {
     groupCheckbox.checked = false;
 }
 
-function entryBoxMousedown(event) {
-  // Prevent text selection when shift-clicking to select multiple entries.
-  if (event.shiftKey)
-    event.preventDefault();
-}
-
 /**
  * Handle click event for entryBoxes.
  * @param {!Event} event A click event.

chrome/test/data/webui/history_browsertest.js

@@ -909,6 +909,59 @@ TEST_F('HistoryWebUIRealBackendTest', 'showConfirmDialogAndRemove', function() {
   assertFalse($('alertOverlay').classList.contains('showing'));
 });
 
+TEST_F('HistoryWebUIRealBackendTest', 'menuButtonActivatesOneRow', function() {
+  var entries = document.querySelectorAll('.entry');
+  assertEquals(3, entries.length);
+  assertTrue(entries[0].classList.contains('active'));
+  assertTrue($('action-menu').hidden);
+
+  // Show the menu via mousedown on the menu button.
+  var menuButton = entries[2].querySelector('.menu-button');
+  menuButton.dispatchEvent(new MouseEvent('mousedown'));
+  expectFalse($('action-menu').hidden);
+
+  // Check that the 'active' item hasn't changed.
+  expectTrue(entries[0].classList.contains('active'));
+  expectFalse(entries[2].classList.contains('active'));
+
+  testDone();
+});
+
+TEST_F('HistoryWebUIRealBackendTest', 'shiftClickActivatesOneRow', function() {
+  var entries = document.querySelectorAll('.entry');
+  assertEquals(3, entries.length);
+  assertTrue(entries[0].classList.contains('active'));
+
+  entries[0].visit.checkBox.focus();
+  assertEquals(entries[0].visit.checkBox, document.activeElement);
+
+  entries[0].visit.checkBox.click();
+  assertTrue(entries[0].visit.checkBox.checked);
+
+  var entryBox = entries[2].querySelector('.entry-box');
+  entryBox.dispatchEvent(new MouseEvent('click', {shiftKey: true}));
+  assertTrue(entries[1].visit.checkBox.checked);
+
+  // Focus shouldn't have changed, but the checkbox should toggle.
+  expectEquals(entries[0].visit.checkBox, document.activeElement);
+
+  expectTrue(entries[0].classList.contains('active'));
+  expectFalse(entries[2].classList.contains('active'));
+
+  var shiftDown = new MouseEvent('mousedown', {shiftKey: true, bubbles: true});
+  entries[2].visit.checkBox.dispatchEvent(shiftDown);
+  expectEquals(entries[2].visit.checkBox, document.activeElement);
+
+  // 'focusin' events aren't dispatched while tests are run in batch (e.g.
+  // --test-launcher-jobs=2). Simulate this. TODO(dbeam): fix instead.
+  cr.dispatchSimpleEvent(document.activeElement, 'focusin', true, true);
+
+  expectFalse(entries[0].classList.contains('active'));
+  expectTrue(entries[2].classList.contains('active'));
+
+  testDone();
+});
+
 /**
  * Fixture for History WebUI testing when deletions are prohibited.
  * @extends {HistoryWebUIRealBackendTest}

ui/webui/resources/js/cr/ui/focus_grid.js

@@ -80,7 +80,7 @@ cr.define('cr.ui', function() {
         row = this.rows.length - 1;
 
       if (!this.rows[row])
-        return;
+        return false;
 
       var colIndex = keyRow.items.indexOf(e.target);
       var col = Math.min(colIndex, this.rows[row].items.length - 1);
@@ -88,6 +88,12 @@ cr.define('cr.ui', function() {
       this.rows[row].focusIndex(col);
 
       e.preventDefault();
+      return true;
+    },
+
+    /** @override */
+    onMousedown: function(row, e) {
+      return false;
     },
 
     /**

ui/webui/resources/js/cr/ui/focus_row.js

@@ -56,7 +56,7 @@ cr.define('cr.ui', function() {
       if (item != document.activeElement)
         item.tabIndex = -1;
 
-      this.eventTracker_.add(item, 'click', this.onClick_.bind(this));
+      this.eventTracker_.add(item, 'mousedown', this.onMousedown_.bind(this));
     }, this);
 
     /**
@@ -75,9 +75,17 @@ cr.define('cr.ui', function() {
      * Called when a key is pressed while an item in |this.items| is focused. If
      * |e|'s default is prevented, further processing is skipped.
      * @param {cr.ui.FocusRow} row The row that detected a keydown.
-     * @param {Event} e The keydown event.
+     * @param {Event} e
+     * @return {boolean} Whether the event was handled.
      */
     onKeydown: assertNotReached,
+
+    /**
+     * @param {cr.ui.FocusRow} row The row that detected the mouse going down.
+     * @param {Event} e
+     * @return {boolean} Whether the event was handled.
+     */
+    onMousedown: assertNotReached,
   };
 
   /** @interface */
@@ -157,10 +165,7 @@ cr.define('cr.ui', function() {
       if (item < 0)
         return;
 
-      if (this.delegate_)
-        this.delegate_.onKeydown(this, e);
-
-      if (e.defaultPrevented)
+      if (this.delegate_ && this.delegate_.onKeydown(this, e))
         return;
 
       var index = -1;
@@ -185,7 +190,10 @@ cr.define('cr.ui', function() {
      * @param {Event} e A click event.
      * @private
      */
-    onClick_: function(e) {
+    onMousedown_: function(e) {
+      if (this.delegate_ && this.delegate_.onMousedown(this, e))
+        return;
+
       if (!e.button)
         this.activeIndex = this.items.indexOf(e.currentTarget);
     },