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

[Damon Nguyen]

On Mon, 27 Feb 2023 19:11:43 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> Damon Nguyen has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Propagate exceptions. Move methods to EDT.
>
> 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?

This was added because the `/` caused issues while debugging with file paths, but it's no longer needed. Updated

> 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.

Updated

> 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`.

Good point. I propagated the exception and re-tested

> 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.

Updated. I moved these accessors out of the method and now call them on the EDT and store the value in vars.

> 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.

Same as above

> 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.

Added to the bottom of the loop

> test/jdk/javax/swing/JComboBox/EditableComboBoxPopupPos.java line 139:
> 
>> 137:         }
>> 138:     }
>> 139: }
> 
> Please add a new line character to the end of the file.

Updated

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

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