Fix MenuBar keyboard navigation and selecting (#17076)

This patch also changes old TB2 test to TB4.

Change-Id: I7a2ba20267d2db99e29003b764530d65a4eab955

client/src/com/vaadin/client/ui/VMenuBar.java

@@ -1509,13 +1509,23 @@ public class VMenuBar extends SimpleFocusablePanel implements
                 // selection there
                 openMenuAndFocusFirstIfPossible(getSelected());
             } else {
-                Command command = getSelected().getCommand();
-                if (command != null) {
-                    command.execute();
-                }
+                final Command command = getSelected().getCommand();
 
                 setSelected(null);
                 hideParents(true);
+
+                // #17076 keyboard selected menuitem without children: do
+                // not leave menu to visible ("hover open") mode
+                menuVisible = false;
+
+                Scheduler.get().scheduleDeferred(new ScheduledCommand() {
+                    @Override
+                    public void execute() {
+                        if (command != null) {
+                            command.execute();
+                        }
+                    }
+                });
             }
         }
 

uitest/src/com/vaadin/tests/components/menubar/MenuBarNavigation.java

@@ -1,20 +1,29 @@
 package com.vaadin.tests.components.menubar;
 
-import com.vaadin.tests.components.TestBase;
-import com.vaadin.tests.util.Log;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUIWithLog;
 import com.vaadin.ui.MenuBar;
 import com.vaadin.ui.MenuBar.Command;
 import com.vaadin.ui.MenuBar.MenuItem;
 
-public class MenuBarNavigation extends TestBase implements Command {
+public class MenuBarNavigation extends AbstractTestUIWithLog implements Command {
 
     private MenuItem edit;
     private MenuItem file;
-    private Log log;
     private MenuItem export;
 
     @Override
-    protected void setup() {
+    public String getDescription() {
+        return "Test case for mouse and keyboard navigation in MenuBar";
+    }
+
+    @Override
+    public Integer getTicketNumber() {
+        return 5174;
+    }
+
+    @Override
+    protected void setup(VaadinRequest request) {
         MenuBar mb = new MenuBar();
         file = mb.addItem("File", null);
         file.addItem("Open", this);
@@ -32,24 +41,11 @@ public class MenuBarNavigation extends TestBase implements Command {
         mb.addItem("Help", this);
 
         addComponent(mb);
-
-        log = new Log(5);
-        addComponent(log);
-    }
-
-    @Override
-    protected String getDescription() {
-        return "Test case for mouse and keyboard navigation in MenuBar";
-    }
-
-    @Override
-    protected Integer getTicketNumber() {
-        return 5174;
     }
 
     @Override
     public void menuSelected(MenuItem selectedItem) {
-        log.log("MenuItem " + getName(selectedItem) + " selected");
+        log("MenuItem " + getName(selectedItem) + " selected");
     }
 
     private String getName(MenuItem selectedItem) {

uitest/src/com/vaadin/tests/components/menubar/MenuBarNavigationKeyboardTest.java

@@ -0,0 +1,95 @@
+package com.vaadin.tests.components.menubar;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.interactions.Actions;
+
+import com.vaadin.testbench.elements.MenuBarElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+public class MenuBarNavigationKeyboardTest extends MultiBrowserTest {
+
+    @Override
+    protected Class<?> getUIClass() {
+        return com.vaadin.tests.components.menubar.MenuBarNavigation.class;
+    }
+
+    @Override
+    protected boolean requireWindowFocusForIE() {
+        return true;
+    }
+
+    @Override
+    protected boolean usePersistentHoverForIE() {
+        return false;
+    }
+
+    @Test
+    public void testKeyboardNavigation() throws Exception {
+        openTestURL();
+
+        openMenu("File");
+        getMenuBar().sendKeys(Keys.DOWN, Keys.DOWN, Keys.DOWN, Keys.DOWN,
+                Keys.RIGHT, Keys.ENTER);
+        Assert.assertEquals("1. MenuItem File/Export../As PDF... selected",
+                getLogRow(0));
+
+        openMenu("File");
+        getMenuBar().sendKeys(Keys.RIGHT, Keys.RIGHT, Keys.RIGHT, Keys.ENTER);
+        Assert.assertEquals("2. MenuItem Help selected", getLogRow(0));
+
+        openMenu("Edit");
+        getMenuBar().sendKeys(Keys.LEFT, Keys.DOWN, Keys.DOWN, Keys.ENTER);
+        Assert.assertEquals("3. MenuItem Edit/Cut selected", getLogRow(0));
+
+        openMenu("Edit");
+        getMenuBar().sendKeys(Keys.ENTER);
+        Assert.assertEquals("3. MenuItem Edit/Cut selected", getLogRow(0));
+
+        getMenuBar().sendKeys(Keys.ENTER);
+        Assert.assertEquals("4. MenuItem Edit/Copy selected", getLogRow(0));
+
+        /* Enter while menubar has focus but no selection should focus "File" */
+        getMenuBar().sendKeys(Keys.ENTER);
+        Assert.assertEquals("4. MenuItem Edit/Copy selected", getLogRow(0));
+
+        /* Enter again should open File and focus Open */
+        getMenuBar().sendKeys(Keys.ENTER);
+        Assert.assertEquals("4. MenuItem Edit/Copy selected", getLogRow(0));
+
+        getMenuBar().sendKeys(Keys.ENTER);
+        Assert.assertEquals("5. MenuItem File/Open selected", getLogRow(0));
+    }
+
+    @Test
+    public void testMenuSelectWithKeyboardStateClearedCorrectly()
+            throws InterruptedException {
+        openTestURL();
+
+        openMenu("File");
+
+        getMenuBar().sendKeys(Keys.ARROW_RIGHT, Keys.ARROW_RIGHT,
+                Keys.ARROW_RIGHT, Keys.ENTER);
+
+        assertTrue("Help menu was not selected",
+                logContainsText("MenuItem Help selected"));
+
+        new Actions(driver).moveToElement(getMenuBar(), 10, 10).perform();
+
+        assertFalse("Unexpected MenuBar popup is visible",
+                isElementPresent(By.className("v-menubar-popup")));
+    }
+
+    public MenuBarElement getMenuBar() {
+        return $(MenuBarElement.class).first();
+    }
+
+    public void openMenu(String name) {
+        getMenuBar().clickItem(name);
+    }
+}

uitest/tb2/com/vaadin/tests/components/menubar/MenuBarNavigationKeyboard.html

@@ -1,178 +0,0 @@
-<?xml version="1.0" encoding="UTF-8"?>
-<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd">
-<html xmlns="http://www.w3.org/1999/xhtml" xml:lang="en" lang="en">
-<head profile="http://selenium-ide.openqa.org/profiles/test-case">
-<meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
-<link rel="selenium.base" href="" />
-<title>New Test</title>
-</head>
-<body>
-<table cellpadding="1" cellspacing="1" border="1">
-<thead>
-<tr><td rowspan="1" colspan="3">New Test</td></tr>
-</thead><tbody>
-<tr>
-	<td>open</td>
-	<td>/run/com.vaadin.tests.components.menubar.MenuBarNavigation?restartApplication</td>
-	<td></td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]#menu0</td>
-	<td>3,10</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>down</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>down</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>down</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>down</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>right</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>enter</td>
-</tr>
-<tr>
-	<td>assertText</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VLabel[0]</td>
-	<td>1. MenuItem File/Export../As PDF... selected</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]#menu0</td>
-	<td>0,7</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>right</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>right</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>right</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>enter</td>
-</tr>
-<tr>
-	<td>assertText</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VLabel[0]</td>
-	<td>2. MenuItem Help selected</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]#menu1</td>
-	<td>4,7</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>left</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>down</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>down</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>enter</td>
-</tr>
-<tr>
-	<td>assertText</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VLabel[0]</td>
-	<td>3. MenuItem Edit/Cut selected</td>
-</tr>
-<tr>
-	<td>mouseClick</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]#menu1</td>
-	<td>4,7</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>enter</td>
-</tr>
-<tr>
-	<td>assertText</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::PID_SLog_row_0</td>
-	<td>3. MenuItem Edit/Cut selected</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>enter</td>
-</tr>
-<tr>
-	<td>assertText</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::PID_SLog_row_0</td>
-	<td>4. MenuItem Edit/Copy selected</td>
-</tr>
-<!--Enter while menubar has focus but no selection should focus "File"-->
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>enter</td>
-</tr>
-<tr>
-	<td>assertText</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::PID_SLog_row_0</td>
-	<td>4. MenuItem Edit/Copy selected</td>
-</tr>
-<!--Enter again should open File and focus Open-->
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>enter</td>
-</tr>
-<tr>
-	<td>assertText</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::PID_SLog_row_0</td>
-	<td>4. MenuItem Edit/Copy selected</td>
-</tr>
-<tr>
-	<td>pressSpecialKey</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-	<td>enter</td>
-</tr>
-<tr>
-	<td>assertText</td>
-	<td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::PID_SLog_row_0</td>
-	<td>5. MenuItem File/Open selected</td>
-</tr>
-</tbody></table>
-</body>
-</html>