changeset 6554:c61f0f745e0d

RT-36280: ComboBox's popup cannot be closed with ENTER
author jgiles
date Tue, 25 Mar 2014 15:43:25 +1300
parents fd6893fc1047
children e530f77a64a4
files modules/controls/src/main/java/com/sun/javafx/scene/control/skin/ComboBoxListViewSkin.java modules/controls/src/main/java/com/sun/javafx/scene/control/skin/ComboBoxPopupControl.java modules/controls/src/test/java/javafx/scene/control/ComboBoxTest.java
diffstat 3 files changed, 300 insertions(+), 37 deletions(-) [+]
line wrap: on
line diff
--- a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/ComboBoxListViewSkin.java	Tue Mar 25 11:28:51 2014 +1300
+++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/ComboBoxListViewSkin.java	Tue Mar 25 15:43:25 2014 +1300
@@ -147,18 +147,20 @@
 
         comboBox.addEventFilter(KeyEvent.ANY, new EventHandler<KeyEvent>() {
             @Override public void handle(KeyEvent ke) {
-                if (textField == null) return;
+                if (textField == null) {
+                    handleKeyEvent(ke, false);
+                } else {
+                    // This prevents a stack overflow from our rebroadcasting of the
+                    // event to the textfield that occurs in the final else statement
+                    // of the conditions below.
+                    if (ke.getTarget().equals(textField)) return;
 
-                // This prevents a stack overflow from our rebroadcasting of the
-                // event to the textfield that occurs in the final else statement
-                // of the conditions below.
-                if (ke.getTarget().equals(textField)) return;
-
-                // Fix for the regression noted in a comment in RT-29885.
-                // This forwards the event down into the TextField when
-                // the key event is actually received by the ComboBox.
-                textField.fireEvent(ke.copyFor(textField, textField));
-                ke.consume();
+                    // Fix for the regression noted in a comment in RT-29885.
+                    // This forwards the event down into the TextField when
+                    // the key event is actually received by the ComboBox.
+                    textField.fireEvent(ke.copyFor(textField, textField));
+                    ke.consume();
+                }
             }
         });
 
@@ -166,27 +168,7 @@
             textField.addEventFilter(KeyEvent.ANY, new EventHandler<KeyEvent>() {
                 @Override public void handle(KeyEvent ke) {
                     if (textField == null) return;
-
-                    // When the user hits the enter or F4 keys, we respond before
-                    // ever giving the event to the TextField.
-                    if (ke.getCode() == KeyCode.ENTER) {
-                        setTextFromTextFieldIntoComboBoxValue();
-                        ke.consume();
-                        return;
-                    } else if (ke.getCode() == KeyCode.F4 && ke.getEventType() == KeyEvent.KEY_RELEASED) {
-                        if (comboBox.isShowing()) comboBox.hide();
-                        else comboBox.show();
-                        ke.consume();
-                        return;
-                    } else if (ke.getCode() == KeyCode.F10 || ke.getCode() == KeyCode.ESCAPE) {
-                        // RT-23275: The TextField fires F10 and ESCAPE key events
-                        // up to the parent, which are then fired back at the
-                        // TextField, and this ends up in an infinite loop until
-                        // the stack overflows. So, here we consume these two
-                        // events and stop them from going any further.
-                        ke.consume();
-                        return;
-                    }
+                    handleKeyEvent(ke, true);
                 }
             });
         }
@@ -221,9 +203,9 @@
         registerChangeListener(comboBox.buttonCellProperty(), "BUTTON_CELL");
         registerChangeListener(comboBox.valueProperty(), "VALUE");
     }
-    
-    
-    
+
+
+
     /***************************************************************************
      *                                                                         *
      * Public API                                                              *
@@ -347,8 +329,31 @@
      *                                                                         *
      * Private methods                                                         *
      *                                                                         *
-     **************************************************************************/    
-    
+     **************************************************************************/
+
+    private void handleKeyEvent(KeyEvent ke, boolean doConsume) {
+        // When the user hits the enter or F4 keys, we respond before
+        // ever giving the event to the TextField.
+        if (ke.getCode() == KeyCode.ENTER) {
+            setTextFromTextFieldIntoComboBoxValue();
+
+            if (doConsume) ke.consume();
+        } else if (ke.getCode() == KeyCode.F4) {
+            if (ke.getEventType() == KeyEvent.KEY_RELEASED) {
+                if (comboBox.isShowing()) comboBox.hide();
+                else comboBox.show();
+            }
+            ke.consume(); // we always do a consume here (otherwise unit tests fail)
+        } else if (ke.getCode() == KeyCode.F10 || ke.getCode() == KeyCode.ESCAPE) {
+            // RT-23275: The TextField fires F10 and ESCAPE key events
+            // up to the parent, which are then fired back at the
+            // TextField, and this ends up in an infinite loop until
+            // the stack overflows. So, here we consume these two
+            // events and stop them from going any further.
+            if (doConsume) ke.consume();
+        }
+    }
+
     private void updateValue() {
         T newValue = comboBox.getValue();
         
--- a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/ComboBoxPopupControl.java	Tue Mar 25 11:28:51 2014 +1300
+++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/ComboBoxPopupControl.java	Tue Mar 25 15:43:25 2014 +1300
@@ -95,6 +95,8 @@
         _popup.getScene().setNodeOrientation(getSkinnable().getEffectiveNodeOrientation());
         reconfigurePopup();
         _popup.show(comboBoxBase.getScene().getWindow(), p.getX(), p.getY());
+
+        getPopupContent().requestFocus();
     }
     
     private void createPopup() {
--- a/modules/controls/src/test/java/javafx/scene/control/ComboBoxTest.java	Tue Mar 25 11:28:51 2014 +1300
+++ b/modules/controls/src/test/java/javafx/scene/control/ComboBoxTest.java	Tue Mar 25 15:43:25 2014 +1300
@@ -25,6 +25,7 @@
 
 package javafx.scene.control;
 
+import com.sun.javafx.scene.control.infrastructure.KeyModifier;
 import com.sun.javafx.tk.Toolkit;
 import javafx.css.PseudoClass;
 
@@ -1261,4 +1262,259 @@
         keyboard.doKeyPress(KeyCode.ENTER);
         assertEquals("TEST", cb.getValue());
     }
+
+    @Test public void test_rt36280_nonEditable_F4ShowsPopup() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        assertFalse(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.F4);  // show the popup
+        assertTrue(cb.isShowing());
+    }
+
+    @Test public void test_rt36280_nonEditable_altUpShowsPopup() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        assertFalse(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.UP, KeyModifier.ALT);  // show the popup
+        assertTrue(cb.isShowing());
+    }
+
+    @Test public void test_rt36280_nonEditable_altDownShowsPopup() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        new StageLoader(cb);
+
+        assertFalse(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.DOWN, KeyModifier.ALT);  // show the popup
+        assertTrue(cb.isShowing());
+    }
+
+    @Test public void test_rt36280_nonEditable_enterHidesShowingPopup() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        ListView listView = ((ComboBoxListViewSkin)cb.getSkin()).getListView();
+        assertNotNull(listView);
+
+        KeyEventFirer lvKeyboard = new KeyEventFirer(listView);
+
+        assertFalse(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.F4);  // show the popup
+        assertTrue(cb.isShowing());
+        lvKeyboard.doKeyPress(KeyCode.ENTER);  // hide the popup
+        assertFalse(cb.isShowing());
+    }
+
+    @Test public void test_rt36280_nonEditable_spaceHidesShowingPopup() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        ListView listView = ((ComboBoxListViewSkin)cb.getSkin()).getListView();
+        assertNotNull(listView);
+
+        KeyEventFirer lvKeyboard = new KeyEventFirer(listView);
+
+        assertFalse(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.F4);  // show the popup
+        assertTrue(cb.isShowing());
+        lvKeyboard.doKeyPress(KeyCode.SPACE);  // hide the popup
+        assertFalse(cb.isShowing());
+    }
+
+    @Test public void test_rt36280_nonEditable_escapeHidesShowingPopup() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        ListView listView = ((ComboBoxListViewSkin)cb.getSkin()).getListView();
+        assertNotNull(listView);
+
+        KeyEventFirer lvKeyboard = new KeyEventFirer(listView);
+
+        assertFalse(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.F4);  // show the popup
+        assertTrue(cb.isShowing());
+        lvKeyboard.doKeyPress(KeyCode.ESCAPE);  // hide the popup
+        assertFalse(cb.isShowing());
+    }
+
+    @Test public void test_rt36280_nonEditable_F4HidesShowingPopup() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        assertFalse(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.F4);  // show the popup
+        assertTrue(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.F4);  // hide the popup
+        assertFalse(cb.isShowing());
+    }
+
+    @Test public void test_rt36280_nonEditable_arrowKeysChangeSelection() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        assertFalse(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.F4);  // show the popup
+        assertTrue(cb.isShowing());
+
+        assertNull(cb.getSelectionModel().getSelectedItem());
+
+        cbKeyboard.doDownArrowPress();
+        assertEquals("a", cb.getSelectionModel().getSelectedItem());
+
+        cbKeyboard.doDownArrowPress();
+        assertEquals("b", cb.getSelectionModel().getSelectedItem());
+
+        cbKeyboard.doUpArrowPress();
+        assertEquals("a", cb.getSelectionModel().getSelectedItem());
+    }
+
+
+
+    @Test public void test_rt36280_editable_F4ShowsPopup() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        cb.setEditable(true);
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        assertFalse(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.F4);  // show the popup
+        assertTrue(cb.isShowing());
+    }
+
+    @Test public void test_rt36280_editable_altUpShowsPopup() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        cb.setEditable(true);
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        assertFalse(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.UP, KeyModifier.ALT);  // show the popup
+        assertTrue(cb.isShowing());
+    }
+
+    @Test public void test_rt36280_editable_altDownShowsPopup_onComboBox() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        cb.setEditable(true);
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        assertFalse(cb.isShowing());
+        assertTrue(cb.getEditor().getText().isEmpty());
+        cbKeyboard.doKeyPress(KeyCode.DOWN, KeyModifier.ALT);  // show the popup
+        assertTrue(cb.isShowing());
+        assertTrue(cb.getEditor().getText().isEmpty());
+    }
+
+    @Test public void test_rt36280_editable_altDownShowsPopup_onTextField() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        cb.setEditable(true);
+        new StageLoader(cb);
+
+        KeyEventFirer tfKeyboard = new KeyEventFirer(cb.getEditor());
+        assertFalse(cb.isShowing());
+        assertTrue(cb.getEditor().getText().isEmpty());
+        tfKeyboard.doKeyPress(KeyCode.DOWN, KeyModifier.ALT);  // show the popup
+        assertTrue(cb.isShowing());
+        assertTrue(cb.getEditor().getText().isEmpty());
+    }
+
+    @Test public void test_rt36280_editable_enterHidesShowingPopup() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        cb.setEditable(true);
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        ListView listView = ((ComboBoxListViewSkin)cb.getSkin()).getListView();
+        assertNotNull(listView);
+
+        KeyEventFirer lvKeyboard = new KeyEventFirer(listView);
+
+        assertFalse(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.F4);  // show the popup
+        assertTrue(cb.isShowing());
+        lvKeyboard.doKeyPress(KeyCode.ENTER);  // hide the popup
+        assertFalse(cb.isShowing());
+    }
+
+    @Test public void test_rt36280_editable_spaceHidesShowingPopup() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        cb.setEditable(true);
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        ListView listView = ((ComboBoxListViewSkin)cb.getSkin()).getListView();
+        assertNotNull(listView);
+
+        KeyEventFirer lvKeyboard = new KeyEventFirer(listView);
+
+        assertFalse(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.F4);  // show the popup
+        assertTrue(cb.isShowing());
+        lvKeyboard.doKeyPress(KeyCode.SPACE);  // hide the popup
+        assertFalse(cb.isShowing());
+    }
+
+    @Test public void test_rt36280_editable_escapeHidesShowingPopup() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        cb.setEditable(true);
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        ListView listView = ((ComboBoxListViewSkin)cb.getSkin()).getListView();
+        assertNotNull(listView);
+
+        KeyEventFirer lvKeyboard = new KeyEventFirer(listView);
+
+        assertFalse(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.F4);  // show the popup
+        assertTrue(cb.isShowing());
+        lvKeyboard.doKeyPress(KeyCode.ESCAPE);  // hide the popup
+        assertFalse(cb.isShowing());
+    }
+
+    @Test public void test_rt36280_editable_F4HidesShowingPopup() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        cb.setEditable(true);
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        assertFalse(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.F4);  // show the popup
+        assertTrue(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.F4);  // hide the popup
+        assertFalse(cb.isShowing());
+    }
+
+    @Test public void test_rt36280_editable_arrowKeysChangeSelection() {
+        final ComboBox<String> cb = new ComboBox<>(FXCollections.observableArrayList("a", "b", "c"));
+        cb.setEditable(true);
+        new StageLoader(cb);
+        KeyEventFirer cbKeyboard = new KeyEventFirer(cb);
+
+        assertFalse(cb.isShowing());
+        cbKeyboard.doKeyPress(KeyCode.F4);  // show the popup
+        assertTrue(cb.isShowing());
+
+        assertNull(cb.getSelectionModel().getSelectedItem());
+
+        cbKeyboard.doDownArrowPress();
+        assertEquals("a", cb.getSelectionModel().getSelectedItem());
+
+        cbKeyboard.doDownArrowPress();
+        assertEquals("b", cb.getSelectionModel().getSelectedItem());
+
+        cbKeyboard.doUpArrowPress();
+        assertEquals("a", cb.getSelectionModel().getSelectedItem());
+    }
 }