Change menu button focus on mouse down

When a button on the action bar has focus, clicking on any menu button
fails to grab focus from any existing focus item, leaving the previously
focused button as selected.

Checks for currently focused element. If a button has focus, let the
browser perform the default action which will set the menu button as
the focused element. If the focus is on any other element, leave it
alone (e.g. in the case where a row in the file list has focus, we
don't want to change that due to the menu activation).

Bug: 906566
Tests: browser_test --gtest_filter="*MultiMenu"
Change-Id: I32ab6b5e52771298c13fae51c2b13fba9cf996b7
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1935567
Commit-Queue: Alex Danilo <adanilo@chromium.org>
Commit-Queue: Noel Gordon <noel@chromium.org>
Reviewed-by: Noel Gordon <noel@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719003}

ui/file_manager/file_manager/foreground/js/quick_view_controller.js

@@ -12,7 +12,7 @@ class QuickViewController {
    * @param {!MetadataModel} metadataModel File system metadata.
    * @param {!FileSelectionHandler} selectionHandler
    * @param {!ListContainer} listContainer
-   * @param {!cr.ui.MenuButton} selectionMenuButton
+   * @param {!cr.ui.MultiMenuButton} selectionMenuButton
    * @param {!QuickViewModel} quickViewModel
    * @param {!TaskController} taskController
    * @param {!cr.ui.ListSelectionModel} fileListSelectionModel

ui/file_manager/file_manager/foreground/js/selection_menu_controller.js

@@ -4,7 +4,7 @@
 
 class SelectionMenuController {
   /**
-   * @param {!cr.ui.MenuButton} selectionMenuButton
+   * @param {!cr.ui.MultiMenuButton} selectionMenuButton
    * @param {!cr.ui.Menu} menu
    */
   constructor(selectionMenuButton, menu) {

ui/file_manager/file_manager/foreground/js/sort_menu_controller.js

@@ -4,7 +4,7 @@
 
 class SortMenuController {
   /**
-   * @param {!cr.ui.MenuButton} sortButton
+   * @param {!cr.ui.MultiMenuButton} sortButton
    * @param {!FilesToggleRipple} toggleRipple
    * @param {!FileListModel} fileListModel
    */

ui/file_manager/file_manager/foreground/js/ui/file_manager_ui.js

@@ -187,11 +187,11 @@ class FileManagerUI {
 
     /**
      * The button to sort the file list.
-     * @type {!cr.ui.MenuButton}
+     * @type {!cr.ui.MultiMenuButton}
      * @const
      */
     this.sortButton =
-        util.queryDecoratedElement('#sort-button', cr.ui.MenuButton);
+        util.queryDecoratedElement('#sort-button', cr.ui.MultiMenuButton);
 
     /**
      * Ripple effect of sort button.
@@ -227,11 +227,11 @@ class FileManagerUI {
 
     /**
      * The button to open context menu in the check-select mode.
-     * @type {!cr.ui.MenuButton}
+     * @type {!cr.ui.MultiMenuButton}
      * @const
      */
-    this.selectionMenuButton =
-        util.queryDecoratedElement('#selection-menu-button', cr.ui.MenuButton);
+    this.selectionMenuButton = util.queryDecoratedElement(
+        '#selection-menu-button', cr.ui.MultiMenuButton);
 
     /**
      * Directory tree.

ui/file_manager/file_manager/foreground/js/ui/multi_menu.js

@@ -321,9 +321,12 @@ cr.define('cr.ui', () => {
             } else if (e.button == 0) {  // Only show the menu when using left
                                          // mouse button.
               this.showMenu(false, {x: e.screenX, y: e.screenY});
-
-              // Prevent the button from stealing focus on mousedown.
-              e.preventDefault();
+              // Prevent the button from stealing focus on mousedown unless
+              // focus is on another button.
+              if (!(document.hasFocus() &&
+                    document.activeElement.tagName === 'BUTTON')) {
+                e.preventDefault();
+              }
             }
           }
 

ui/file_manager/file_manager/foreground/js/ui/multi_menu_unittest.js

@@ -47,6 +47,8 @@ function setUp() {
     '  <cr-menu-item id="first" class="custom-appearance"></cr-menu-item>',
     '  <cr-menu-item id="second" class="custom-appearance"></cr-menu-item>',
     '</cr-menu>',
+    '<div id="focus-div" tabindex="1"/>',
+    '<button id="focus-button" tabindex="2"/>',
   ].join('');
 
   // Initialize cr.ui.Command with the <command>s.
@@ -71,6 +73,20 @@ function sendMouseOver(targetQuery) {
   return target.dispatchEvent(event);
 }
 
+/**
+ * Send a 'mousedown' event to the element target of a query.
+ * @param {string} targetQuery Query to specify the element.
+ */
+function sendMouseDown(targetQuery) {
+  const event = new MouseEvent('mousedown', {
+    bubbles: true,
+    composed: true,  // Allow the event to bubble past shadow DOM root.
+  });
+  const target = document.querySelector(targetQuery);
+  assertTrue(!!target);
+  return target.dispatchEvent(event);
+}
+
 /**
  * Send a 'mouseover' event to the element target of a query.
  * @param {string} targetQuery Query to specify the element.
@@ -348,3 +364,33 @@ function testShrinkWindowSizesTopMenu() {
   const shrunkPosition = topMenu.getBoundingClientRect();
   assertTrue(shrunkPosition.bottom < window.innerHeight);
 }
+
+/**
+ * Tests that mousedown the menu button grabs focus.
+ */
+function testFocusMenuButtonWithMouse() {
+  // Set focus on a div element.
+  //* @type {HTMLElement} */
+  const divElement = document.querySelector('#focus-div');
+  divElement.focus();
+
+  // Send mousedown event to the menu button.
+  sendMouseDown('#test-menu-button');
+
+  // Verify that the previously focused element still has focus.
+  assertTrue(document.hasFocus() && document.activeElement === divElement);
+
+  // Set focus on a button element.
+  //* @type {HTMLElement} */
+  const buttonElement = document.querySelector('#focus-button');
+  buttonElement.focus();
+
+  // Send mousedown event to the menu button.
+  sendMouseDown('#test-menu-button');
+
+  // Verify that the previously focused button has lost focus.
+  assertFalse(document.hasFocus() && document.activeElement === buttonElement);
+
+  // Verify the menu button has taken focus.
+  assertTrue(document.hasFocus() && document.activeElement === menubutton);
+}