8241999: ChoiceBox: incorrect toggle selected for uncontained

Reviewed-by: aghaisas
openjdk · May 5, 2020 · 99f7747 · 99f7747
1 parent 1cae6a8
commit 99f7747
Show file tree

Hide file tree

Showing 2 changed files with 48 additions and 11 deletions.
diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/ChoiceBox.java b/modules/javafx.controls/src/main/java/javafx/scene/control/ChoiceBox.java
@@ -591,6 +591,20 @@ public ChoiceBoxSelectionModel(final ChoiceBox<T> cb) {
             }
         }
 
+        /**
+         * {@inheritDoc} <p>
+         *
+         * Overridden to clear <code>selectedIndex</code> if <code>selectedItem</code> is not contained
+         * in the <code>items</code>.
+         */
+        @Override
+        public void select(T obj) {
+            super.select(obj);
+            if (obj != null && !choiceBox.getItems().contains(obj)) {
+                setSelectedIndex(-1);
+            }
+        }
+
         /** {@inheritDoc} */
         @Override public void selectPrevious() {
             // overridden to properly handle Separators

diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/ChoiceBoxSelectionTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/ChoiceBoxSelectionTest.java
@@ -29,7 +29,6 @@
 
 import org.junit.After;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import static org.junit.Assert.*;
@@ -51,18 +50,23 @@
 import javafx.stage.Stage;
 
 /**
- * Temporary collection of tests around state of toggle in popup.
+ * Collection of tests around state related to selection.
  * <p>
  *
+ * Tests that toggles are un/selected as required (JDK-8242489).
+ *
  * Note that the selection should be correct even if the
- * popup has not yet been shown! That's hampered by JDK-8242489 (see
- * analysis in bug).
+ * popup has not yet been shown.
  *
  * <p>
  * Need to test (testBaseToggle):
  * a) initial sync of selection state: selected toggle must be that of selectedIndex or none
  * b) change selection state after skin: selected toggle must follow
  *
+ * <p>
+ *
+ * Tests around selectedIndex and uncontained value (testSynced).
+ *
  */
 public class ChoiceBoxSelectionTest {
     private Scene scene;
@@ -266,13 +270,9 @@ protected void assertToggleSelected(int selectedIndex) {
         assertToggleSelected(box, selectedIndex);
     }
 
-    //------------- ignored tests, other issues
+    //------------- tests for JDK-8241999
 
-    /**
-     * Issue "8241999": toggle not unselected on setting uncontained value.
-     */
     @Test
-    @Ignore("8241999")
     public void testSyncedToggleUncontainedValue() {
         SingleSelectionModel<String> sm = box.getSelectionModel();
         sm.select(2);
@@ -285,13 +285,36 @@ public void testSyncedToggleUncontainedValue() {
      * Base reason for "8241999": selected index not sync'ed.
      */
     @Test
-    @Ignore("8241999")
     public void testSyncedSelectedIndexUncontained() {
         box.setValue(box.getItems().get(1));
         box.setValue(uncontained);
-        assertEquals(-1, box.getSelectionModel().getSelectedIndex());
+        assertEquals("selectedIndex for uncontained value ", -1, box.getSelectionModel().getSelectedIndex());
+    }
+
+    /**
+     * From review of JDK-8087555:
+     * select contained -> select uncontained -> clearselection -> nulls value
+     */
+    @Test
+    public void testSyncedSelectedOnPreselectedThenUncontained() {
+        box.setValue(box.getItems().get(1));
+        box.setValue(uncontained);
+        box.getSelectionModel().clearSelection();
+        assertEquals("uncontained value must be unchanged after clearSelection", uncontained, box.getValue());
+    }
+
+    /**
+     * From review of JDK-8087555:
+     * select uncontained -> clearselection -> nulls value
+     */
+    @Test
+    public void testSyncedClearSelectionUncontained() {
+        box.setValue(uncontained);
+        box.getSelectionModel().clearSelection();
+        assertEquals(uncontained, box.getValue());
     }
 
+
     //----------- setup and sanity test for initial state
 
     @Test