Skip to content

Commit 88aad93

Browse files
committedMay 15, 2024
8279140: ComboBox can lose selected value on item change via setAll
Reviewed-by: angorya
1 parent 97b1402 commit 88aad93

File tree

2 files changed

+64
-4
lines changed

2 files changed

+64
-4
lines changed
 

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

+1-4
Original file line numberDiff line numberDiff line change
@@ -618,14 +618,11 @@ public ComboBoxSelectionModel(final ComboBox<T> cb) {
618618
T selectedItem = getSelectedItem();
619619
for (int i = 0; i < comboBox.getItems().size(); i++) {
620620
if (selectedItem.equals(comboBox.getItems().get(i))) {
621-
comboBox.setValue(null);
622-
setSelectedItem(null);
623-
setSelectedIndex(i);
621+
clearAndSelect(i);
624622
break;
625623
}
626624
}
627625
}
628-
629626
comboBox.previousItemCount = getItemCount();
630627
}
631628
};

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

+63
Original file line numberDiff line numberDiff line change
@@ -2318,4 +2318,67 @@ public void test_rt_40012_selectedItemNotificationOnDisjointRemovesAbove() {
23182318

23192319
assertEquals("ComboBox skinProperty changed more than once, which is not expected.", 1, skinChangedCount);
23202320
}
2321+
2322+
//JDK-8279140
2323+
@Test
2324+
public void testSelectionOnItemChangeUsingSetAllThroughPropertyBinding() {
2325+
ObservableList<String> comboBoxItemsList = FXCollections.observableArrayList();
2326+
ObjectProperty<String> selectedValue = new SimpleObjectProperty<>();
2327+
2328+
List<String> strings1 = List.of("A", "B", "C");
2329+
List<String> strings2 = List.of("D", "E", "F");
2330+
2331+
comboBox = new ComboBox<>();
2332+
comboBox.setItems(comboBoxItemsList);
2333+
2334+
selectedValue.addListener((obs, oldValue, newValue) -> {
2335+
if ("D".equals(newValue) || "A".equals(newValue)) {
2336+
List<String> newContent = "A".equals(newValue) ? strings1 : strings2;
2337+
comboBoxItemsList.setAll(newContent);
2338+
}
2339+
});
2340+
2341+
comboBox.valueProperty().bindBidirectional(selectedValue);
2342+
2343+
selectedValue.set("A");
2344+
assertEquals("A", comboBox.getSelectionModel().getSelectedItem());
2345+
assertEquals("A", selectedValue.get());
2346+
2347+
selectedValue.set("D");
2348+
assertEquals("D", comboBox.getSelectionModel().getSelectedItem());
2349+
assertEquals("D", selectedValue.get());
2350+
}
2351+
2352+
//JDK-8279139
2353+
@Test
2354+
public void testSelectionOnItemChangeUsingSetAllOnButtonPress() {
2355+
ObservableList<String> comboBoxItemsList = FXCollections.observableArrayList();
2356+
2357+
List<String> strings1 = List.of("A", "B", "C");
2358+
List<String> strings2 = List.of("D", "B", "F");
2359+
2360+
comboBox = new ComboBox<>();
2361+
comboBox.setItems(comboBoxItemsList);
2362+
comboBox.setValue("B");
2363+
2364+
comboBoxItemsList.setAll(strings2);
2365+
assertEquals("B", comboBox.getSelectionModel().getSelectedItem());
2366+
comboBoxItemsList.setAll(strings1);
2367+
assertEquals("B", comboBox.getSelectionModel().getSelectedItem());
2368+
2369+
Button button = new Button("Change content");
2370+
button.setOnAction(e -> {
2371+
if (comboBoxItemsList.equals(strings1)) {
2372+
comboBoxItemsList.setAll(strings2);
2373+
} else {
2374+
comboBoxItemsList.setAll(strings1);
2375+
}
2376+
});
2377+
2378+
MouseEventFirer mouse = new MouseEventFirer(button);
2379+
mouse.fireMousePressAndRelease();
2380+
assertEquals("B", comboBox.getSelectionModel().getSelectedItem());
2381+
mouse.fireMousePressAndRelease();
2382+
assertEquals("B", comboBox.getSelectionModel().getSelectedItem());
2383+
}
23212384
}

0 commit comments

Comments
 (0)
Please sign in to comment.