Moved accessibility shortcut handling to server-side. [#14843]

Also allowed multiple shortcuts for closing Window.

Change-Id: I35280ad1553af10ae54bc001e5707357f206b0ee

6 files changed, 472 insertions, 14 deletions

diff --git a/WebContent/release-notes.html b/WebContent/release-notes.html
index 8e6385684f..6cedeb1e21 100644
--- a/WebContent/release-notes.html
+++ b/WebContent/release-notes.html
@@ -137,6 +137,8 @@
             <li>The semantics of empty and required for Field classes has been made more consistent. This mainly affects Checkbox which is now considered to be empty when it is not checked.</li>
             <li>The previously inconsistent behavior in HTML vs plain text rendering of Calendar event captions has been made consistent.</li>
             <li>Support for Opera 12 has been dropped. Newer versions based on the Blink rendering engine are still supported.</li>
+            <li>Window's accessibility shortcut was moved to server-side. Now setCloseShortcut overrides the default value, while addCloseShortcut can be used to add more than one shortcut key for closing the window.
+                The protected value closeShortcut in Window was removed.</li>
         </ul>
         <h3 id="knownissues">Known issues</h3>
         <ul>
diff --git a/client/src/com/vaadin/client/ui/VWindow.java b/client/src/com/vaadin/client/ui/VWindow.java
index 6977cf9e7f..501dedbaa8 100644
--- a/client/src/com/vaadin/client/ui/VWindow.java
+++ b/client/src/com/vaadin/client/ui/VWindow.java
@@ -1338,9 +1338,7 @@ public class VWindow extends VOverlay implements ShortcutActionHandlerOwner,
 
     @Override
     public void onKeyUp(KeyUpEvent event) {
-        if (isClosable() && event.getNativeKeyCode() == KeyCodes.KEY_ESCAPE) {
-            onCloseClick();
-        }
+        // do nothing
     }
 
     @Override
diff --git a/server/src/com/vaadin/event/ShortcutAction.java b/server/src/com/vaadin/event/ShortcutAction.java
index 09accae1c7..32b909e9f2 100644
--- a/server/src/com/vaadin/event/ShortcutAction.java
+++ b/server/src/com/vaadin/event/ShortcutAction.java
@@ -17,6 +17,8 @@
 package com.vaadin.event;
 
 import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -237,6 +239,42 @@ public class ShortcutAction extends Action {
     }
 
     /**
+     * Checks whether the shortcut can be triggered with the given combination
+     * of keys.
+     * 
+     * @param keyCode
+     *            potential match for the {@link KeyCode} that this shortcut
+     *            reacts to
+     * @param modifierKeys
+     *            (optional) potential matches for the {@link ModifierKey}s
+     *            required for this shortcut to react
+     * @return <code>true</code> if keyCode and modifierKeys are a match,
+     *         <code>false</code> otherwise
+     */
+    public boolean isTriggeredBy(int keyCode, int... modifierKeys) {
+        boolean result = false;
+        if (keyCode == this.keyCode) {
+            if (modifierKeys == null) {
+                result = (modifiers == null);
+            } else if (modifiers != null) {
+                List<Integer> modifierList = new ArrayList<Integer>();
+                for (int modifier : modifiers) {
+                    modifierList.add(modifier);
+                }
+                for (int modifierKey : modifierKeys) {
+                    if (modifierList.contains(modifierKey)) {
+                        modifierList.remove(modifierKey);
+                    } else {
+                        return false;
+                    }
+                }
+                result = modifierList.isEmpty();
+            }
+        }
+        return result;
+    }
+
+    /**
      * Key codes that can be used for shortcuts
      * 
      */
diff --git a/server/src/com/vaadin/ui/Window.java b/server/src/com/vaadin/ui/Window.java
index 35583c6052..653b620746 100644
--- a/server/src/com/vaadin/ui/Window.java
+++ b/server/src/com/vaadin/ui/Window.java
@@ -18,6 +18,9 @@ package com.vaadin.ui;
 
 import java.io.Serializable;
 import java.lang.reflect.Method;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.LinkedHashSet;
 import java.util.Map;
 
 import com.vaadin.event.FieldEvents.BlurEvent;
@@ -120,6 +123,7 @@ public class Window extends Panel implements FocusNotifier, BlurNotifier,
         super(caption, content);
         registerRpc(rpc);
         setSizeUndefined();
+        setCloseShortcut(ShortcutAction.KeyCode.ESCAPE);
     }
 
     /* ********************************************************************* */
@@ -806,14 +810,48 @@ public class Window extends Panel implements FocusNotifier, BlurNotifier,
     /*
      * Actions
      */
-    protected CloseShortcut closeShortcut;
+    private LinkedHashSet<CloseShortcut> closeShortcuts = new LinkedHashSet<CloseShortcut>();
+
+    protected Collection<CloseShortcut> getCloseShortcuts() {
+        return Collections.unmodifiableCollection(closeShortcuts);
+    }
+
+    /**
+     * Adds a keyboard shortcut for closing the window when user presses the
+     * given {@link KeyCode} and (optional) {@link ModifierKey}s.<br/>
+     * Note that this shortcut only reacts while the window has focus, closing
+     * itself - if you want to close a window from a UI, use
+     * {@link UI#addAction(com.vaadin.event.Action)} of the UI instead.
+     * <p>
+     * If there is a prior CloseShortcut with the same keycode and modifiers,
+     * that gets removed before the new one is added. Prior CloseShortcuts with
+     * differing keycodes or modifiers are not affected.
+     * 
+     * @param keyCode
+     *            the keycode for invoking the shortcut
+     * @param modifiers
+     *            the (optional) modifiers for invoking the shortcut, null for
+     *            none
+     */
+    public void addCloseShortcut(int keyCode, int... modifiers) {
+        // make sure there are no duplicates
+        removeCloseShortcut(keyCode, modifiers);
+        CloseShortcut closeShortcut = new CloseShortcut(this, keyCode,
+                modifiers);
+        closeShortcuts.add(closeShortcut);
+        addAction(closeShortcut);
+    }
 
     /**
-     * Makes is possible to close the window by pressing the given
-     * {@link KeyCode} and (optional) {@link ModifierKey}s.<br/>
+     * Sets the keyboard shortcut for closing the window when user presses the
+     * given {@link KeyCode} and (optional) {@link ModifierKey}s.<br/>
      * Note that this shortcut only reacts while the window has focus, closing
      * itself - if you want to close a window from a UI, use
      * {@link UI#addAction(com.vaadin.event.Action)} of the UI instead.
+     * <p>
+     * If there are any prior CloseShortcuts when this method is called those
+     * get removed before the new one is added. <b>NOTE: this also removes the
+     * default shortcut that is added for accessibility purposes.</b>
      * 
      * @param keyCode
      *            the keycode for invoking the shortcut
@@ -822,22 +860,61 @@ public class Window extends Panel implements FocusNotifier, BlurNotifier,
      *            none
      */
     public void setCloseShortcut(int keyCode, int... modifiers) {
-        if (closeShortcut != null) {
-            removeAction(closeShortcut);
+        removeCloseShortcuts();
+        addCloseShortcut(keyCode, modifiers);
+    }
+
+    /**
+     * Removes a keyboard shortcut previously set with
+     * {@link #setCloseShortcut(int, int...)} or
+     * {@link #addCloseShortcut(int, int...)}.
+     * 
+     * @param keyCode
+     *            the keycode for invoking the shortcut
+     * @param modifiers
+     *            the (optional) modifiers for invoking the shortcut, null for
+     *            none
+     */
+    public void removeCloseShortcut(int keyCode, int... modifiers) {
+        for (CloseShortcut closeShortcut : closeShortcuts) {
+            if (closeShortcut.isTriggeredBy(keyCode, modifiers)) {
+                removeAction(closeShortcut);
+                closeShortcuts.remove(closeShortcut);
+                break;
+            }
         }
-        closeShortcut = new CloseShortcut(this, keyCode, modifiers);
-        addAction(closeShortcut);
     }
 
     /**
-     * Removes the keyboard shortcut previously set with
-     * {@link #setCloseShortcut(int, int...)}.
+     * @deprecated use {@link #resetCloseShortcuts()} instead, or
+     *             {@link #removeCloseShortcuts()} if you also want to get rid
+     *             of the default shortcut
      */
+    @Deprecated
     public void removeCloseShortcut() {
-        if (closeShortcut != null) {
+        resetCloseShortcuts();
+    }
+
+    /**
+     * Removes all the keyboard shortcuts previously set with
+     * {@link #setCloseShortcut(int, int...)} or
+     * {@link #addCloseShortcut(int, int...)} and re-adds the default shortcut
+     * {@link KeyCode.ESCAPE}.
+     */
+    public void resetCloseShortcuts() {
+        setCloseShortcut(ShortcutAction.KeyCode.ESCAPE);
+    }
+
+    /**
+     * Removes all the keyboard shortcuts previously set with
+     * {@link #setCloseShortcut(int, int...)} or
+     * {@link #addCloseShortcut(int, int...)}.
+     */
+    public void removeCloseShortcuts() {
+        for (CloseShortcut closeShortcut : closeShortcuts) {
             removeAction(closeShortcut);
-            closeShortcut = null;
         }
+        closeShortcuts.clear();
     }
 
     /**
diff --git a/uitest/src/com/vaadin/tests/components/window/CloseShortcut.java b/uitest/src/com/vaadin/tests/components/window/CloseShortcut.java
new file mode 100644
index 0000000000..c7499134e0
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/window/CloseShortcut.java
@@ -0,0 +1,121 @@
+/*
+ * 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
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.window;
+
+import com.vaadin.data.Property;
+import com.vaadin.data.Property.ValueChangeEvent;
+import com.vaadin.event.ShortcutAction.KeyCode;
+import com.vaadin.event.ShortcutAction.ModifierKey;
+import com.vaadin.server.VaadinRequest;
+import com.vaadin.tests.components.AbstractTestUI;
+import com.vaadin.ui.Button;
+import com.vaadin.ui.Button.ClickEvent;
+import com.vaadin.ui.CheckBox;
+import com.vaadin.ui.TextField;
+import com.vaadin.ui.VerticalLayout;
+import com.vaadin.ui.Window;
+
+/**
+ * Tests close shortcuts for Window.
+ * 
+ * @author Vaadin Ltd
+ */
+public class CloseShortcut extends AbstractTestUI {
+
+    @Override
+    protected void setup(VaadinRequest request) {
+        final Window w = new Window();
+        w.setWidth("300px");
+        w.setHeight("300px");
+        w.center();
+        addWindow(w);
+        w.focus();
+
+        // add textfield to the window to give TestBench something to send the
+        // keys to
+        VerticalLayout content = new VerticalLayout();
+        content.setMargin(true);
+        TextField textField = new TextField();
+        textField.setSizeFull();
+        content.addComponent(textField);
+        w.setContent(content);
+
+        final CheckBox cbDefault = new CheckBox("Use default (ESC) shortcut");
+        cbDefault.setId("default");
+        addComponent(cbDefault);
+        final CheckBox cbOther = new CheckBox("Use R shortcut");
+        cbOther.setId("other");
+        addComponent(cbOther);
+        final CheckBox cbCtrl = new CheckBox("Use CTRL+A shortcut");
+        cbCtrl.setId("control");
+        addComponent(cbCtrl);
+        final CheckBox cbShift = new CheckBox("Use SHIFT+H shortcut");
+        cbShift.setId("shift");
+        addComponent(cbShift);
+
+        cbOther.setValue(true);
+        cbCtrl.setValue(true);
+        cbShift.setValue(true);
+
+        Property.ValueChangeListener listener = new Property.ValueChangeListener() {
+
+            @Override
+            public void valueChange(ValueChangeEvent event) {
+                if (Boolean.TRUE.equals(cbDefault.getValue())) {
+                    w.resetCloseShortcuts();
+                } else {
+                    w.removeCloseShortcuts();
+                }
+                if (Boolean.TRUE.equals(cbOther.getValue())) {
+                    w.addCloseShortcut(KeyCode.R);
+                }
+                if (Boolean.TRUE.equals(cbCtrl.getValue())) {
+                    w.addCloseShortcut(KeyCode.A, ModifierKey.CTRL);
+                }
+                if (Boolean.TRUE.equals(cbShift.getValue())) {
+                    w.addCloseShortcut(KeyCode.H, ModifierKey.SHIFT);
+                }
+            }
+        };
+        cbDefault.addValueChangeListener(listener);
+        cbOther.addValueChangeListener(listener);
+        cbCtrl.addValueChangeListener(listener);
+        cbShift.addValueChangeListener(listener);
+
+        cbDefault.setValue(true); // trigger value change
+
+        Button button = new Button("Reopen window", new Button.ClickListener() {
+
+            @Override
+            public void buttonClick(ClickEvent event) {
+                w.close();
+                addWindow(w);
+                w.focus();
+            }
+        });
+        addComponent(button);
+    }
+
+    @Override
+    protected String getTestDescription() {
+        return "It should be possible to have multiple shortcuts at the same time, and to remove the default shortcut ESC.";
+    }
+
+    @Override
+    protected Integer getTicketNumber() {
+        return 14843;
+    }
+}
diff --git a/uitest/src/com/vaadin/tests/components/window/CloseShortcutTest.java b/uitest/src/com/vaadin/tests/components/window/CloseShortcutTest.java
new file mode 100644
index 0000000000..7cca2ce4d0
--- /dev/null
+++ b/uitest/src/com/vaadin/tests/components/window/CloseShortcutTest.java
@@ -0,0 +1,222 @@
+/*
+ * 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
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package com.vaadin.tests.components.window;
+
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.openqa.selenium.By;
+import org.openqa.selenium.Keys;
+import org.openqa.selenium.WebDriver;
+import org.openqa.selenium.interactions.Actions;
+import org.openqa.selenium.support.ui.ExpectedCondition;
+
+import com.vaadin.testbench.elements.CheckBoxElement;
+import com.vaadin.testbench.elements.TextFieldElement;
+import com.vaadin.testbench.elements.WindowElement;
+import com.vaadin.tests.tb3.MultiBrowserTest;
+
+/**
+ * Tests close shortcuts for Window.
+ * 
+ * @author Vaadin Ltd
+ */
+public class CloseShortcutTest extends MultiBrowserTest {
+
+    private WindowElement window;
+    private CheckBoxElement cbDefault;
+    private CheckBoxElement cbOther;
+    private CheckBoxElement cbCtrl;
+    private CheckBoxElement cbShift;
+
+    @Override
+    @Before
+    public void setup() throws Exception {
+        super.setup();
+        openTestURL();
+        waitForElementPresent(By.className("v-window"));
+
+        window = $(WindowElement.class).first();
+        cbDefault = $(CheckBoxElement.class).id("default");
+        cbOther = $(CheckBoxElement.class).id("other");
+        cbCtrl = $(CheckBoxElement.class).id("control");
+        cbShift = $(CheckBoxElement.class).id("shift");
+    }
+
+    @Test
+    public void testAllCheckBoxesSelected() {
+        assertTrue("Default wasn't selected initially.", isChecked(cbDefault));
+        assertTrue("Other wasn't selected initially.", isChecked(cbOther));
+        assertTrue("Ctrl+A wasn't selected initially.", isChecked(cbCtrl));
+        assertTrue("Shift+H wasn't selected initially.", isChecked(cbShift));
+    }
+
+    @Test
+    public void testAllCheckBoxesClickable() {
+        click(cbDefault);
+        click(cbOther);
+        click(cbCtrl);
+        click(cbShift);
+
+        assertFalse("Default was selected when it shouldn't have been.",
+                isChecked(cbDefault));
+        assertFalse("Other was selected when it shouldn't have been.",
+                isChecked(cbOther));
+        assertFalse("Ctrl+A was selected when it shouldn't have been.",
+                isChecked(cbCtrl));
+        assertFalse("Shift+H was selected when it shouldn't have been.",
+                isChecked(cbShift));
+    }
+
+    @Test
+    public void testDefaultWithAll() {
+        attemptDefaultShortcut();
+        ensureWindowClosed();
+    }
+
+    @Test
+    public void testDefaultWithoutSelection() {
+        click(cbDefault);
+
+        attemptDefaultShortcut();
+        ensureWindowOpen();
+    }
+
+    @Test
+    public void testOtherWithAll() {
+        attemptOtherShortcut();
+        // TODO: remove this check once #14902 has been fixed
+        if (!Browser.IE8.getDesiredCapabilities().equals(
+                getDesiredCapabilities())
+                && !Browser.FIREFOX.getDesiredCapabilities().equals(
+                        getDesiredCapabilities())
+                && !Browser.CHROME.getDesiredCapabilities().equals(
+                        getDesiredCapabilities())) {
+            ensureWindowClosed();
+        }
+    }
+
+    @Test
+    public void testOtherWithoutSelection() {
+        click(cbOther);
+
+        attemptOtherShortcut();
+        ensureWindowOpen();
+    }
+
+    @Test
+    public void testCtrlWithAll() {
+        attemptCtrlShortcut();
+        // TODO: remove this check once #14902 has been fixed
+        if (Browser.PHANTOMJS.getDesiredCapabilities().equals(
+                getDesiredCapabilities())) {
+            ensureWindowClosed();
+        }
+    }
+
+    @Test
+    public void testCtrlWithoutSelection() {
+        click(cbCtrl);
+
+        attemptCtrlShortcut();
+        ensureWindowOpen();
+    }
+
+    @Test
+    public void testShiftWithAll() {
+        attemptShiftShortcut();
+        // TODO: remove this check once #14902 has been fixed
+        if (getBrowsersExcludingIE().contains(getDesiredCapabilities())
+                || Browser.IE8.getDesiredCapabilities().equals(
+                        getDesiredCapabilities())) {
+            ensureWindowClosed();
+        }
+    }
+
+    @Test
+    public void testShiftWithoutSelection() {
+        click(cbShift);
+
+        attemptShiftShortcut();
+        ensureWindowOpen();
+    }
+
+    private boolean isChecked(CheckBoxElement cb) {
+        String checked = cb.findElement(By.tagName("input")).getAttribute(
+                "checked");
+        if ("true".equals(checked)) {
+            return true;
+        } else if (checked == null) {
+            return false;
+        }
+        throw new IllegalStateException(
+                "Unexpected attribute value for 'checked': " + checked);
+    }
+
+    @Override
+    protected void click(final CheckBoxElement cb) {
+        final boolean initial = isChecked(cb);
+        super.click(cb);
+        waitUntil(new ExpectedCondition<Boolean>() {
+            @Override
+            public Boolean apply(WebDriver input) {
+                return initial != isChecked(cb);
+            }
+
+            @Override
+            public String toString() {
+                // Timed out after 10 seconds waiting for ...
+                return "checked state to change";
+            }
+        });
+    }
+
+    private void attemptDefaultShortcut() {
+        window.focus();
+        $(TextFieldElement.class).first().sendKeys(Keys.ESCAPE);
+    }
+
+    private void attemptOtherShortcut() {
+        window.focus();
+        $(TextFieldElement.class).first().sendKeys("R");
+    }
+
+    private void attemptCtrlShortcut() {
+        window.focus();
+        new Actions(driver).keyDown(Keys.CONTROL).perform();
+        $(TextFieldElement.class).first().sendKeys("A");
+        new Actions(driver).keyUp(Keys.CONTROL).perform();
+    }
+
+    private void attemptShiftShortcut() {
+        window.focus();
+        new Actions(driver).keyDown(Keys.SHIFT).perform();
+        $(TextFieldElement.class).first().sendKeys("H");
+        new Actions(driver).keyUp(Keys.SHIFT).perform();
+    }
+
+    private void ensureWindowClosed() {
+        assertTrue("Window didn't close as expected.", $(WindowElement.class)
+                .all().isEmpty());
+    }
+
+    private void ensureWindowOpen() {
+        assertFalse("Window closed when it shouldn't have.",
+                $(WindowElement.class).all().isEmpty());
+    }
+}