Maintain focus when deleting items from chrome://history.

Focus was not maintained because the elements in the row did not have focus when the menu was displayed. Change makes it so that the delete happens when the menu is no longer displayed. This allows the normal code path to run and maintain focus.

Updated unit test so that the appropriate event is tested.

BUG=431029

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

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

chrome/browser/resources/history/history.html

@@ -135,9 +135,11 @@
   </div>
 </div>
 
+<command id="remove-visit-command"></command>
 <cr-menu id="action-menu" hidden>
   <button id="more-from-site" i18n-content="moreFromSite"></button>
-  <button id="remove-visit" i18n-content="removeFromHistory"></button>
+  <button id="remove-visit" i18n-content="removeFromHistory"
+      command="#remove-visit-command"></button>
 </cr-menu>
 
 <script src="chrome://history-frame/strings.js"></script>

chrome/browser/resources/history/history.js

@@ -32,6 +32,7 @@ var activeVisit = null;
 /** @const */ var FocusOutlineManager = cr.ui.FocusOutlineManager;
 /** @const */ var Menu = cr.ui.Menu;
 /** @const */ var MenuButton = cr.ui.MenuButton;
+/** @const */ var MenuItem = cr.ui.MenuItem;
 
 /**
  * Enum that shows the filtering behavior for a host or URL to a supervised
@@ -1935,18 +1936,14 @@ function load() {
       searchField.blur();  // Dismiss the keyboard.
   };
 
-  var mayRemoveVisits = loadTimeData.getBoolean('allowDeletingHistory');
-  $('remove-visit').disabled = !mayRemoveVisits;
+  var removeMenu = $('remove-visit');
+  // Decorate remove-visit before disabling/hiding because the values are
+  // overwritten when decorating a MenuItem that has a Command.
+  cr.ui.decorate(removeMenu, MenuItem);
+  removeMenu.disabled = !loadTimeData.getBoolean('allowDeletingHistory');
+  removeMenu.hidden = loadTimeData.getBoolean('hideDeleteVisitUI');
 
-  if (mayRemoveVisits) {
-    $('remove-visit').addEventListener('activate', function(e) {
-      activeVisit.removeFromHistory();
-      activeVisit = null;
-    });
-  }
-
-  if (loadTimeData.getBoolean('hideDeleteVisitUI'))
-    $('remove-visit').hidden = true;
+  document.addEventListener('command', handleCommand);
 
   searchField.addEventListener('search', doSearch);
   $('search-button').addEventListener('click', doSearch);
@@ -2016,6 +2013,26 @@ function load() {
 </if> /* is_ios */
 }
 
+/**
+ * Handle all commands in the history page.
+ * @param {!Event} e is a command event.
+ */
+function handleCommand(e) {
+  switch (e.command.id) {
+    case 'remove-visit-command':
+      // Removing visited items needs to be done with a command in order to have
+      // proper focus. This is because the command event is handled after the
+      // menu dialog is no longer visible and focus has returned to the history
+      // items. The activate event is handled when the menu dialog is still
+      // visible and focus is lost.
+      // removeEntryFromHistory_ will update activeVisit to the newly focused
+      // history item.
+      assert(!$('remove-visit').disabled);
+      activeVisit.removeEntryFromHistory_(e);
+      break;
+  }
+}
+
 /**
  * Updates the filter status labels of a host/URL entry to the current value.
  * @param {Element} statusElement The div which contains the status labels.

chrome/test/data/webui/history_browsertest.js

@@ -149,6 +149,7 @@ BaseHistoryWebUITest.prototype = {
   /** @override */
   accessibilityIssuesAreErrors: true,
 
+  /** @override */
   isAsync: true,
 };
 
@@ -825,7 +826,10 @@ TEST_F('HistoryWebUIRealBackendTest', 'singleDeletion', function() {
     waitForCallback('onEntryRemoved', callback);
 
     cr.dispatchSimpleEvent(dropDownButton, 'mousedown');
-    cr.dispatchSimpleEvent(removeMenuItem, 'activate');
+
+    var e = new Event('command', {bubbles: true});
+    e.command = removeMenuItem.command;
+    removeMenuItem.dispatchEvent(e);
   };
 
   var secondTitle = document.querySelectorAll('.entry a')[1].textContent;
@@ -889,7 +893,7 @@ TEST_F('HistoryWebUIRealBackendTest', 'showConfirmDialogAndCancel', function() {
   var esc = document.createEvent('KeyboardEvent');
   esc.initKeyboardEvent('keydown', true, true, window, 'U+001B');
 
-  document.dispatchEvent(esc);
+  document.documentElement.dispatchEvent(esc);
   assertFalse($('alertOverlay').classList.contains('showing'));
 
   testDone();
@@ -906,7 +910,7 @@ TEST_F('HistoryWebUIRealBackendTest', 'showConfirmDialogAndRemove', function() {
 
   var enter = document.createEvent('KeyboardEvent');
   enter.initKeyboardEvent('keydown', true, true, window, 'Enter');
-  document.dispatchEvent(enter);
+  document.documentElement.dispatchEvent(enter);
   assertFalse($('alertOverlay').classList.contains('showing'));
 });