On Sat, 21 Mar 2026 23:25:17 GMT, Marius Hanl <[email protected]> wrote:
>> Very similar to https://github.com/openjdk/jfx/pull/1829, this PR removes >> the questionable behavior that the `MouseEventFirer` may create a temporary >> `Stage` for your `Node`. >> >> Take the following test code: >> >> Button button = new Button("Button"); >> MouseEventFirer mouse = new MouseEventFirer(button); >> mouse.fireMousePressAndRelease(); >> mouse.fireMousePressAndRelease(); >> mouse.dispose() >> >> What it does is to create a `Stage` in the first method. The second method >> does not. This is not immediately clear. >> That is also the reason why the dispose method exists. To MAY clean it up. >> Or forget to call it. >> >> This does not test a realistic scenario and the chance is quite high that >> developers used that methods without even knowing that it contains such >> logic. >> >> So the idea is to remove the StageLoader code from MouseEventFirer and >> rather use it in the Test code before calling the Util methods. >> >> For the example above, this would result in: >> >> Button button = new Button("Button"); >> stageLoader = new StageLoader(button ); >> MouseEventFirer mouse = new MouseEventFirer(button); >> mouse.fireMousePressAndRelease(); >> mouse.fireMousePressAndRelease(); >> >> >> There were only two real tests that did not have a `Stage` yet. So most of >> the tests already had a good setup and this could never run. > > modules/javafx.controls/src/test/java/test/javafx/scene/control/cell/TextFieldTableCellTest.java > line 398: > >> 396: >> 397: TextField textField = (TextField) cell.getGraphic(); >> 398: MouseEventFirer mouse = new MouseEventFirer(textField); > > This is what happened before. > We created the following structure: > > Group > └ TextField > > And then testing the `TextField` in isolation without the table or cell > (since both are not inside the scene graph). > > Guess what, I also reverted the change, this testcase was supposed to test, > and still green. It does not make any sense. So removed it. (And now, we > avoid that this kind of test can be written, as you need to take care of the > scene graph by e.g. just using the `stageLoader`). I am a bit confused - you reverted the fix for https://bugs.openjdk.org/browse/JDK-8119995 ? I am a bit reluctant to remove the test, can we keep it? ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2124#discussion_r2977504345
