-
Enhancement
-
Resolution: Fixed
-
P4
-
None
Issue | Fix Version | Assignee | Priority | Status | Resolution | Resolved In Build |
---|---|---|---|---|---|---|
JDK-8280523 | 8u331 | Kevin Rushforth | P4 | Resolved | Fixed | b04 |
copied from https://github.com/javafxports/openjdk-jfx/issues/585
Currently I'm digging into issues around TextField/event dispatch and stumbled across a regression JDK-8229914 Wrote a test (for convenience, copied below from https://github.com/kleopatra/openjdk-jfx/tree/regression-false-green-JDK-8229914 ) .. which passed, astonishingly (for me ;).
At the end of the day, the false green was produced by firing the keyEvent onto a node that is never the focusOwner (the editor of the comboBox). Firing on its parent, on the other hand, produces the expected failure.
Actually, I think that it is (nearly) always wrong to fire a KeyEvent directly onto the target: out in the wild, keyEvents are passed from the scene onto the focusOwner. By-passing that process makes the tests brittle, whatever the outcome, I can't be entirely certain whether it's a false green/red.
Therefore I suggest to enhance the KeyEventFirer to conditionally support injecting the event into scene and always use that version. To not interfere with existing tests, it could be implemented to fire either directly (the old version, default) and inject with the new, something like:
/**
* Instantiates a KeyEventFirer that can be configured to fire the
* keyEvent directly onto the target or via injection into the scene,
* depending on useScene being false/true, respectively. If true,
* the target is focused before injection.
*
* @param target the target of the KeyEvent.
* @param useScene flag to control the firing behavior.
*/
public KeyEventFirer(EventTarget target, boolean useScene) {
this.target = target;
this.useScene = useScene;
}
private void fireEvents(KeyEvent... events) {
for (KeyEvent evt : events) {
Scene scene = null;
if (useScene && target instanceof Node) {
scene = ((Node) target).getScene();
((Node) target).requestFocus();
}
if (scene != null) {
scene.processKeyEvent(evt);
} else {
Event.fireEvent(target, evt);
}
}
}
The test:
package test.javafx.scene.control;
import java.util.ArrayList;
import java.util.List;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import com.sun.javafx.tk.Toolkit;
import static org.junit.Assert.*;
import static javafx.scene.input.KeyCode.*;
import static javafx.scene.input.KeyEvent.*;
import javafx.scene.Scene;
import javafx.scene.control.ComboBox;
import javafx.scene.control.skin.ComboBoxPopupControl;
import javafx.scene.input.KeyEvent;
import javafx.scene.layout.StackPane;
import javafx.stage.Stage;
import test.com.sun.javafx.pgstub.StubToolkit;
import test.com.sun.javafx.scene.control.infrastructure.KeyEventFirer;
/**
* Test to expose false green with KeyEventFirer.
* <p>
* The concrete issue is that a keyPressed filter on the editor is not notified.
* It's a regression https://bugs.openjdk.java.net/browse/JDK-8229914.
* It was fixed as https://bugs.openjdk.java.net/browse/JDK-8145515 (without tests).
* It was re-introduced with fixing https://bugs.openjdk.java.net/browse/JDK-8149622.
* <p>
* First try to test the failure failed: firing the event on on the editor passes, which
* is a false green (as can be reproduced by the example in the bug report).
* Reason for the false green is that the editor is-a FakeFocusTextField which never is
* the focusOwner. Firing the event on it produces a dispatch chain that contains the
* eventFilter, such that the filter is indeed notified. Firing on its parent combo exposes
* the issue.
*
*/
public class ComboBoxEnterTest8229914 {
private StackPane root;
private Stage stage;
private Scene scene;
private ComboBox<String> comboBox;
/**
* Here we fire on the editor with support in KeyEventFirer to
* use the scene as KeyEvent injector. With core version
* of ComboBoxPopupControl, the test fails
* as expected.
*/
@Test
public void testEnterPressedFilterEditorOnScene() {
List<KeyEvent> keys = new ArrayList<>();
comboBox.getEditor().addEventFilter(KEY_PRESSED, keys::add);
KeyEventFirer keyboard = new KeyEventFirer(comboBox.getEditor(), true);
keyboard.doKeyPress(ENTER);
assertEquals("pressed ENTER filter on editor", 1, keys.size());
}
@Test
public void testEnterPressedFilterEditorComboBox() {
List<KeyEvent> keys = new ArrayList<>();
comboBox.getEditor().addEventFilter(KEY_PRESSED, keys::add);
KeyEventFirer keyboard = new KeyEventFirer(comboBox);
keyboard.doKeyPress(ENTER);
assertEquals("pressed ENTER filter on editor", 1, keys.size());
}
/**
* Note: this is a false green!
*/
@Test
public void testEnterPressedFilterEditor() {
List<KeyEvent> keys = new ArrayList<>();
comboBox.getEditor().addEventFilter(KEY_PRESSED, keys::add);
KeyEventFirer keyboard = new KeyEventFirer(comboBox.getEditor());
keyboard.doKeyPress(ENTER);
assertEquals("pressed ENTER filter on editor", 1, keys.size());
}
@After
public void cleanup() {
stage.hide();
}
@Before
public void setup() {
ComboBoxPopupControl c;
// This step is not needed (Just to make sure StubToolkit is loaded into VM)
Toolkit tk = (StubToolkit) Toolkit.getToolkit();
root = new StackPane();
scene = new Scene(root);
stage = new Stage();
stage.setScene(scene);
comboBox = new ComboBox<>();
comboBox.getItems().addAll("Test", "hello", "world");
comboBox.setEditable(true);
root.getChildren().addAll(comboBox);
stage.show();
}
}
Currently I'm digging into issues around TextField/event dispatch and stumbled across a regression JDK-8229914 Wrote a test (for convenience, copied below from https://github.com/kleopatra/openjdk-jfx/tree/regression-false-green-JDK-8229914 ) .. which passed, astonishingly (for me ;).
At the end of the day, the false green was produced by firing the keyEvent onto a node that is never the focusOwner (the editor of the comboBox). Firing on its parent, on the other hand, produces the expected failure.
Actually, I think that it is (nearly) always wrong to fire a KeyEvent directly onto the target: out in the wild, keyEvents are passed from the scene onto the focusOwner. By-passing that process makes the tests brittle, whatever the outcome, I can't be entirely certain whether it's a false green/red.
Therefore I suggest to enhance the KeyEventFirer to conditionally support injecting the event into scene and always use that version. To not interfere with existing tests, it could be implemented to fire either directly (the old version, default) and inject with the new, something like:
/**
* Instantiates a KeyEventFirer that can be configured to fire the
* keyEvent directly onto the target or via injection into the scene,
* depending on useScene being false/true, respectively. If true,
* the target is focused before injection.
*
* @param target the target of the KeyEvent.
* @param useScene flag to control the firing behavior.
*/
public KeyEventFirer(EventTarget target, boolean useScene) {
this.target = target;
this.useScene = useScene;
}
private void fireEvents(KeyEvent... events) {
for (KeyEvent evt : events) {
Scene scene = null;
if (useScene && target instanceof Node) {
scene = ((Node) target).getScene();
((Node) target).requestFocus();
}
if (scene != null) {
scene.processKeyEvent(evt);
} else {
Event.fireEvent(target, evt);
}
}
}
The test:
package test.javafx.scene.control;
import java.util.ArrayList;
import java.util.List;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import com.sun.javafx.tk.Toolkit;
import static org.junit.Assert.*;
import static javafx.scene.input.KeyCode.*;
import static javafx.scene.input.KeyEvent.*;
import javafx.scene.Scene;
import javafx.scene.control.ComboBox;
import javafx.scene.control.skin.ComboBoxPopupControl;
import javafx.scene.input.KeyEvent;
import javafx.scene.layout.StackPane;
import javafx.stage.Stage;
import test.com.sun.javafx.pgstub.StubToolkit;
import test.com.sun.javafx.scene.control.infrastructure.KeyEventFirer;
/**
* Test to expose false green with KeyEventFirer.
* <p>
* The concrete issue is that a keyPressed filter on the editor is not notified.
* It's a regression https://bugs.openjdk.java.net/browse/JDK-8229914.
* It was fixed as https://bugs.openjdk.java.net/browse/JDK-8145515 (without tests).
* It was re-introduced with fixing https://bugs.openjdk.java.net/browse/JDK-8149622.
* <p>
* First try to test the failure failed: firing the event on on the editor passes, which
* is a false green (as can be reproduced by the example in the bug report).
* Reason for the false green is that the editor is-a FakeFocusTextField which never is
* the focusOwner. Firing the event on it produces a dispatch chain that contains the
* eventFilter, such that the filter is indeed notified. Firing on its parent combo exposes
* the issue.
*
*/
public class ComboBoxEnterTest8229914 {
private StackPane root;
private Stage stage;
private Scene scene;
private ComboBox<String> comboBox;
/**
* Here we fire on the editor with support in KeyEventFirer to
* use the scene as KeyEvent injector. With core version
* of ComboBoxPopupControl, the test fails
* as expected.
*/
@Test
public void testEnterPressedFilterEditorOnScene() {
List<KeyEvent> keys = new ArrayList<>();
comboBox.getEditor().addEventFilter(KEY_PRESSED, keys::add);
KeyEventFirer keyboard = new KeyEventFirer(comboBox.getEditor(), true);
keyboard.doKeyPress(ENTER);
assertEquals("pressed ENTER filter on editor", 1, keys.size());
}
@Test
public void testEnterPressedFilterEditorComboBox() {
List<KeyEvent> keys = new ArrayList<>();
comboBox.getEditor().addEventFilter(KEY_PRESSED, keys::add);
KeyEventFirer keyboard = new KeyEventFirer(comboBox);
keyboard.doKeyPress(ENTER);
assertEquals("pressed ENTER filter on editor", 1, keys.size());
}
/**
* Note: this is a false green!
*/
@Test
public void testEnterPressedFilterEditor() {
List<KeyEvent> keys = new ArrayList<>();
comboBox.getEditor().addEventFilter(KEY_PRESSED, keys::add);
KeyEventFirer keyboard = new KeyEventFirer(comboBox.getEditor());
keyboard.doKeyPress(ENTER);
assertEquals("pressed ENTER filter on editor", 1, keys.size());
}
@After
public void cleanup() {
stage.hide();
}
@Before
public void setup() {
ComboBoxPopupControl c;
// This step is not needed (Just to make sure StubToolkit is loaded into VM)
Toolkit tk = (StubToolkit) Toolkit.getToolkit();
root = new StackPane();
scene = new Scene(root);
stage = new Stage();
stage.setScene(scene);
comboBox = new ComboBox<>();
comboBox.getItems().addAll("Test", "hello", "world");
comboBox.setEditable(true);
root.getChildren().addAll(comboBox);
stage.show();
}
}
- backported by
-
JDK-8280523 Test Infrastructure: enhance KeyEventFirer to inject keyEvents into scene
- Resolved
- blocks
-
JDK-8244075 Accelerator of ContextMenu's MenuItem is not removed when ContextMenu is removed from Scene
- Resolved
- relates to
-
JDK-8229914 Regression: eventFilter on TextField in editable ComboBox doesn't receive ENTER
- Open
-
JDK-8145515 addEventFilter for combobox editor does not react on key action enter
- Resolved
-
JDK-8149622 ComboBox produces unexpected event behaviour when ENTER key is pressed
- Resolved
- links to
(3 links to)