JDK-8231692 : Test Infrastructure: enhance KeyEventFirer to inject keyEvents into scene
  • Type: Enhancement
  • Component: javafx
  • Sub-Component: controls
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2019-10-01
  • Updated: 2022-01-24
  • Resolved: 2019-11-12
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 8 Other
8u331Fixed openjfx14Fixed
Related Reports
Blocks :  
Relates :  
Relates :  
Relates :  
Description
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();
    }

}


Comments
Changeset: 94bcf3f Author: Jeanette Winzenburg Committer: Ajit Ghaisas Date: 2019-11-12 04:21:17 +0000 URL: https://git.openjdk.java.net/jfx/commit/94bcf3fc 8231692: Test Infrastructure: enhance KeyEventFirer to inject keyEvents into scene Reviewed-by: aghaisas
12-11-2019

Added option to create a KeyEventFirer that injects keyEvents into the scene. - added constructor which takes both event target and its scene - changed implementation of fireEvents to either fire directly on the target (if scene == null) or let the scene's processKeyEvent handle them (if scene != null) - added tests to verify that the false greens when using the first option are failing as expected by the second option Note: checking whether there are old tests that produce false greens is off scope for this issue, would need a new issue.
23-10-2019