[merge from 6.7] Fixed NPE:s in keyboard navigation in MenuBar (#8559/#8560)

svn changeset:23289/svn branch:6.8

diff --git a/src/com/vaadin/terminal/gwt/client/ui/VMenuBar.java b/src/com/vaadin/terminal/gwt/client/ui/VMenuBar.java
index 4ac7fdce451bdda22329387029e8ff56feb60e13..f4bacc05b578ce71ab991830b73e03c38c7ae6e1 100644 (file)
--- a/src/com/vaadin/terminal/gwt/client/ui/VMenuBar.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/VMenuBar.java
@@ -1435,12 +1435,7 @@ public class VMenuBar extends SimpleFocusablePanel implements Paintable,
                 }
 
                 root.setSelected(selected);
-                root.showChildMenu(selected);
-                VMenuBar submenu = selected.getSubMenu();
-
-                // Select the first item in the newly open submenu
-                submenu.setSelected(submenu.getItems().get(0));
-
+                openMenuAndFocusFirstIfPossible(selected);
             } else {
                 getParentMenu().getSelected().getSubMenu().setSelected(null);
                 getParentMenu().hideChildren();
@@ -1498,12 +1493,7 @@ public class VMenuBar extends SimpleFocusablePanel implements Paintable,
                 }
 
                 root.setSelected(selected);
-                root.showChildMenu(selected);
-                VMenuBar submenu = selected.getSubMenu();
-
-                // Select the first item in the newly open submenu
-                submenu.setSelected(submenu.getItems().get(0));
-
+                openMenuAndFocusFirstIfPossible(selected);
             } else if (visibleChildMenu != null) {
                 // Redirect all navigation to the submenu
                 visibleChildMenu.handleNavigation(keycode, ctrl, shift);
@@ -1542,16 +1532,11 @@ public class VMenuBar extends SimpleFocusablePanel implements Paintable,
 
             if (getSelected() == null) {
                 // If nothing is selected then select the first item
-                setSelected(items.get(0));
-                if (!getSelected().isSelectable()) {
-                    handleNavigation(keycode, ctrl, shift);
-                }
+                selectFirstItem();
             } else if (visibleChildMenu == null && getParentMenu() == null) {
                 // If this is the root menu the show the child menu with arrow
-                // down
-                showChildMenu(getSelected());
-                menuVisible = true;
-                visibleChildMenu.handleNavigation(keycode, ctrl, shift);
+                // down, if there is a child menu
+                openMenuAndFocusFirstIfPossible(getSelected());
             } else if (visibleChildMenu != null) {
                 // Redirect all navigation to the submenu
                 visibleChildMenu.handleNavigation(keycode, ctrl, shift);
@@ -1576,17 +1561,18 @@ public class VMenuBar extends SimpleFocusablePanel implements Paintable,
             menuVisible = false;
 
         } else if (keycode == getNavigationSelectKey()) {
-            if (visibleChildMenu != null) {
+            if (getSelected() == null) {
+                // If nothing is selected then select the first item
+                selectFirstItem();
+            } else if (visibleChildMenu != null) {
                 // Redirect all navigation to the submenu
                 visibleChildMenu.handleNavigation(keycode, ctrl, shift);
                 menuVisible = false;
             } else if (visibleChildMenu == null
                     && getSelected().getSubMenu() != null) {
-                // If the item has a submenu then show it and move the selection
-                // there
-                showChildMenu(getSelected());
-                menuVisible = true;
-                visibleChildMenu.handleNavigation(keycode, ctrl, shift);
+                // If the item has a sub menu then show it and move the
+                // selection there
+                openMenuAndFocusFirstIfPossible(getSelected());
             } else {
                 Command command = getSelected().getCommand();
                 if (command != null) {
@@ -1601,6 +1587,34 @@ public class VMenuBar extends SimpleFocusablePanel implements Paintable,
         return false;
     }
 
+    private void selectFirstItem() {
+        for (int i = 0; i < items.size(); i++) {
+            CustomMenuItem item = items.get(i);
+            if (!item.isSelectable()) {
+                continue;
+            }
+
+            setSelected(item);
+            break;
+        }
+    }
+
+    private void openMenuAndFocusFirstIfPossible(CustomMenuItem menuItem) {
+        VMenuBar subMenu = menuItem.getSubMenu();
+        if (subMenu == null) {
+            // No child menu? Nothing to do
+            return;
+        }
+
+        VMenuBar parentMenu = menuItem.getParentMenu();
+        parentMenu.showChildMenu(menuItem);
+
+        menuVisible = true;
+        // Select the first item in the newly open submenu
+        subMenu.setSelected(subMenu.getItems().get(0));
+
+    }
+
     /*
      * (non-Javadoc)
      * 

diff --git a/tests/testbench/com/vaadin/tests/components/menubar/MenuBarNavigation.java b/tests/testbench/com/vaadin/tests/components/menubar/MenuBarNavigation.java
index 7e801a9fafc5cd98a9bc6a6548a85a71b6214d71..ede0c0c5a5db54de4c0fbd24ca479639d4e23ea2 100644 (file)
--- a/tests/testbench/com/vaadin/tests/components/menubar/MenuBarNavigation.java
+++ b/tests/testbench/com/vaadin/tests/components/menubar/MenuBarNavigation.java
@@ -34,7 +34,6 @@ public class MenuBarNavigation extends TestBase implements Command {
         addComponent(mb);
 
         log = new Log(5);
-        log.setNumberLogRows(false);
         addComponent(log);
     }
 

diff --git a/tests/testbench/com/vaadin/tests/components/menubar/MenuBarNavigationKeyboard.html b/tests/testbench/com/vaadin/tests/components/menubar/MenuBarNavigationKeyboard.html
index 9791c95f80e78fcff7a602f834435c83e34a73a8..6a7dccb6ede304e1532c6756db5f072c7e2eaace 100644 (file)
--- a/tests/testbench/com/vaadin/tests/components/menubar/MenuBarNavigationKeyboard.html
+++ b/tests/testbench/com/vaadin/tests/components/menubar/MenuBarNavigationKeyboard.html
@@ -16,85 +16,45 @@
        <td>/run/com.vaadin.tests.components.menubar.MenuBarNavigation?restartApplication</td>
        <td></td>
 </tr>
-<tr>
-       <td>waitForVaadin</td>
-       <td></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>waitForVaadin</td>
-       <td></td>
-       <td></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>waitForVaadin</td>
-       <td></td>
-       <td></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>waitForVaadin</td>
-       <td></td>
-       <td></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>waitForVaadin</td>
-       <td></td>
-       <td></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>waitForVaadin</td>
-       <td></td>
-       <td></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>waitForVaadin</td>
-       <td></td>
-       <td></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>waitForVaadin</td>
-       <td></td>
-       <td></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>MenuItem File/Export../As PDF... selected</td>
+       <td>1. MenuItem File/Export../As PDF... selected</td>
 </tr>
 <tr>
        <td>mouseClick</td>
@@ -102,9 +62,9 @@
        <td>0,7</td>
 </tr>
 <tr>
-       <td>waitForVaadin</td>
-       <td></td>
-       <td></td>
+       <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>
@@ -112,101 +72,107 @@
        <td>right</td>
 </tr>
 <tr>
-       <td>waitForVaadin</td>
-       <td></td>
-       <td></td>
+       <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>
+       <td>enter</td>
 </tr>
 <tr>
-       <td>waitForVaadin</td>
-       <td></td>
-       <td></td>
+       <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>right</td>
+       <td>left</td>
 </tr>
 <tr>
-       <td>waitForVaadin</td>
-       <td></td>
-       <td></td>
+       <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>
+       <td>down</td>
 </tr>
 <tr>
-       <td>waitForVaadin</td>
-       <td></td>
-       <td></td>
+       <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>MenuItem Help selected</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>waitForVaadin</td>
-       <td></td>
-       <td></td>
-</tr>
 <tr>
        <td>pressSpecialKey</td>
-       <td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[0]/VMenuBar[0]</td>
-       <td>left</td>
+       <td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/VVerticalLayout[0]/VMenuBar[0]</td>
+       <td>enter</td>
 </tr>
 <tr>
-       <td>waitForVaadin</td>
-       <td></td>
-       <td></td>
+       <td>assertText</td>
+       <td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/VVerticalLayout[0]/VVerticalLayout[0]/VLabel[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>down</td>
+       <td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/VVerticalLayout[0]/VMenuBar[0]</td>
+       <td>enter</td>
 </tr>
 <tr>
-       <td>waitForVaadin</td>
-       <td></td>
-       <td></td>
+       <td>assertText</td>
+       <td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/VVerticalLayout[0]/VVerticalLayout[0]/VLabel[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>down</td>
+       <td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/VVerticalLayout[0]/VMenuBar[0]</td>
+       <td>enter</td>
 </tr>
 <tr>
-       <td>waitForVaadin</td>
-       <td></td>
-       <td></td>
+       <td>assertText</td>
+       <td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/VVerticalLayout[0]/VVerticalLayout[0]/VLabel[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>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/VVerticalLayout[0]/VMenuBar[0]</td>
        <td>enter</td>
 </tr>
 <tr>
-       <td>waitForVaadin</td>
-       <td></td>
-       <td></td>
+       <td>assertText</td>
+       <td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/VVerticalLayout[0]/VVerticalLayout[0]/VLabel[0]</td>
+       <td>4. MenuItem Edit/Copy selected</td>
+</tr>
+<tr>
+       <td>pressSpecialKey</td>
+       <td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/VVerticalLayout[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>MenuItem Edit/Cut selected</td>
+       <td>vaadin=runcomvaadintestscomponentsmenubarMenuBarNavigation::/VVerticalLayout[0]/VVerticalLayout[0]/VVerticalLayout[0]/VLabel[0]</td>
+       <td>5. MenuItem File/Open selected</td>
 </tr>
-
 </tbody></table>
 </body>
 </html>