RFR: 8302558: Editable JComboBox 's popup blocks user from seeing characters in Acq look and feel

[Alexey Ivanov]

On Fri, 24 Feb 2023 21:54:30 GMT, Damon Nguyen <dnguyen at openjdk.org> wrote:

> The issue is in Aqua L&F when an editable JComboBox with a border is used. In this case, when the comboBox is clicked for the drop-down menu to show, the drop-down menu appears at the wrong coordinates (blocking the text of the comboBox and making it unreadable).
> 
> This seems to have been the case for a while and a similar issue appeared recently where an editable Aqua JComboBox also had wrong positioning due to having a border.
> 
> This fix checks for a border and modifies the bounds to accommodate the border's size. Then the usual calculations for the comboBox popup works as expected.
> 
> The new headful test creates an editable comboBox with a TitledBorder and with no border. Then, it automatically clicks the comboBox to open the popup, and clicks where the position of the first selectionItem should be. Finally, it checks if the selected item is correct. This is for all L&F's and the test passes on all OS's.

Changes requested by aivanov (Reviewer).

src/java.desktop/macosx/classes/com/apple/laf/AquaComboBoxPopup.java line 189:

> 187: 
> 188:         int popupBoundsY = comboBox.getBounds().height;
> 189:         if (comboBox.isEditable() && comboBox.getBorder() != null) {

Does it not affect a non-editable combo box with with a border that has large insets?

test/jdk/javax/swing/JComboBox/EditableComboBoxPopupPos.java line 62:

> 60: 
> 61:             for (UIManager.LookAndFeelInfo laf : UIManager.getInstalledLookAndFeels()) {
> 62:                 lafName = laf.getName().equals("CDE/Motif") ? "Motif" : laf.getName();

Is it really needed? Is `CDE/Motif` such a bad name that it needs adjusting?

test/jdk/javax/swing/JComboBox/EditableComboBoxPopupPos.java line 89:

> 87:                 // first selection item is in the correct position on screen.
> 88:                 cb1.setSelectedIndex(1);
> 89:                 cb2.setSelectedIndex(1);

These methods should be called on EDT.

test/jdk/javax/swing/JComboBox/EditableComboBoxPopupPos.java line 97:

> 95:             }
> 96:         } catch (Exception e) {
> 97:             e.printStackTrace();

You should not catch the exception but propagate it for jtreg to fail the test if it throws the exception. Otherwise, the test is considered passed even though it may print `Test failed`.

test/jdk/javax/swing/JComboBox/EditableComboBoxPopupPos.java line 117:

> 115:         robot.mouseMove(cb.getLocationOnScreen().x + cb.getWidth()
> 116:                 - BUTTON_OFFSET, cb.getLocationOnScreen().y
> 117:                 + POPUP_OFFSET);

`getLocationOnScreen()`, `getWidth()` should be called on EDT.

test/jdk/javax/swing/JComboBox/EditableComboBoxPopupPos.java line 131:

> 129:             throws InterruptedException, InvocationTargetException {
> 130:         if (c1.getSelectedItem().toString().equals("One")
> 131:                 && c2.getSelectedItem().toString().equals("One")) {

`getSelectedItem` should be called on EDT.

test/jdk/javax/swing/JComboBox/EditableComboBoxPopupPos.java line 135:

> 133:             SwingUtilities.invokeAndWait(() -> frame.dispose());
> 134:         } else {
> 135:             SwingUtilities.invokeAndWait(() -> frame.dispose());

Your `main` method disposes of the frame. You should just move try-finally block into the for-loop over available Look-and-Feels.

test/jdk/javax/swing/JComboBox/EditableComboBoxPopupPos.java line 139:

> 137:         }
> 138:     }
> 139: }

Please add a new line character to the end of the file.

-------------

PR: https://git.openjdk.org/jdk/pull/12750