On Wed, 11 Mar 2026 01:00:33 GMT, Ziad El Midaoui <[email protected]>
wrote:
>> The bug occurs when `showRoot` is set to false on a root with no children,
>> the expanded item count drops is 0 causing `isFocused(0)` to return false
>> even though `focusedIndex` is still 0. When items are added afterwards the
>> `treeItemListener` sees `focusedIndex=0` and incorrectly shifts it to 1
>> placing the focus on second item of the TreeView.
>> The fix replaces `isFocused(0)` with `getFocusedIndex() >= 0`, which reads
>> the raw stored index.
>
> Ziad El Midaoui has updated the pull request incrementally with one
> additional commit since the last revision:
>
> minor changes
Providing a few comments on the test.
The test consistently FAILS on my macOs 26.2 machine, with fix.
TreeViewInitialFocusTest >
testInitialFocusDoesNotMoveToSecondVisibleItemWhenRootHidden() FAILED
org.opentest4j.AssertionFailedError: Focused index must be cleared ==>
expected: <-1> but was: <0>
at
app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
at
app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
at
app//org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
at
app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:150)
at
app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:563)
at
app//test.robot.javafx.scene.treeview.TreeViewInitialFocusTest.testInitialFocusDoesNotMoveToSecondVisibleItemWhenRootHidden(TreeViewInitialFocusTest.java:121)
tests/system/src/test/java/test/robot/javafx/scene/treeview/TreeViewInitialFocusTest.java
line 70:
> 68: static Robot robot;
> 69: static volatile Stage stage;
> 70: static volatile Scene scene;
The variables `stage` and `scene` are not used outside the `TestApp.start()`
method, so we don't need them as class members.
tests/system/src/test/java/test/robot/javafx/scene/treeview/TreeViewInitialFocusTest.java
line 85:
> 83: @Test
> 84: public void
> testInitialFocusDoesNotMoveToSecondVisibleItemWhenRootHidden() {
> 85: Util.waitForLatch(startupLatch, 10, "Timeout waiting for test
> application to start");
The `Util.launch()` method does wait on the provided CountDownLatch, so this
`Util.waitForLatch()` call is not needed/no op here.
tests/system/src/test/java/test/robot/javafx/scene/treeview/TreeViewInitialFocusTest.java
line 94:
> 92: robot.mouseMove((int) x, (int) y);
> 93: robot.mousePress(MouseButton.PRIMARY);
> 94: robot.mouseRelease(MouseButton.PRIMARY);
Can use `robot.mouseClick(MouseButton.PRIMARY);`
tests/system/src/test/java/test/robot/javafx/scene/treeview/TreeViewInitialFocusTest.java
line 148:
> 146: stage.setAlwaysOnTop(true);
> 147: stage.addEventHandler(WindowEvent.WINDOW_SHOWN,
> 148: e -> Platform.runLater(startupLatch::countDown));
minor/optional: Would recommend to use `stage.setOnShown(event ->
Platform.runLater(startupLatch::countDown));`
-------------
Changes requested by arapte (Reviewer).
PR Review: https://git.openjdk.org/jfx/pull/2095#pullrequestreview-3927264592
PR Review Comment: https://git.openjdk.org/jfx/pull/2095#discussion_r2916452141
PR Review Comment: https://git.openjdk.org/jfx/pull/2095#discussion_r2916375734
PR Review Comment: https://git.openjdk.org/jfx/pull/2095#discussion_r2916237934
PR Review Comment: https://git.openjdk.org/jfx/pull/2095#discussion_r2916466715
