4199622: RFE: JComboBox shouldn't sending ActionEvents for keyboard navigation

Reviewed-by: alexp, alexsch

jdk/src/share/classes/javax/swing/plaf/basic/BasicComboBoxUI.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2011, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -1120,7 +1120,9 @@ public class BasicComboBoxUI extends ComboBoxUI {
             listBox.setSelectedIndex( si + 1 );
             listBox.ensureIndexIsVisible( si + 1 );
             if ( !isTableCellEditor ) {
-                comboBox.setSelectedIndex(si+1);
+                if (!(UIManager.getBoolean("ComboBox.noActionOnKeyNavigation") && comboBox.isPopupVisible())) {
+                    comboBox.setSelectedIndex(si+1);
+                }
             }
             comboBox.repaint();
         }
@@ -1144,7 +1146,9 @@ public class BasicComboBoxUI extends ComboBoxUI {
             listBox.setSelectedIndex( si - 1 );
             listBox.ensureIndexIsVisible( si - 1 );
             if ( !isTableCellEditor ) {
-                comboBox.setSelectedIndex(si-1);
+                if (!(UIManager.getBoolean("ComboBox.noActionOnKeyNavigation") && comboBox.isPopupVisible())) {
+                    comboBox.setSelectedIndex(si-1);
+                }
             }
             comboBox.repaint();
         }
@@ -1490,7 +1494,13 @@ public class BasicComboBoxUI extends ComboBoxUI {
                      key == HOME || key == END) {
                 int index = getNextIndex(comboBox, key);
                 if (index >= 0 && index < comboBox.getItemCount()) {
-                    comboBox.setSelectedIndex(index);
+                    if (UIManager.getBoolean("ComboBox.noActionOnKeyNavigation") && comboBox.isPopupVisible()) {
+                        ui.listBox.setSelectedIndex(index);
+                        ui.listBox.ensureIndexIsVisible(index);
+                        comboBox.repaint();
+                    } else {
+                        comboBox.setSelectedIndex(index);
+                    }
                 }
             }
             else if (key == DOWN) {
@@ -1558,22 +1568,33 @@ public class BasicComboBoxUI extends ComboBoxUI {
 
             else if (key == ENTER) {
                 if (comboBox.isPopupVisible()) {
-                    // Forces the selection of the list item
+                    // If ComboBox.noActionOnKeyNavigation is set,
-                    boolean isEnterSelectablePopup =
+                    // forse selection of list item
-                            UIManager.getBoolean("ComboBox.isEnterSelectablePopup");
+                    if (UIManager.getBoolean("ComboBox.noActionOnKeyNavigation")) {
-                    if (!comboBox.isEditable() || isEnterSelectablePopup
-                            || ui.isTableCellEditor) {
                         Object listItem = ui.popup.getList().getSelectedValue();
                         if (listItem != null) {
-                            // Use the selected value from popup
-                            // to set the selected item in combo box,
-                            // but ensure before that JComboBox.actionPerformed()
-                            // won't use editor's value to set the selected item
                             comboBox.getEditor().setItem(listItem);
                             comboBox.setSelectedItem(listItem);
                         }
+                        comboBox.setPopupVisible(false);
+                    } else {
+                        // Forces the selection of the list item
+                        boolean isEnterSelectablePopup =
+                                UIManager.getBoolean("ComboBox.isEnterSelectablePopup");
+                        if (!comboBox.isEditable() || isEnterSelectablePopup
+                                || ui.isTableCellEditor) {
+                            Object listItem = ui.popup.getList().getSelectedValue();
+                            if (listItem != null) {
+                                // Use the selected value from popup
+                                // to set the selected item in combo box,
+                                // but ensure before that JComboBox.actionPerformed()
+                                // won't use editor's value to set the selected item
+                                comboBox.getEditor().setItem(listItem);
+                                comboBox.setSelectedItem(listItem);
+                            }
+                        }
+                        comboBox.setPopupVisible(false);
                     }
-                    comboBox.setPopupVisible(false);
                 }
                 else {
                     // Hide combo box if it is a table cell editor
@@ -1604,14 +1625,20 @@ public class BasicComboBoxUI extends ComboBoxUI {
         }
 
         private int getNextIndex(JComboBox comboBox, String key) {
+            int listHeight = comboBox.getMaximumRowCount();
+
+            int selectedIndex = comboBox.getSelectedIndex();
+            if (UIManager.getBoolean("ComboBox.noActionOnKeyNavigation")
+                    && (comboBox.getUI() instanceof BasicComboBoxUI)) {
+                selectedIndex = ((BasicComboBoxUI) comboBox.getUI()).listBox.getSelectedIndex();
+            }
+
             if (key == PAGE_UP) {
-                int listHeight = comboBox.getMaximumRowCount();
+                int index = selectedIndex - listHeight;
-                int index = comboBox.getSelectedIndex() - listHeight;
                 return (index < 0 ? 0: index);
             }
             else if (key == PAGE_DOWN) {
-                int listHeight = comboBox.getMaximumRowCount();
+                int index = selectedIndex + listHeight;
-                int index = comboBox.getSelectedIndex() + listHeight;
                 int max = comboBox.getItemCount();
                 return (index < max ? index: max-1);
             }

jdk/src/share/classes/javax/swing/plaf/basic/BasicLookAndFeel.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2010, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 2013, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -861,6 +861,7 @@ public abstract class BasicLookAndFeel extends LookAndFeel implements Serializab
                          "END", "endPassThrough",
                        "ENTER", "enterPressed"
                  }),
+            "ComboBox.noActionOnKeyNavigation", Boolean.FALSE,
 
             // *** FileChooser
 

jdk/test/javax/swing/JComboBox/4199622/bug4199622.java

@@ -0,0 +1,243 @@
+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/* @test
+   @bug 4199622
+   @summary RFE: JComboBox shouldn't send ActionEvents for keyboard navigation
+   @author Vladislav Karnaukhov
+   @run main bug4199622
+*/
+
+import com.sun.java.swing.plaf.windows.WindowsLookAndFeel;
+import sun.awt.OSInfo;
+import sun.awt.SunToolkit;
+
+import javax.swing.*;
+import javax.swing.plaf.metal.MetalLookAndFeel;
+import java.awt.*;
+import java.awt.event.ActionEvent;
+import java.awt.event.ActionListener;
+import java.awt.event.KeyEvent;
+import java.lang.reflect.InvocationTargetException;
+
+public class bug4199622 extends JFrame implements ActionListener {
+
+    static final int nElems = 20;
+    static JComboBox<String> cb = null;
+
+    bug4199622(LookAndFeel laf) {
+        super();
+
+        try {
+            UIManager.setLookAndFeel(laf);
+        } catch (UnsupportedLookAndFeelException e) {
+            throw new RuntimeException("Test failed", e);
+        }
+
+        setDefaultCloseOperation(DISPOSE_ON_CLOSE);
+        cb = new JComboBox<>();
+        for (int i = 0; i < nElems; i++) {
+            cb.addItem(String.valueOf(i + 1));
+        }
+        cb.addActionListener(this);
+        add(cb);
+
+        setSize(300, 300);
+        pack();
+    }
+
+    @Override
+    public void actionPerformed(ActionEvent e) {
+        if (UIManager.getBoolean("ComboBox.noActionOnKeyNavigation") && cb.isPopupVisible()) {
+            throw new RuntimeException("Test failed. actionPerformed generated");
+        }
+    }
+
+    static Robot robot = null;
+    static SunToolkit toolkit = null;
+
+    static void doTest() {
+        if (robot == null) {
+            try {
+                robot = new Robot();
+                robot.setAutoDelay(20);
+            } catch (AWTException e) {
+                throw new RuntimeException("Can't create robot. Test failed", e);
+            }
+        }
+
+        toolkit = (SunToolkit) Toolkit.getDefaultToolkit();
+        if (toolkit == null) {
+            throw new RuntimeException("Can't get the toolkit. Test failed");
+        }
+        toolkit.realSync();
+
+        doActualTest();
+
+        try {
+            SwingUtilities.invokeAndWait(new Runnable() {
+                @Override
+                public void run() {
+                    cb.hidePopup();
+                    cb.setEditable(true);
+                    cb.updateUI();
+                }
+            });
+        } catch (InterruptedException e) {
+            throw new RuntimeException("Test failed", e);
+        } catch (InvocationTargetException e) {
+            throw new RuntimeException("Test failed", e);
+        }
+
+        toolkit.realSync();
+        doActualTest();
+    }
+
+    static void doActualTest() {
+        UIManager.put("ComboBox.noActionOnKeyNavigation", true);
+        doTestUpDown();
+        UIManager.put("ComboBox.noActionOnKeyNavigation", false);
+        doTestUpDown();
+
+        UIManager.put("ComboBox.noActionOnKeyNavigation", true);
+        doTestPgUpDown();
+        UIManager.put("ComboBox.noActionOnKeyNavigation", false);
+        doTestPgUpDown();
+
+        UIManager.put("ComboBox.noActionOnKeyNavigation", true);
+        doTestHomeEnd();
+        UIManager.put("ComboBox.noActionOnKeyNavigation", false);
+        doTestHomeEnd();
+    }
+
+    static void doTestHomeEnd() {
+        try {
+            SwingUtilities.invokeAndWait(new Runnable() {
+                @Override
+                public void run() {
+                    cb.hidePopup();
+                    cb.setSelectedIndex(0);
+                }
+            });
+        } catch (InterruptedException e) {
+            throw new RuntimeException("Test failed", e);
+        } catch (InvocationTargetException e) {
+            throw new RuntimeException("Test failed", e);
+        }
+        toolkit.realSync();
+
+        robot.keyPress(KeyEvent.VK_END);
+        toolkit.realSync();
+        robot.keyPress(KeyEvent.VK_HOME);
+        toolkit.realSync();
+    }
+
+    static void doTestUpDown() {
+        try {
+            SwingUtilities.invokeAndWait(new Runnable() {
+                @Override
+                public void run() {
+                    cb.hidePopup();
+                    cb.setSelectedIndex(0);
+                }
+            });
+        } catch (InterruptedException e) {
+            throw new RuntimeException("Test failed", e);
+        } catch (InvocationTargetException e) {
+            throw new RuntimeException("Test failed", e);
+        }
+        toolkit.realSync();
+
+        for (int i = 0; i < nElems; i++) {
+            robot.keyPress(KeyEvent.VK_DOWN);
+            toolkit.realSync();
+        }
+
+        for (int i = 0; i < nElems; i++) {
+            robot.keyPress(KeyEvent.VK_UP);
+            toolkit.realSync();
+        }
+    }
+
+    static void doTestPgUpDown() {
+        try {
+            SwingUtilities.invokeAndWait(new Runnable() {
+                @Override
+                public void run() {
+                    cb.hidePopup();
+                    cb.setSelectedIndex(0);
+                }
+            });
+        } catch (InterruptedException e) {
+            throw new RuntimeException("Test failed", e);
+        } catch (InvocationTargetException e) {
+            throw new RuntimeException("Test failed", e);
+        }
+        toolkit.realSync();
+
+        int listHeight = cb.getMaximumRowCount();
+        for (int i = 0; i < nElems; i += listHeight) {
+            robot.keyPress(KeyEvent.VK_PAGE_DOWN);
+            toolkit.realSync();
+        }
+
+        for (int i = 0; i < nElems; i += listHeight) {
+            robot.keyPress(KeyEvent.VK_PAGE_UP);
+            toolkit.realSync();
+        }
+    }
+
+    public static void main(String[] args) {
+        try {
+            SwingUtilities.invokeAndWait(new Runnable() {
+                @Override
+                public void run() {
+                    bug4199622 test = new bug4199622(new MetalLookAndFeel());
+                    test.setVisible(true);
+                }
+            });
+        } catch (InterruptedException e) {
+            throw new RuntimeException("Test failed", e);
+        } catch (InvocationTargetException e) {
+            throw new RuntimeException("Test failed", e);
+        }
+        doTest();
+
+        if (OSInfo.getOSType() == OSInfo.OSType.WINDOWS) {
+            try {
+                SwingUtilities.invokeAndWait(new Runnable() {
+                    @Override
+                    public void run() {
+                        bug4199622 test = new bug4199622(new WindowsLookAndFeel());
+                        test.setVisible(true);
+                    }
+                });
+            } catch (InterruptedException e) {
+                throw new RuntimeException("Test failed", e);
+            } catch (InvocationTargetException e) {
+                throw new RuntimeException("Test failed", e);
+            }
+            doTest();
+        }
+    }
+}