Skip to content

Commit d3c26c4

Browse files
author
Johan Vos
committed
8286261: Selection of non-expanded non-leaf treeItem grows unexpectedly when adding two-level descendants
Backport-of: 19a855e
1 parent f84dac2 commit d3c26c4

File tree

5 files changed

+179
-8
lines changed

5 files changed

+179
-8
lines changed

‎modules/javafx.controls/src/main/java/javafx/scene/control/ControlUtils.java

+14
Original file line numberDiff line numberDiff line change
@@ -232,4 +232,18 @@ public static <S> int getIndexOfChildWithDescendant(TreeItem<S> parent, TreeItem
232232
}
233233
return -1;
234234
}
235+
236+
public static <S> boolean isTreeItemIncludingAncestorsExpanded(TreeItem<S> item) {
237+
if (item == null || !item.isExpanded()) {
238+
return false;
239+
}
240+
TreeItem<S> ancestor = item.getParent();
241+
while (ancestor != null) {
242+
if (!ancestor.isExpanded()) {
243+
return false;
244+
}
245+
ancestor = ancestor.getParent();
246+
}
247+
return true;
248+
}
235249
}

‎modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -2610,7 +2610,7 @@ private void updateTreeEventListener(TreeItem<S> oldRoot, TreeItem<S> newRoot) {
26102610
}
26112611
} else if (e.wasAdded()) {
26122612
// shuffle selection by the number of added items
2613-
shift += treeItem.isExpanded() ? addedSize : 0;
2613+
shift += ControlUtils.isTreeItemIncludingAncestorsExpanded(treeItem) ? addedSize : 0;
26142614

26152615
// RT-32963: We were taking the startRow from the TreeItem
26162616
// in which the children were added, rather than from the
@@ -2646,7 +2646,7 @@ private void updateTreeEventListener(TreeItem<S> oldRoot, TreeItem<S> newRoot) {
26462646

26472647
// shuffle selection by the number of removed items
26482648
// only if removed items are before the current selection.
2649-
if (treeItem.isExpanded()) {
2649+
if (ControlUtils.isTreeItemIncludingAncestorsExpanded(treeItem)) {
26502650
int lastSelectedSiblingIndex = selectedItems.stream()
26512651
.map(item -> ControlUtils.getIndexOfChildWithDescendant(treeItem, item))
26522652
.max(Comparator.naturalOrder())
@@ -3490,7 +3490,7 @@ private void updateTreeEventListener(TreeItem<S> oldRoot, TreeItem<S> newRoot) {
34903490
// get the TreeItem the event occurred on - we only need to
34913491
// shift if the tree item is expanded
34923492
TreeItem<S> eventTreeItem = e.getTreeItem();
3493-
if (eventTreeItem.isExpanded()) {
3493+
if (ControlUtils.isTreeItemIncludingAncestorsExpanded(eventTreeItem)) {
34943494
for (int i = 0; i < e.getAddedChildren().size(); i++) {
34953495
// get the added item and determine the row it is in
34963496
TreeItem<S> item = e.getAddedChildren().get(i);
@@ -3512,7 +3512,7 @@ private void updateTreeEventListener(TreeItem<S> oldRoot, TreeItem<S> newRoot) {
35123512
}
35133513
}
35143514

3515-
if (e.getTreeItem().isExpanded()) {
3515+
if (ControlUtils.isTreeItemIncludingAncestorsExpanded(e.getTreeItem())) {
35163516
int focusedSiblingRow = ControlUtils.getIndexOfChildWithDescendant(e.getTreeItem(), getFocusedItem());
35173517
if (e.getFrom() <= focusedSiblingRow) {
35183518
// shuffle selection by the number of removed items

‎modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java

+4-4
Original file line numberDiff line numberDiff line change
@@ -1398,7 +1398,7 @@ private void updateTreeEventListener(TreeItem<T> oldRoot, TreeItem<T> newRoot) {
13981398
// no-op
13991399
} else if (e.wasAdded()) {
14001400
// shuffle selection by the number of added items
1401-
shift += treeItem.isExpanded() ? addedSize : 0;
1401+
shift += ControlUtils.isTreeItemIncludingAncestorsExpanded(treeItem) ? addedSize : 0;
14021402

14031403
// RT-32963: We were taking the startRow from the TreeItem
14041404
// in which the children were added, rather than from the
@@ -1426,7 +1426,7 @@ private void updateTreeEventListener(TreeItem<T> oldRoot, TreeItem<T> newRoot) {
14261426

14271427
// shuffle selection by the number of removed items
14281428
// only if removed items are before the current selection.
1429-
if (treeItem.isExpanded()) {
1429+
if (ControlUtils.isTreeItemIncludingAncestorsExpanded(treeItem)) {
14301430
int lastSelectedSiblingIndex = selectedItems.stream()
14311431
.map(item -> ControlUtils.getIndexOfChildWithDescendant(treeItem, item))
14321432
.max(Comparator.naturalOrder())
@@ -1675,7 +1675,7 @@ private void updateTreeEventListener(TreeItem<T> oldRoot, TreeItem<T> newRoot) {
16751675
// get the TreeItem the event occurred on - we only need to
16761676
// shift if the tree item is expanded
16771677
TreeItem<T> eventTreeItem = e.getTreeItem();
1678-
if (eventTreeItem.isExpanded()) {
1678+
if (ControlUtils.isTreeItemIncludingAncestorsExpanded(eventTreeItem)) {
16791679
for (int i = 0; i < e.getAddedChildren().size(); i++) {
16801680
// get the added item and determine the row it is in
16811681
TreeItem<T> item = e.getAddedChildren().get(i);
@@ -1697,7 +1697,7 @@ private void updateTreeEventListener(TreeItem<T> oldRoot, TreeItem<T> newRoot) {
16971697
}
16981698
}
16991699

1700-
if (e.getTreeItem().isExpanded()) {
1700+
if (ControlUtils.isTreeItemIncludingAncestorsExpanded(e.getTreeItem())) {
17011701
int focusedSiblingRow = ControlUtils.getIndexOfChildWithDescendant(e.getTreeItem(), getFocusedItem());
17021702
if (e.getFrom() <= focusedSiblingRow) {
17031703
// shuffle selection by the number of removed items

‎modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableViewTest.java

+83
Original file line numberDiff line numberDiff line change
@@ -6832,4 +6832,87 @@ public void test_ChangeToStringMouseMultipleSelectionCellMode() {
68326832
Thread.currentThread().setUncaughtExceptionHandler(exceptionHandler);
68336833
}
68346834

6835+
// JDK-8286261
6836+
@Test
6837+
public void testAddTreeItemToCollapsedAncestorKeepsSelectedItem() {
6838+
TreeItem<String> rootNode = new TreeItem<>("Root");
6839+
rootNode.setExpanded(true);
6840+
TreeItem<String> level1 = new TreeItem<>("Node 0");
6841+
level1.setExpanded(false);
6842+
TreeItem<String> level2 = new TreeItem<>("Node 1");
6843+
level2.getChildren().add(new TreeItem<>("Node 2"));
6844+
level2.setExpanded(true);
6845+
6846+
rootNode.getChildren().add(level1);
6847+
rootNode.getChildren().add(new TreeItem<>("Node 3"));
6848+
6849+
level1.getChildren().add(level2);
6850+
6851+
TreeTableColumn<String, String> column = new TreeTableColumn<>("Nodes");
6852+
column.setCellValueFactory(p -> new ReadOnlyStringWrapper(p.getValue().getValue()));
6853+
column.setPrefWidth(200);
6854+
6855+
TreeTableView<String> table = new TreeTableView<>(rootNode);
6856+
table.setShowRoot(false);
6857+
table.getColumns().add(column);
6858+
table.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
6859+
6860+
table.getSelectionModel().select(level1);
6861+
assertEquals(0, table.getSelectionModel().getSelectedIndex());
6862+
assertEquals("Node 0", table.getSelectionModel().getSelectedItem().getValue());
6863+
assertEquals(0, table.getFocusModel().getFocusedIndex());
6864+
assertEquals("Node 0", table.getFocusModel().getFocusedItem().getValue());
6865+
6866+
// add new node at level 3, that has a collapsed ancestor
6867+
level2.getChildren().add(new TreeItem<>("Node 4"));
6868+
6869+
// selection and focus remain at level1
6870+
assertEquals(0, table.getSelectionModel().getSelectedIndex());
6871+
assertEquals("Node 0", table.getSelectionModel().getSelectedItem().getValue());
6872+
assertEquals(0, table.getFocusModel().getFocusedIndex());
6873+
assertEquals("Node 0", table.getFocusModel().getFocusedItem().getValue());
6874+
}
6875+
6876+
// JDK-8286261
6877+
@Test
6878+
public void testRemoveTreeItemFromCollapsedAncestorKeepsSelectedItem() {
6879+
TreeItem<String> rootNode = new TreeItem<>("Root");
6880+
rootNode.setExpanded(true);
6881+
TreeItem<String> level1 = new TreeItem<>("Node 0");
6882+
level1.setExpanded(false);
6883+
TreeItem<String> level2 = new TreeItem<>("Node 1");
6884+
level2.getChildren().add(new TreeItem<>("Node 2"));
6885+
level2.getChildren().add(new TreeItem<>("Node 3"));
6886+
6887+
level2.setExpanded(true);
6888+
6889+
rootNode.getChildren().add(level1);
6890+
rootNode.getChildren().add(new TreeItem<>("Node 4"));
6891+
6892+
level1.getChildren().add(level2);
6893+
6894+
TreeTableColumn<String, String> column = new TreeTableColumn<>("Nodes");
6895+
column.setCellValueFactory(p -> new ReadOnlyStringWrapper(p.getValue().getValue()));
6896+
column.setPrefWidth(200);
6897+
6898+
TreeTableView<String> table = new TreeTableView<>(rootNode);
6899+
table.setShowRoot(false);
6900+
table.getColumns().add(column);
6901+
table.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
6902+
6903+
table.getSelectionModel().select(level1);
6904+
assertEquals(0, table.getSelectionModel().getSelectedIndex());
6905+
assertEquals("Node 0", table.getSelectionModel().getSelectedItem().getValue());
6906+
assertEquals(0, table.getFocusModel().getFocusedIndex());
6907+
assertEquals("Node 0", table.getFocusModel().getFocusedItem().getValue());
6908+
6909+
// remove Node 2 at level 3, that has a collapsed ancestor
6910+
level2.getChildren().remove(0);
6911+
6912+
// selection and focus remain at level1
6913+
assertEquals(0, table.getSelectionModel().getSelectedIndex());
6914+
assertEquals("Node 0", table.getSelectionModel().getSelectedItem().getValue());
6915+
assertEquals(0, table.getFocusModel().getFocusedIndex());
6916+
assertEquals("Node 0", table.getFocusModel().getFocusedItem().getValue());
6917+
}
68356918
}

‎modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeViewTest.java

+74
Original file line numberDiff line numberDiff line change
@@ -3832,6 +3832,80 @@ public void testRemoveTreeItemChangesSelectedItem() {
38323832
assertEquals("Node 1", table.getSelectionModel().getSelectedItem().getValue());
38333833
}
38343834

3835+
// JDK-8286261
3836+
@Test
3837+
public void testAddTreeItemToCollapsedAncestorKeepsSelectedItem() {
3838+
TreeItem<String> rootNode = new TreeItem<>("Root");
3839+
rootNode.setExpanded(true);
3840+
TreeItem<String> level1 = new TreeItem<>("Node 0");
3841+
level1.setExpanded(false);
3842+
TreeItem<String> level2 = new TreeItem<>("Node 1");
3843+
level2.getChildren().add(new TreeItem<>("Node 2"));
3844+
level2.setExpanded(true);
3845+
3846+
rootNode.getChildren().add(level1);
3847+
rootNode.getChildren().add(new TreeItem<>("Node 3"));
3848+
3849+
level1.getChildren().add(level2);
3850+
3851+
TreeView<String> table = new TreeView<>(rootNode);
3852+
table.setShowRoot(false);
3853+
table.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
3854+
3855+
table.getSelectionModel().select(level1);
3856+
assertEquals(0, table.getSelectionModel().getSelectedIndex());
3857+
assertEquals("Node 0", table.getSelectionModel().getSelectedItem().getValue());
3858+
assertEquals(0, table.getFocusModel().getFocusedIndex());
3859+
assertEquals("Node 0", table.getFocusModel().getFocusedItem().getValue());
3860+
3861+
// add new node at level 3, that has a collapsed ancestor
3862+
level2.getChildren().add(new TreeItem<>("Node 4"));
3863+
3864+
// selection and focus remain at level1
3865+
assertEquals(0, table.getSelectionModel().getSelectedIndex());
3866+
assertEquals("Node 0", table.getSelectionModel().getSelectedItem().getValue());
3867+
assertEquals(0, table.getFocusModel().getFocusedIndex());
3868+
assertEquals("Node 0", table.getFocusModel().getFocusedItem().getValue());
3869+
}
3870+
3871+
// JDK-8286261
3872+
@Test
3873+
public void testRemoveTreeItemFromCollapsedAncestorKeepsSelectedItem() {
3874+
TreeItem<String> rootNode = new TreeItem<>("Root");
3875+
rootNode.setExpanded(true);
3876+
TreeItem<String> level1 = new TreeItem<>("Node 0");
3877+
level1.setExpanded(false);
3878+
TreeItem<String> level2 = new TreeItem<>("Node 1");
3879+
level2.getChildren().add(new TreeItem<>("Node 2"));
3880+
level2.getChildren().add(new TreeItem<>("Node 3"));
3881+
3882+
level2.setExpanded(true);
3883+
3884+
rootNode.getChildren().add(level1);
3885+
rootNode.getChildren().add(new TreeItem<>("Node 4"));
3886+
3887+
level1.getChildren().add(level2);
3888+
3889+
TreeView<String> table = new TreeView<>(rootNode);
3890+
table.setShowRoot(false);
3891+
table.getSelectionModel().setSelectionMode(SelectionMode.MULTIPLE);
3892+
3893+
table.getSelectionModel().select(level1);
3894+
assertEquals(0, table.getSelectionModel().getSelectedIndex());
3895+
assertEquals("Node 0", table.getSelectionModel().getSelectedItem().getValue());
3896+
assertEquals(0, table.getFocusModel().getFocusedIndex());
3897+
assertEquals("Node 0", table.getFocusModel().getFocusedItem().getValue());
3898+
3899+
// remove Node 2 at level 3, that has a collapsed ancestor
3900+
level2.getChildren().remove(0);
3901+
3902+
// selection and focus remain at level1
3903+
assertEquals(0, table.getSelectionModel().getSelectedIndex());
3904+
assertEquals("Node 0", table.getSelectionModel().getSelectedItem().getValue());
3905+
assertEquals(0, table.getFocusModel().getFocusedIndex());
3906+
assertEquals("Node 0", table.getFocusModel().getFocusedItem().getValue());
3907+
}
3908+
38353909
public static class MisbehavingOnCancelTreeCell<S> extends TreeCell<S> {
38363910

38373911
@Override

0 commit comments

Comments
 (0)