From 9b52497261f0063b5a2d0431b8d7d61930a46161 Mon Sep 17 00:00:00 2001
From: Automerge <automerge@vaadin.com>
Date: Mon, 21 May 2012 13:19:35 +0000
Subject: [PATCH] [merge from 6.7] Don't trigger actions if space or enter is
 pressed with Button focused (#7191)

svn changeset:23774/svn branch:6.8
---
 .../terminal/gwt/client/ui/VButton.java       | 37 ++++-------
 src/com/vaadin/ui/Button.java                 |  3 -
 .../button/ButtonEnterWithWindowShortcut.html | 62 +++++++++++++++++++
 .../button/ButtonEnterWithWindowShortcut.java | 55 ++++++++++++++++
 4 files changed, 129 insertions(+), 28 deletions(-)
 create mode 100644 tests/testbench/com/vaadin/tests/components/button/ButtonEnterWithWindowShortcut.html
 create mode 100644 tests/testbench/com/vaadin/tests/components/button/ButtonEnterWithWindowShortcut.java

diff --git a/src/com/vaadin/terminal/gwt/client/ui/VButton.java b/src/com/vaadin/terminal/gwt/client/ui/VButton.java
index 98103dc41e..e77ad710b1 100644
--- a/src/com/vaadin/terminal/gwt/client/ui/VButton.java
+++ b/src/com/vaadin/terminal/gwt/client/ui/VButton.java
@@ -77,7 +77,9 @@ public class VButton extends FocusWidget implements Paintable, ClickHandler,
     private boolean isCapturing;
 
     /**
-     * If <code>true</code>, this widget has focus with the space bar down.
+     * If <code>true</code>, this widget has focus with the space bar down. This
+     * means that we will get events when the button is released, but we should
+     * trigger the button only if the button is still focused at that point.
      */
     private boolean isFocusing;
 
@@ -91,8 +93,6 @@ public class VButton extends FocusWidget implements Paintable, ClickHandler,
     private HandlerRegistration focusHandlerRegistration;
     private HandlerRegistration blurHandlerRegistration;
 
-    private int clickShortcut = 0;
-
     /**
      * If caption should be rendered in HTML
      */
@@ -174,10 +174,6 @@ public class VButton extends FocusWidget implements Paintable, ClickHandler,
                 icon = null;
             }
         }
-
-        if (uidl.hasAttribute("keycode")) {
-            clickShortcut = uidl.getIntAttribute("keycode");
-        }
     }
 
     public void setText(String text) {
@@ -316,37 +312,28 @@ public class VButton extends FocusWidget implements Paintable, ClickHandler,
         if ((event.getTypeInt() & Event.KEYEVENTS) != 0) {
             switch (type) {
             case Event.ONKEYDOWN:
+                // Stop propagation when the user starts pressing a button that
+                // we are handling to prevent actions from getting triggered
                 if (event.getKeyCode() == 32 /* space */) {
                     isFocusing = true;
                     event.preventDefault();
+                    event.stopPropagation();
+                } else if (event.getKeyCode() == KeyCodes.KEY_ENTER) {
+                    event.stopPropagation();
                 }
                 break;
             case Event.ONKEYUP:
                 if (isFocusing && event.getKeyCode() == 32 /* space */) {
                     isFocusing = false;
-
-                    /*
-                     * If click shortcut is space then the shortcut handler will
-                     * take care of the click.
-                     */
-                    if (clickShortcut != 32 /* space */) {
-                        onClick();
-                    }
-
+                    onClick();
+                    event.stopPropagation();
                     event.preventDefault();
                 }
                 break;
             case Event.ONKEYPRESS:
                 if (event.getKeyCode() == KeyCodes.KEY_ENTER) {
-
-                    /*
-                     * If click shortcut is enter then the shortcut handler will
-                     * take care of the click.
-                     */
-                    if (clickShortcut != KeyCodes.KEY_ENTER) {
-                        onClick();
-                    }
-
+                    onClick();
+                    event.stopPropagation();
                     event.preventDefault();
                 }
                 break;
diff --git a/src/com/vaadin/ui/Button.java b/src/com/vaadin/ui/Button.java
index fdaef046e5..0032db1752 100644
--- a/src/com/vaadin/ui/Button.java
+++ b/src/com/vaadin/ui/Button.java
@@ -158,9 +158,6 @@ public class Button extends AbstractField implements FieldEvents.BlurNotifier,
         if (isDisableOnClick()) {
             target.addAttribute(VButton.ATTR_DISABLE_ON_CLICK, true);
         }
-        if (clickShortcut != null) {
-            target.addAttribute("keycode", clickShortcut.getKeyCode());
-        }
 
         if (isHtmlContentAllowed()) {
             target.addAttribute("html-caption", true);
diff --git a/tests/testbench/com/vaadin/tests/components/button/ButtonEnterWithWindowShortcut.html b/tests/testbench/com/vaadin/tests/components/button/ButtonEnterWithWindowShortcut.html
new file mode 100644
index 0000000000..5ec33f09fa
--- /dev/null
+++ b/tests/testbench/com/vaadin/tests/components/button/ButtonEnterWithWindowShortcut.html
@@ -0,0 +1,62 @@
+<?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.button.ButtonEnterWithWindowShortcut?restartApplication</td>
+	<td></td>
+</tr>
+<tr>
+	<td>pressSpecialKey</td>
+	<td>vaadin=runcomvaadintestscomponentsbuttonButtonEnterWithWindowShortcut::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VButton[0]</td>
+	<td>enter</td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestscomponentsbuttonButtonEnterWithWindowShortcut::PID_SLog_row_0</td>
+	<td>1. button click listener fired</td>
+</tr>
+<tr>
+	<td>pressSpecialKey</td>
+	<td>vaadin=runcomvaadintestscomponentsbuttonButtonEnterWithWindowShortcut::</td>
+	<td>enter</td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestscomponentsbuttonButtonEnterWithWindowShortcut::PID_SLog_row_0</td>
+	<td>2. enter pressed in window</td>
+</tr>
+<!-- Can't test using space because of #8827 --> 
+<!-- <tr>
+	<td>pressSpecialKey</td>
+	<td>vaadin=runcomvaadintestscomponentsbuttonButtonEnterWithWindowShortcut::/VVerticalLayout[0]/ChildComponentContainer[1]/VVerticalLayout[0]/ChildComponentContainer[1]/VButton[0]</td>
+	<td>space</td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestscomponentsbuttonButtonEnterWithWindowShortcut::PID_SLog_row_0</td>
+	<td>3. button click listener fired</td>
+</tr>
+<tr>
+	<td>pressSpecialKey</td>
+	<td>vaadin=runcomvaadintestscomponentsbuttonButtonEnterWithWindowShortcut::</td>
+	<td>space</td>
+</tr>
+<tr>
+	<td>assertText</td>
+	<td>vaadin=runcomvaadintestscomponentsbuttonButtonEnterWithWindowShortcut::PID_SLog_row_0</td>
+	<td>4. space pressed in window</td>
+</tr> -->
+</tbody></table>
+</body>
+</html>
diff --git a/tests/testbench/com/vaadin/tests/components/button/ButtonEnterWithWindowShortcut.java b/tests/testbench/com/vaadin/tests/components/button/ButtonEnterWithWindowShortcut.java
new file mode 100644
index 0000000000..7efd40ca5d
--- /dev/null
+++ b/tests/testbench/com/vaadin/tests/components/button/ButtonEnterWithWindowShortcut.java
@@ -0,0 +1,55 @@
+package com.vaadin.tests.components.button;
+
+import com.vaadin.event.Action;
+import com.vaadin.event.Action.Handler;
+import com.vaadin.event.ShortcutAction;
+import com.vaadin.tests.components.TestBase;
+import com.vaadin.tests.util.Log;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.Button.ClickListener;
+
+public class ButtonEnterWithWindowShortcut extends TestBase {
+    Log log = new Log(5);
+
+    @Override
+    protected void setup() {
+        getMainWindow().addActionHandler(new Handler() {
+            private static final long serialVersionUID = -4976129418325394913L;
+
+            public void handleAction(Action action, Object sender, Object target) {
+                log.log(action.getCaption() + " pressed in window");
+            }
+
+            public Action[] getActions(Object target, Object sender) {
+                ShortcutAction enter = new ShortcutAction("enter",
+                        ShortcutAction.KeyCode.ENTER, null);
+                ShortcutAction space = new ShortcutAction("space",
+                        ShortcutAction.KeyCode.SPACEBAR, null);
+                return new Action[] { enter, space };
+            }
+        });
+
+        Button button = new Button("Focus me and press enter",
+                new ClickListener() {
+                    public void buttonClick(ClickEvent event) {
+                        log.log("button click listener fired");
+                    }
+                });
+        button.focus();
+
+        addComponent(log);
+        addComponent(button);
+    }
+
+    @Override
+    protected String getDescription() {
+        return "Pressing enter or space with the button focused should trigger the button click listener and not the shortcut action on the window.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return Integer.valueOf(5433);
+    }
+
+}
-- 
2.39.5