Keep focus on the original place when closing menu by mouse or touchscreen.

This will also affect other UI elements derived from cr.ui.MenuButton like:
- Combo Button
- Context Menu Button

The UI elements are also referred in other places than the Files app.
- "Apps" menu in login screen
- media control UI in the video player
- Bookmark Manager

The button is made not to steal the focus when clicking it to open menu,
https://cs.chromium.org/chromium/src/ui/webui/resources/js/cr/ui/menu_button.js?q=file:menu_button.js+stealing+focus&sq=package:chromium&dr&l=139
however, it had taken focus when the menu item is activated by a click.
It made the focus left on the button after finishing operation on a button
using either mouse or touchscreen, requiring MenuButton class to hide that
focus highlight by attaching "using-mouse" class attribute.


Test: browser_tests --gtest_filter=WebUIResourceBrowserTest.MenuButtonTest*
Bug: 771024,769593
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: I23fb1b0ce907a21407ffca514032bfdc083e36de
Reviewed-on: https://chromium-review.googlesource.com/816376
Commit-Queue: Tatsuhisa Yamaguchi <yamaguchi@chromium.org>
Reviewed-by: Michael Giuffrida <michaelpg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523331}

chrome/test/data/webui/menu_button_test.html

@@ -48,6 +48,57 @@ function testMenuShowAndHideEvents() {
   assertFalse(menuButton.classList.contains('using-mouse'));
 }
 
+function testFocusMoves() {
+  var menu = document.createElement('div');
+  var otherButton = document.createElement('button');
+  cr.ui.decorate(menu, cr.ui.Menu);
+  menu.addMenuItem({});
+  document.body.appendChild(menu);
+  document.body.appendChild(otherButton);
+
+  var menuButton = document.createElement('div');
+  cr.ui.decorate(menuButton, cr.ui.MenuButton);
+  // Allow to put focus on the menu button by focus().
+  menuButton.tabIndex = 1;
+  menuButton.menu = menu;
+  document.body.appendChild(menuButton);
+
+  // Case 1: Close by mouse click outside the menu.
+  otherButton.focus();
+  // Click to show menu.
+  menuButton.dispatchEvent(new MouseEvent('mousedown'));
+  assertTrue(menuButton.isMenuShown());
+  // Click again to hide menu by clicking the button.
+  menuButton.dispatchEvent(new MouseEvent('mousedown'));
+  assertFalse(menuButton.isMenuShown());
+  // Focus should be kept on the original place.
+  assertEquals(otherButton, document.activeElement);
+
+  // Case 2: Activate menu item with mouse.
+  menuButton.dispatchEvent(new MouseEvent('mousedown'));
+  assertTrue(menuButton.isMenuShown());
+  // Emulate choosing a menu item by mouse click.
+  var menuItem = menu.menuItems[0];
+  menuItem.selected = true;
+  menuItem.dispatchEvent(new MouseEvent('mouseup', {buttons: 1}));
+  assertFalse(menuButton.isMenuShown());
+  // Focus should be kept on the original place.
+  assertEquals(otherButton, document.activeElement);
+
+  // Case 3: Open menu and activate by keyboard.
+  menuButton.focus();
+  assertEquals(menuButton, document.activeElement);
+  // Open menu
+  menuButton.dispatchEvent(new KeyboardEvent('keydown', {key: 'Enter'}));
+  assertTrue(menuButton.isMenuShown(), 'menu opened');
+  // Select an item and activate
+  menu.dispatchEvent(new KeyboardEvent('keydown', {key: 'ArrowDown'}));
+  menu.dispatchEvent(new KeyboardEvent('keydown', {key: 'Enter'}));
+  assertFalse(menuButton.isMenuShown(), 'menu closed');
+  // Focus should be still on the menu button.
+  assertEquals(menuButton, document.activeElement);
+}
+
 </script>
 </body>
 </html>

chrome/test/data/webui/webui_resource_browsertest.cc

@@ -209,6 +209,7 @@ IN_PROC_BROWSER_TEST_F(WebUIResourceBrowserTest, MenuButtonTest) {
   AddLibrary(IDR_WEBUI_JS_CR_UI);
   AddLibrary(IDR_WEBUI_JS_CR_UI_POSITION_UTIL);
   AddLibrary(IDR_WEBUI_JS_CR_UI_MENU_BUTTON);
+  AddLibrary(IDR_WEBUI_JS_CR_UI_MENU_ITEM);
   AddLibrary(IDR_WEBUI_JS_CR_UI_MENU);
   LoadFile(base::FilePath(FILE_PATH_LITERAL("menu_button_test.html")));
 }

ui/webui/resources/js/cr/ui/menu_button.js

@@ -120,19 +120,19 @@ cr.define('cr.ui', function() {
           // Touch on the menu button itself is ignored to avoid that the menu
           // opened again by the mousedown event following the touch events.
           if (this.shouldDismissMenu_(e)) {
-            this.hideMenu();
+            this.hideMenuWithoutTakingFocus_();
           }
           break;
         case 'mousedown':
           if (e.currentTarget == this.ownerDocument) {
             if (this.shouldDismissMenu_(e)) {
-              this.hideMenu();
+              this.hideMenuWithoutTakingFocus_();
             } else {
               e.preventDefault();
             }
           } else {
             if (this.isMenuShown()) {
-              this.hideMenu();
+              this.hideMenuWithoutTakingFocus_();
             } else if (e.button == 0) {  // Only show the menu when using left
                                          // mouse button.
               this.showMenu(false, {x: e.screenX, y: e.screenY});
@@ -173,7 +173,14 @@ cr.define('cr.ui', function() {
         case 'activate':
           var hideDelayed =
               e.target instanceof cr.ui.MenuItem && e.target.checkable;
-          this.hideMenu(hideDelayed ? HideType.DELAYED : HideType.INSTANT);
+          var hideType = hideDelayed ? HideType.DELAYED : HideType.INSTANT;
+          if (e.originalEvent instanceof MouseEvent ||
+              e.originalEvent instanceof TouchEvent) {
+            this.hideMenuWithoutTakingFocus_(hideType);
+          } else {
+            // Keyboard. Take focus to continue keyboard operation.
+            this.hideMenu(hideType);
+          }
           break;
         case 'scroll':
           if (!(e.target == this.menu || this.menu.contains(e.target)))
@@ -244,6 +251,27 @@ cr.define('cr.ui', function() {
      *     default: cr.ui.HideType.INSTANT.
      */
     hideMenu: function(opt_hideType) {
+      this.hideMenuInternal_(true, opt_hideType);
+    },
+
+    /**
+     * Hides the menu. If your menu can go out of scope, make sure to call this
+     * first.
+     * @param {cr.ui.HideType=} opt_hideType Type of hide.
+     *     default: cr.ui.HideType.INSTANT.
+     */
+    hideMenuWithoutTakingFocus_: function(opt_hideType) {
+      this.hideMenuInternal_(false, opt_hideType);
+    },
+
+    /**
+     * Hides the menu. If your menu can go out of scope, make sure to call this
+     * first.
+     * @param {boolean} shouldTakeFocus Moves the focus to the button if true.
+     * @param {cr.ui.HideType=} opt_hideType Type of hide.
+     *     default: cr.ui.HideType.INSTANT.
+     */
+    hideMenuInternal_: function(shouldTakeFocus, opt_hideType) {
       if (!this.isMenuShown())
         return;
 
@@ -255,7 +283,8 @@ cr.define('cr.ui', function() {
       this.menu.hide();
 
       this.showingEvents_.removeAll();
-      this.focus();
+      if (shouldTakeFocus)
+        this.focus();
 
       var event = new UIEvent(
           'menuhide', {bubbles: true, cancelable: false, view: window});