From d2d44b4a3ebb0b1c2698c1bb57f3b27ef7ad793b Mon Sep 17 00:00:00 2001 From: Jeanette Winzenburg Date: Tue, 17 Dec 2019 11:44:42 +0000 Subject: [PATCH] 8207759: VK_ENTER not consumed by TextField when default Button exists Reviewed-by: aghaisas, kcr --- .../control/behavior/TextFieldBehavior.java | 25 +- .../behavior/TextInputControlBehavior.java | 18 +- .../control/DefaultCancelButtonTestBase.java | 341 ++++++++++++++++++ .../PasswordFieldDefaultCancelButtonTest.java | 46 +++ .../TextAreaDefaultCancelButtonTest.java | 82 +++++ .../TextFieldDefaultCancelButtonTest.java | 46 +++ .../javafx/scene/control/TextFieldTest.java | 91 +++++ ...dWithFormatterDefaultCancelButtonTest.java | 57 +++ 8 files changed, 681 insertions(+), 25 deletions(-) create mode 100644 modules/javafx.controls/src/test/java/test/javafx/scene/control/DefaultCancelButtonTestBase.java create mode 100644 modules/javafx.controls/src/test/java/test/javafx/scene/control/PasswordFieldDefaultCancelButtonTest.java create mode 100644 modules/javafx.controls/src/test/java/test/javafx/scene/control/TextAreaDefaultCancelButtonTest.java create mode 100644 modules/javafx.controls/src/test/java/test/javafx/scene/control/TextFieldDefaultCancelButtonTest.java create mode 100644 modules/javafx.controls/src/test/java/test/javafx/scene/control/TextFieldWithFormatterDefaultCancelButtonTest.java diff --git a/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextFieldBehavior.java b/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextFieldBehavior.java index 40d67148d9f..df9734a051b 100644 --- a/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextFieldBehavior.java +++ b/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextFieldBehavior.java @@ -25,7 +25,16 @@ package com.sun.javafx.scene.control.behavior; + +import com.sun.javafx.PlatformUtil; +import com.sun.javafx.geom.transform.Affine3D; +import com.sun.javafx.scene.NodeHelper; import com.sun.javafx.scene.control.Properties; +import com.sun.javafx.scene.control.skin.Utils; +import com.sun.javafx.stage.WindowHelper; + +import static com.sun.javafx.PlatformUtil.*; + import javafx.beans.value.ChangeListener; import javafx.beans.value.WeakChangeListener; import javafx.event.ActionEvent; @@ -35,23 +44,14 @@ import javafx.geometry.Rectangle2D; import javafx.scene.Node; import javafx.scene.Scene; -import javafx.scene.control.ContextMenu; import javafx.scene.control.TextField; import javafx.scene.control.skin.TextFieldSkin; -import com.sun.javafx.scene.control.skin.Utils; import javafx.scene.input.ContextMenuEvent; import javafx.scene.input.KeyEvent; import javafx.scene.input.MouseEvent; import javafx.scene.text.HitInfo; import javafx.stage.Screen; import javafx.stage.Window; -import com.sun.javafx.PlatformUtil; -import com.sun.javafx.geom.transform.Affine3D; - -import static com.sun.javafx.PlatformUtil.isMac; -import static com.sun.javafx.PlatformUtil.isWindows; -import com.sun.javafx.scene.NodeHelper; -import com.sun.javafx.stage.WindowHelper; /** * Text field behavior. @@ -183,9 +183,10 @@ public void setTextFieldSkin(TextFieldSkin skin) { textField.commitValue(); textField.fireEvent(actionEvent); - - if (onAction == null && !actionEvent.isConsumed()) { - forwardToParent(event); + // fix of JDK-8207759: reverted logic + // mapping not auto-consume and consume if handled by action + if (onAction != null || actionEvent.isConsumed()) { + event.consume(); } } diff --git a/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextInputControlBehavior.java b/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextInputControlBehavior.java index 9ec9f1924d8..c20c7b30212 100644 --- a/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextInputControlBehavior.java +++ b/modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TextInputControlBehavior.java @@ -124,6 +124,7 @@ public TextInputControlBehavior(T c) { final Predicate validOnLinux = e -> !PlatformUtil.isLinux(); KeyMapping cancelEditMapping; + KeyMapping fireMapping; KeyMapping consumeMostPressedEventsMapping; // create a child input map for mappings which are applicable on all @@ -136,7 +137,7 @@ public TextInputControlBehavior(T c) { keyMapping(HOME, e -> c.home()), keyMapping(DOWN, e -> c.end()), keyMapping(END, e -> c.end()), - keyMapping(ENTER, this::fire), + fireMapping = keyMapping(ENTER, this::fire), keyMapping(new KeyBinding(HOME).shortcut(), e -> c.home()), keyMapping(new KeyBinding(END).shortcut(), e -> c.end()), @@ -213,6 +214,8 @@ public TextInputControlBehavior(T c) { ); cancelEditMapping.setAutoConsume(false); + // fix of JDK-8207759: don't auto-consume + fireMapping.setAutoConsume(false); consumeMostPressedEventsMapping.setAutoConsume(false); // mac os specific mappings @@ -620,18 +623,7 @@ private void rightWord() { } protected void fire(KeyEvent event) { } // TODO move to TextFieldBehavior - protected void cancelEdit(KeyEvent event) { forwardToParent(event);} // not autoconsumed - - protected void forwardToParent(KeyEvent event) { - // fix for JDK-8145515 - if (getNode().getProperties().containsKey(DISABLE_FORWARD_TO_PARENT)) { - return; - } - - if (getNode().getParent() != null) { - getNode().getParent().fireEvent(event); - } - } + protected void cancelEdit(KeyEvent event) { }; protected void selectHome() { getNode().selectHome(); diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/DefaultCancelButtonTestBase.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/DefaultCancelButtonTestBase.java new file mode 100644 index 00000000000..c8dd04b1b27 --- /dev/null +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/DefaultCancelButtonTestBase.java @@ -0,0 +1,341 @@ +/* + * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package test.javafx.scene.control; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.function.Consumer; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import com.sun.javafx.tk.Toolkit; + +import static javafx.scene.input.KeyCode.*; +import static javafx.scene.input.KeyEvent.*; +import static org.junit.Assert.*; + +import javafx.event.ActionEvent; +import javafx.event.EventHandler; +import javafx.scene.Scene; +import javafx.scene.control.Button; +import javafx.scene.control.Control; +import javafx.scene.input.KeyCode; +import javafx.scene.input.KeyEvent; +import javafx.scene.layout.VBox; +import javafx.stage.Stage; +import test.com.sun.javafx.pgstub.StubToolkit; +import test.com.sun.javafx.scene.control.infrastructure.KeyEventFirer; + +/** + * Abstract base test for interplay of ENTER/ESCAPE handlers on Controls with + * default/cancel button actions. + *

+ * Need to test all combinations of + *

+ * + * The test parameterized on all combination of the first 3 bullets, handling + * the last by 4 test methods. + */ +@RunWith(Parameterized.class) +public abstract class DefaultCancelButtonTestBase { + /** + * State of default/cancel button. + */ + public static enum ButtonState { + + DEFAULT(ENTER), + CANCEL(ESCAPE); + + KeyCode key; + + ButtonState(KeyCode key) { + this.key = key; + } + + /** + * KeyCode that external handlers/button type is interested in. + * @return + */ + public KeyCode getCode() { + return key; + } + + /** + * Creates and returns a handler that consumes the event for + * keyCode. + * + * @return handler that consumes if the keyCode of the + * event is the same as this type's keyCode. + */ + public EventHandler getConsumingHandler() { + return e -> { + if (getCode() == e.getCode()) e.consume(); + }; + } + + /** + * Configures the given button as either default or + * cancel, based on keyCode. + * + * @param button to configure. + */ + public void configureButton(Button button) { + if (getCode() == ENTER) { + button.setDefaultButton(true); + } else if (getCode() == ESCAPE) { + button.setCancelButton(true); + } + + } + } + + public static class ButtonType { + Button button; + ButtonState type; + + public ButtonType(ButtonState type) { + this.type = type; + button = new Button(); + type.configureButton(button); + } + + public Button getButton() { + return button; + } + + public KeyCode getCode() { + return type.getCode(); + } + + /** + * Returns a competing handler (for our keyCode) that not/consumes + * a received keyEvent depending on the given consuming flag. The + * handler can be registered on another control in the same scene. + * + * @param consuming + * @return + */ + public EventHandler getKeyHandler(boolean consuming) { + return consuming ? type.getConsumingHandler() : e -> {}; + } + + @Override + public String toString() { + return "" + type; + } + + + } + + private Stage stage; + private VBox root; + private C control; + private Button fallback; + private Scene scene; + + private ButtonType buttonType; + private boolean consume; + private boolean registerAfterShowing; + + // TODO name doesn't compile with gradle :controls:test + // because the junit version is 4.8.2 - name was introduced in 4.11 + // commenting for now until upgrade to newer junit + @Parameterized.Parameters //( name = "{index}: Button {0}, consuming {1}, registerAfterShowing {2} " ) + public static Collection data() { + Object[][] data = new Object[][] { + // buttonType, consuming, registerAfterShowing + {new ButtonType(ButtonState.DEFAULT), true, true}, + {new ButtonType(ButtonState.DEFAULT), true, false}, + {new ButtonType(ButtonState.DEFAULT), false, true}, + {new ButtonType(ButtonState.DEFAULT), false, false}, + {new ButtonType(ButtonState.CANCEL), true, true}, + {new ButtonType(ButtonState.CANCEL), true, false}, + {new ButtonType(ButtonState.CANCEL), false, true}, + {new ButtonType(ButtonState.CANCEL), false, false}, + }; + return Arrays.asList(data); + } + + public DefaultCancelButtonTestBase(ButtonType buttonType, boolean consume, + boolean registerAfterShowing) { + this.buttonType = buttonType; + this.consume = consume; + this.registerAfterShowing = registerAfterShowing; + } + + + @Test + public void testFallbackFilter() { + registerHandlerAndAssertFallbackNotification(this::addEventFilter); + } + + @Test + public void testFallbackHandler() { + registerHandlerAndAssertFallbackNotification(this::addEventHandler); + + } + + @Test + public void testFallbackSingletonHandler() { + registerHandlerAndAssertFallbackNotification(this::setOnKeyPressed); + + } + + @Test + public void testFallbackNoHandler() { + if (consume) return; + show(); + assertTargetNotification(buttonType.getCode(), buttonType.getButton(), 1); + } + + protected void registerHandlerAndAssertFallbackNotification(Consumer> consumer) { + if (registerAfterShowing) { + show(); + } + consumer.accept(buttonType.getKeyHandler(consume)); + if (!registerAfterShowing) { + show(); + } + + int expected = consume ? 0 : 1; + assertTargetNotification(buttonType.getCode(), buttonType.getButton(), expected); + + } + /** + * Registers the given handler on the textfield by adding as handler for keyPressed. + * @param handler the handler to register + */ + protected void addEventHandler(EventHandler handler) { + control.addEventHandler(KEY_PRESSED, handler); + } + + /** + * Registers the given handler on the textfield by setting as singleton + * keyPressed handler. + * @param handler the handler to register + */ + protected void setOnKeyPressed(EventHandler handler) { + control.setOnKeyPressed(handler); + } + + /** + * Registers the given handler on the textfield by adding as filter for keyPressed. + * @param handler the handler to register + */ + protected void addEventFilter(EventHandler filter) { + control.addEventFilter(KEY_PRESSED, filter); + } + +// ------------ assert helpers + /** + * Fires the key onto the control and asserts that + * the target button receives the expected number of notifications in + * its action handler. + * + * @param key the key to fire on the control + * @param target the target button to test for nori + * @param expected number of notifications in target button's action handler + */ + protected void assertTargetNotification(KeyCode key, Button target, int expected) { + List actions = new ArrayList<>(); + target.setOnAction(actions::add); + KeyEventFirer keyFirer = new KeyEventFirer(control); + keyFirer.doKeyPress(key); + String exp = expected > 0 ? " must " : " must not "; + assertEquals(key + exp + " trigger ", expected, actions.size()); + } + + + /** + * sanity test of initial state and test assumptions + */ + @Test + public void testInitial() { + show(); + assertTrue(control.isFocused()); + assertSame(root, control.getParent()); + assertSame(root, fallback.getParent()); + } + + + protected boolean isEnter() { + return buttonType.getCode() == ENTER; + } + + protected abstract C createControl(); + protected C getControl() { + return control; + }; + + protected void show() { + stage.show(); + // PENDING JW: a bit weird - sometimes need to focus the stage before + // the node, sometimes not + stage.requestFocus(); + control.requestFocus(); + } + + private void initStage() { + //This step is not needed (Just to make sure StubToolkit is loaded into VM) + @SuppressWarnings("unused") + Toolkit tk = (StubToolkit)Toolkit.getToolkit(); + root = new VBox(); + scene = new Scene(root); + stage = new Stage(); + stage.setScene(scene); + } + + @Before + public void setup() { + initStage(); + control = createControl(); + + fallback = buttonType.getButton(); + root.getChildren().addAll(control, fallback); + + } + + @After + public void cleanup() { + if (stage != null) { + stage.hide(); + } + } + +} diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/PasswordFieldDefaultCancelButtonTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/PasswordFieldDefaultCancelButtonTest.java new file mode 100644 index 00000000000..339cb57aebc --- /dev/null +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/PasswordFieldDefaultCancelButtonTest.java @@ -0,0 +1,46 @@ +/* + * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package test.javafx.scene.control; + +import javafx.scene.control.PasswordField; + +/** + * Test for interplay of ENTER/ESCAPE handlers on PasswordField with + * default/cancel button actions. + */ +public class PasswordFieldDefaultCancelButtonTest extends DefaultCancelButtonTestBase { + + public PasswordFieldDefaultCancelButtonTest(ButtonType buttonType, + boolean consume, boolean registerAfterShowing) { + super(buttonType, consume, registerAfterShowing); + } + + @Override + protected PasswordField createControl() { + return new PasswordField(); + } + +} diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TextAreaDefaultCancelButtonTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TextAreaDefaultCancelButtonTest.java new file mode 100644 index 00000000000..1b2f49d2712 --- /dev/null +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TextAreaDefaultCancelButtonTest.java @@ -0,0 +1,82 @@ +/* + * Copyright (c) 2019, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. Oracle designates this + * particular file as subject to the "Classpath" exception as provided + * by Oracle in the LICENSE file that accompanied this code. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package test.javafx.scene.control; + +import javafx.scene.control.TextArea; + +/** + * Test for interplay of ENTER/ESCAPE handlers on TextArea with + * default/cancel button actions. + */ +public class TextAreaDefaultCancelButtonTest extends DefaultCancelButtonTestBase