From c7ae45cc2404c2eab557866f310a2bb2130ddffe Mon Sep 17 00:00:00 2001 From: Artur Signell Date: Tue, 15 Oct 2013 09:33:43 +0300 Subject: [PATCH] Validate that the connector is enabled before triggering actions for it (#12743) Automated test enabled only for IE9-IE11 because of #12785 Change-Id: I265e5d1ead3fa56469861c5a98dcc9d0106d1051 --- .../vaadin/event/ConnectorActionManager.java | 88 +++++++++++++++++++ .../communication/ServerRpcHandler.java | 33 +++++-- .../src/com/vaadin/ui/AbstractComponent.java | 5 +- server/src/com/vaadin/ui/Button.java | 2 +- .../actions/ActionsOnInvisibleComponents.java | 76 ++++++++++++++++ .../ActionsOnInvisibleComponentsTest.java | 46 ++++++++++ 6 files changed, 238 insertions(+), 12 deletions(-) create mode 100644 server/src/com/vaadin/event/ConnectorActionManager.java create mode 100644 uitest/src/com/vaadin/tests/actions/ActionsOnInvisibleComponents.java create mode 100644 uitest/src/com/vaadin/tests/actions/ActionsOnInvisibleComponentsTest.java diff --git a/server/src/com/vaadin/event/ConnectorActionManager.java b/server/src/com/vaadin/event/ConnectorActionManager.java new file mode 100644 index 0000000000..297f78f179 --- /dev/null +++ b/server/src/com/vaadin/event/ConnectorActionManager.java @@ -0,0 +1,88 @@ +/* + * Copyright 2000-2013 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.event; + +import java.util.logging.Logger; + +import com.vaadin.event.Action.Container; +import com.vaadin.server.ClientConnector; +import com.vaadin.server.VariableOwner; +import com.vaadin.server.communication.ServerRpcHandler; +import com.vaadin.ui.Component; + +/** + * An ActionManager connected to a connector. Takes care of verifying that the + * connector can receive events before triggering an action. + *

+ * This is mostly a workaround until shortcut actions are re-implemented in a + * more sensible way. + * + * @since 7.1.8 + * @author Vaadin Ltd + */ +public class ConnectorActionManager extends ActionManager { + + private ClientConnector connector; + + /** + * Initialize an action manager for the given connector. + * + * @param connector + * the owner of this action manager + */ + public ConnectorActionManager(ClientConnector connector) { + super(); + this.connector = connector; + } + + /** + * Initialize an action manager for the given connector using the given + * viewer. + * + * @param connector + * the owner of this action manager + * @param viewer + * the viewer connected + */ + public ConnectorActionManager( + ClientConnector connector, T viewer) { + super(viewer); + this.connector = connector; + } + + /* + * (non-Javadoc) + * + * @see com.vaadin.event.ActionManager#handleAction(com.vaadin.event.Action, + * java.lang.Object, java.lang.Object) + */ + @Override + public void handleAction(Action action, Object sender, Object target) { + if (!connector.isConnectorEnabled()) { + getLogger().warning( + ServerRpcHandler.getIgnoredDisabledError("action", + connector)); + return; + } + + super.handleAction(action, sender, target); + } + + private static final Logger getLogger() { + return Logger.getLogger(ConnectorActionManager.class.getName()); + } + +} diff --git a/server/src/com/vaadin/server/communication/ServerRpcHandler.java b/server/src/com/vaadin/server/communication/ServerRpcHandler.java index 3cc85909ee..f14d703454 100644 --- a/server/src/com/vaadin/server/communication/ServerRpcHandler.java +++ b/server/src/com/vaadin/server/communication/ServerRpcHandler.java @@ -182,15 +182,8 @@ public class ServerRpcHandler implements Serializable { } // Connector is disabled, log a warning and move to the next - String msg = "Ignoring RPC call for disabled connector " - + connector.getClass().getName(); - if (connector instanceof Component) { - String caption = ((Component) connector).getCaption(); - if (caption != null) { - msg += ", caption=" + caption; - } - } - getLogger().warning(msg); + getLogger().warning( + getIgnoredDisabledError("RPC call", connector)); continue; } // DragAndDropService has null UI @@ -466,4 +459,26 @@ public class ServerRpcHandler implements Serializable { private static final Logger getLogger() { return Logger.getLogger(ServerRpcHandler.class.getName()); } + + /** + * Generates an error message when the client is trying to to something + * ('what') with a connector which is disabled or invisible. + * + * @since 7.1.8 + * @param connector + * the connector which is disabled (or invisible) + * @return an error message + */ + public static String getIgnoredDisabledError(String what, + ClientConnector connector) { + String msg = "Ignoring " + what + " for disabled connector " + + connector.getClass().getName(); + if (connector instanceof Component) { + String caption = ((Component) connector).getCaption(); + if (caption != null) { + msg += ", caption=" + caption; + } + } + return msg; + } } diff --git a/server/src/com/vaadin/ui/AbstractComponent.java b/server/src/com/vaadin/ui/AbstractComponent.java index 262d47af18..61bcf00ad8 100644 --- a/server/src/com/vaadin/ui/AbstractComponent.java +++ b/server/src/com/vaadin/ui/AbstractComponent.java @@ -27,6 +27,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import com.vaadin.event.ActionManager; +import com.vaadin.event.ConnectorActionManager; import com.vaadin.event.ShortcutListener; import com.vaadin.server.AbstractClientConnector; import com.vaadin.server.ComponentSizeValidator; @@ -90,7 +91,7 @@ public abstract class AbstractComponent extends AbstractClientConnector * Keeps track of the Actions added to this component; the actual * handling/notifying is delegated, usually to the containing window. */ - private ActionManager actionManager; + private ConnectorActionManager actionManager; private boolean visible = true; @@ -929,7 +930,7 @@ public abstract class AbstractComponent extends AbstractClientConnector */ protected ActionManager getActionManager() { if (actionManager == null) { - actionManager = new ActionManager(); + actionManager = new ConnectorActionManager(this); setActionManagerViewer(); } return actionManager; diff --git a/server/src/com/vaadin/ui/Button.java b/server/src/com/vaadin/ui/Button.java index 1bcf802f12..589f30d631 100644 --- a/server/src/com/vaadin/ui/Button.java +++ b/server/src/com/vaadin/ui/Button.java @@ -340,7 +340,7 @@ public class Button extends AbstractComponent implements * No action is taken is the button is disabled. */ public void click() { - if (isEnabled() && !isReadOnly()) { + if (isConnectorEnabled() && !isReadOnly()) { fireClick(); } } diff --git a/uitest/src/com/vaadin/tests/actions/ActionsOnInvisibleComponents.java b/uitest/src/com/vaadin/tests/actions/ActionsOnInvisibleComponents.java new file mode 100644 index 0000000000..0f4c179cee --- /dev/null +++ b/uitest/src/com/vaadin/tests/actions/ActionsOnInvisibleComponents.java @@ -0,0 +1,76 @@ +package com.vaadin.tests.actions; + +import com.vaadin.event.ShortcutAction.KeyCode; +import com.vaadin.server.VaadinRequest; +import com.vaadin.tests.components.AbstractTestUIWithLog; +import com.vaadin.ui.Button; +import com.vaadin.ui.Button.ClickEvent; + +public class ActionsOnInvisibleComponents extends AbstractTestUIWithLog { + + private static final long serialVersionUID = -5993467736906948993L; + + @Override + protected void setup(VaadinRequest request) { + getContent().setId("test-root"); + log("'A' triggers a click on an invisible button"); + log("'B' triggers a click on a disabled button"); + log("'C' triggers a click on a visible and enabled button"); + + Button invisibleButton = new Button("Invisible button with shortcut"); + invisibleButton.setClickShortcut(KeyCode.A); + invisibleButton.addClickListener(new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + log("Click event for invisible button"); + } + }); + + invisibleButton.setVisible(false); + addComponent(invisibleButton); + + Button disabledButton = new Button("Disabled button with shortcut"); + disabledButton.setClickShortcut(KeyCode.B); + disabledButton.addClickListener(new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + log("Click event for disabled button"); + } + }); + + disabledButton.setEnabled(false); + addComponent(disabledButton); + + Button enabledButton = new Button("Enabled button with shortcut"); + enabledButton.setClickShortcut(KeyCode.C); + enabledButton.addClickListener(new Button.ClickListener() { + @Override + public void buttonClick(ClickEvent event) { + log("Click event for enabled button"); + } + }); + + addComponent(enabledButton); + } + + /* + * (non-Javadoc) + * + * @see com.vaadin.tests.components.AbstractTestUI#getTestDescription() + */ + @Override + protected String getTestDescription() { + return "Test to ensure actions are not performed on disabled/invisible components"; + } + + /* + * (non-Javadoc) + * + * @see com.vaadin.tests.components.AbstractTestUI#getTicketNumber() + */ + @Override + protected Integer getTicketNumber() { + return 12743; + } + +} diff --git a/uitest/src/com/vaadin/tests/actions/ActionsOnInvisibleComponentsTest.java b/uitest/src/com/vaadin/tests/actions/ActionsOnInvisibleComponentsTest.java new file mode 100644 index 0000000000..bb2a7d0995 --- /dev/null +++ b/uitest/src/com/vaadin/tests/actions/ActionsOnInvisibleComponentsTest.java @@ -0,0 +1,46 @@ +package com.vaadin.tests.actions; + +import java.util.Collection; + +import org.junit.Assert; +import org.junit.Test; +import org.openqa.selenium.WebElement; +import org.openqa.selenium.remote.DesiredCapabilities; + +import com.vaadin.tests.tb3.MultiBrowserTest; + +public class ActionsOnInvisibleComponentsTest extends MultiBrowserTest { + + private static final String LAST_INIT_LOG = "3. 'C' triggers a click on a visible and enabled button"; + + // This method should be removed once #12785 is fixed + @Override + public Collection getBrowsersToTest() { + Collection browsers = super.getBrowsersToTest(); + // sendKeys does nothing on these browsers + browsers.remove(BrowserUtil.firefox(24)); + browsers.remove(BrowserUtil.ie(8)); + browsers.remove(BrowserUtil.opera(12)); + + // Causes 'cannot focus element' + browsers.remove(BrowserUtil.chrome(29)); + return browsers; + } + + @Test + public void testShortcutsOnInvisibleDisabledButtons() { + openTestURL(); + Assert.assertEquals(LAST_INIT_LOG, getLogRow(0)); + invokeShortcut("A"); + Assert.assertEquals(LAST_INIT_LOG, getLogRow(0)); + invokeShortcut("B"); + Assert.assertEquals(LAST_INIT_LOG, getLogRow(0)); + invokeShortcut("C"); + Assert.assertEquals("4. Click event for enabled button", getLogRow(0)); + } + + private void invokeShortcut(CharSequence key) { + WebElement shortcutTarget = vaadinElementById("test-root"); + shortcutTarget.sendKeys(key); + } +} -- 2.39.5