Bug fix menu navigation with space (#14041)

Change-Id: I1466381b12a147fea90240420d3f6c05cd156a1b

3 files changed, 77 insertions, 43 deletions

diff --git a/client/src/com/vaadin/client/Util.java b/client/src/com/vaadin/client/Util.java
index e031b37422..f12a02c64f 100644
--- a/client/src/com/vaadin/client/Util.java
+++ b/client/src/com/vaadin/client/Util.java
@@ -35,6 +35,7 @@ import com.google.gwt.dom.client.Style;
 import com.google.gwt.dom.client.Style.Display;
 import com.google.gwt.dom.client.Style.Unit;
 import com.google.gwt.dom.client.Touch;
+import com.google.gwt.event.dom.client.KeyEvent;
 import com.google.gwt.user.client.Command;
 import com.google.gwt.user.client.DOM;
 import com.google.gwt.user.client.Event;
@@ -66,7 +67,23 @@ public class Util {
     }-*/;
 
     /**
-     * 
+     * Helper method for a bug fix #14041. For mozilla getKeyCode return 0 for
+     * space bar (because space is considered as char). If return 0 use
+     * getCharCode.
+     *
+     * @param event
+     * @return return key code
+     */
+    public static int getKeyCode(KeyEvent<?> event) {
+        int keyCode = event.getNativeEvent().getKeyCode();
+        if (keyCode == 0) {
+            keyCode = event.getNativeEvent().getCharCode();
+        }
+        return keyCode;
+    }
+
+    /**
+     *
      * Returns the topmost element of from given coordinates.
      * 
      * TODO fix crossplat issues clientX vs pageX. See quircksmode. Not critical
diff --git a/client/src/com/vaadin/client/ui/VMenuBar.java b/client/src/com/vaadin/client/ui/VMenuBar.java
index 11fda6222c..909b25f7fb 100644
--- a/client/src/com/vaadin/client/ui/VMenuBar.java
+++ b/client/src/com/vaadin/client/ui/VMenuBar.java
@@ -1,12 +1,12 @@
 /*
  * Copyright 2000-2014 Vaadin Ltd.
- * 
+ *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
  * use this file except in compliance with the License. You may obtain a copy of
  * the License at
- * 
+ *
  * http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
  * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
@@ -243,7 +243,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
     /**
      * This is called by the items in the menu and it communicates the
      * information to the server
-     * 
+     *
      * @param clickedItemId
      *            id of the item that was clicked
      */
@@ -280,7 +280,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
     /**
      * Returns the containing element of the menu
-     * 
+     *
      * @return
      */
     @Override
@@ -290,7 +290,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
     /**
      * Add a new item to this menu
-     * 
+     *
      * @param html
      *            items text
      * @param cmd
@@ -307,7 +307,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
     /**
      * Add a new item to this menu
-     * 
+     *
      * @param item
      */
     public void addItem(CustomMenuItem item) {
@@ -332,7 +332,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
     /**
      * Remove the given item from this menu
-     * 
+     *
      * @param item
      */
     public void removeItem(CustomMenuItem item) {
@@ -429,7 +429,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
     /**
      * When an item is clicked
-     * 
+     *
      * @param item
      */
     public void itemClick(CustomMenuItem item) {
@@ -460,7 +460,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
     /**
      * When the user hovers the mouse over the item
-     * 
+     *
      * @param item
      */
     public void itemOver(CustomMenuItem item) {
@@ -485,7 +485,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
     /**
      * When the mouse is moved away from an item
-     * 
+     *
      * @param item
      */
     public void itemOut(CustomMenuItem item) {
@@ -543,7 +543,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
     /**
      * Shows the child menu of an item. The caller must ensure that the item has
      * a submenu.
-     * 
+     *
      * @param item
      */
     public void showChildMenu(CustomMenuItem item) {
@@ -665,7 +665,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
     /**
      * Hides the submenu of an item
-     * 
+     *
      * @param item
      */
     public void hideChildMenu(CustomMenuItem item) {
@@ -729,7 +729,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
     /**
      * Returns the parent menu of this menu, or null if this is the top-level
      * menu
-     * 
+     *
      * @return
      */
     public VMenuBar getParentMenu() {
@@ -738,7 +738,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
     /**
      * Set the parent menu of this menu
-     * 
+     *
      * @param parent
      */
     public void setParentMenu(VMenuBar parent) {
@@ -748,7 +748,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
     /**
      * Returns the currently selected item of this menu, or null if nothing is
      * selected
-     * 
+     *
      * @return
      */
     public CustomMenuItem getSelected() {
@@ -757,7 +757,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
     /**
      * Set the currently selected item of this menu
-     * 
+     *
      * @param item
      */
     public void setSelected(CustomMenuItem item) {
@@ -774,9 +774,9 @@ public class VMenuBar extends SimpleFocusablePanel implements
     }
 
     /**
-     * 
+     *
      * A class to hold information on menu items
-     * 
+     *
      */
     public static class CustomMenuItem extends Widget implements HasHTML {
 
@@ -795,7 +795,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
         /**
          * Default menu item {@link Widget} constructor for GWT.create().
-         * 
+         *
          * Use {@link #setHTML(String)} and {@link #setCommand(Command)} after
          * constructing a menu item.
          */
@@ -805,7 +805,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
         /**
          * Creates a menu item {@link Widget}.
-         * 
+         *
          * @param html
          * @param cmd
          * @deprecated use the default constructor and {@link #setHTML(String)}
@@ -1039,7 +1039,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
         /**
          * Checks if the item can be selected.
-         * 
+         *
          * @return true if it is possible to select this item, false otherwise
          */
         public boolean isSelectable() {
@@ -1157,7 +1157,14 @@ public class VMenuBar extends SimpleFocusablePanel implements
      */
     @Override
     public void onKeyPress(KeyPressEvent event) {
-        if (handleNavigation(event.getNativeEvent().getKeyCode(),
+        // A bug fix for #14041
+        // getKeyCode and getCharCode return different values for different
+        // browsers
+        int keyCode = event.getNativeEvent().getKeyCode();
+        if (keyCode == 0) {
+            keyCode = event.getNativeEvent().getCharCode();
+        }
+        if (handleNavigation(keyCode,
                 event.isControlKeyDown() || event.isMetaKeyDown(),
                 event.isShiftKeyDown())) {
             event.preventDefault();
@@ -1173,7 +1180,14 @@ public class VMenuBar extends SimpleFocusablePanel implements
      */
     @Override
     public void onKeyDown(KeyDownEvent event) {
-        if (handleNavigation(event.getNativeEvent().getKeyCode(),
+        // A bug fix for #14041
+        // getKeyCode and getCharCode return different values for different
+        // browsers
+        int keyCode = event.getNativeEvent().getKeyCode();
+        if (keyCode == 0) {
+            keyCode = event.getNativeEvent().getCharCode();
+        }
+        if (handleNavigation(keyCode,
                 event.isControlKeyDown() || event.isMetaKeyDown(),
                 event.isShiftKeyDown())) {
             event.preventDefault();
@@ -1184,7 +1198,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
      * Get the key that moves the selection upwards. By default it is the up
      * arrow key but by overriding this you can change the key to whatever you
      * want.
-     * 
+     *
      * @return The keycode of the key
      */
     protected int getNavigationUpKey() {
@@ -1195,7 +1209,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
      * Get the key that moves the selection downwards. By default it is the down
      * arrow key but by overriding this you can change the key to whatever you
      * want.
-     * 
+     *
      * @return The keycode of the key
      */
     protected int getNavigationDownKey() {
@@ -1206,7 +1220,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
      * Get the key that moves the selection left. By default it is the left
      * arrow key but by overriding this you can change the key to whatever you
      * want.
-     * 
+     *
      * @return The keycode of the key
      */
     protected int getNavigationLeftKey() {
@@ -1217,7 +1231,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
      * Get the key that moves the selection right. By default it is the right
      * arrow key but by overriding this you can change the key to whatever you
      * want.
-     * 
+     *
      * @return The keycode of the key
      */
     protected int getNavigationRightKey() {
@@ -1227,7 +1241,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
     /**
      * Get the key that selects a menu item. By default it is the Enter key but
      * by overriding this you can change the key to whatever you want.
-     * 
+     *
      * @deprecated use {@link #isNavigationSelectKey(int)} instead
      * @return
      */
@@ -1240,7 +1254,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
      * Checks whether key code selects a menu item. By default it is the Enter
      * and Space keys but by overriding this you can change the keys to whatever
      * you want.
-     * 
+     *
      * @since 7.2
      * @param keycode
      * @return true if key selects menu item
@@ -1253,7 +1267,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
     /**
      * Get the key that closes the menu. By default it is the escape key but by
      * overriding this yoy can change the key to whatever you want.
-     * 
+     *
      * @return
      */
     protected int getCloseMenuKey() {
@@ -1262,7 +1276,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
     /**
      * Handles the keyboard events handled by the MenuBar
-     * 
+     *
      * @param event
      *            The keyboard event received
      * @return true iff the navigation event was handled
@@ -1549,7 +1563,7 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
     /**
      * Get menu item with given DOM element
-     * 
+     *
      * @param element
      *            Element used in search
      * @return Menu item or null if not found
@@ -1578,11 +1592,11 @@ public class VMenuBar extends SimpleFocusablePanel implements
 
     /**
      * Get menu item with given DOM element
-     * 
+     *
      * @param element
      *            Element used in search
      * @return Menu item or null if not found
-     * 
+     *
      * @since 7.2
      */
     public CustomMenuItem getMenuItemWithElement(Element element) {
diff --git a/uitest/src/com/vaadin/tests/components/menubar/SpaceMenuBarNavigationTest.java b/uitest/src/com/vaadin/tests/components/menubar/SpaceMenuBarNavigationTest.java
index 3e0053d0d1..a32a611d9a 100644
--- a/uitest/src/com/vaadin/tests/components/menubar/SpaceMenuBarNavigationTest.java
+++ b/uitest/src/com/vaadin/tests/components/menubar/SpaceMenuBarNavigationTest.java
@@ -1,12 +1,12 @@
 /*
  * Copyright 2000-2014 Vaadin Ltd.
- * 
+ *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
  * use this file except in compliance with the License. You may obtain a copy of
  * the License at
- * 
+ *
  * http://www.apache.org/licenses/LICENSE-2.0
- * 
+ *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
  * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
@@ -23,10 +23,11 @@ import org.openqa.selenium.Keys;
 import org.openqa.selenium.WebElement;
 
 import com.vaadin.testbench.By;
+import com.vaadin.testbench.elements.MenuBarElement;
 import com.vaadin.tests.tb3.MultiBrowserTest;
 
 /**
- * 
+ *
  * @since 7.2
  * @author Vaadin Ltd
  */
@@ -36,19 +37,21 @@ public class SpaceMenuBarNavigationTest extends MultiBrowserTest {
     public void testEnableParentLayout() {
         openTestURL();
 
-        WebElement menu = driver.findElement(By.className("menu-bar"));
+        MenuBarElement menu = $(MenuBarElement.class).get(0);
+        menu.focus();
         menu.sendKeys(Keys.ARROW_RIGHT);
-        menu.sendKeys(Keys.SPACE);
+        menu.sendKeys(Keys.ENTER);
 
         List<WebElement> captions = driver.findElements(By
                 .className("v-menubar-menuitem-caption"));
         boolean found = false;
+
         for (WebElement caption : captions) {
             if ("subitem".equals(caption.getText())) {
                 found = true;
             }
         }
-        Assert.assertTrue("Sub menu is not opened on SPACE key", found);
+        Assert.assertTrue("Sub menu is not opened on ENTER key", found);
 
         menu.sendKeys(Keys.SPACE);