On Fri, 25 Apr 2025 18:35:09 GMT, Anass Baya <ab...@openjdk.org> wrote:
>> This test was designed to manually verify that clicking on the JComboBox
>> when the frame containing it is about to close does not cause an
>> IllegalStateException.
>>
>> The test allowed the tester extra time to click on the JComboBox when
>> closing the frame by adding a Thread.sleep() in the close button handler.
>>
>> In this test, a JComboBox is displayed with a Close button at the bottom.
>> The tester should click the Close button, then try to click the JComboBox
>> arrow button to display the popup.
>>
>> In the automated test, we save the JComboBox location size before closing
>> the frame. We then use this information to click on the JComboBox right
>> before the frame is closed.
>
> Anass Baya has updated the pull request incrementally with three additional
> commits since the last revision:
>
> - Put the frame in the centre of the screen.
>
> Co-authored-by: Alexey Ivanov <alexey.iva...@oracle.com>
> - library regtesthelpers is no more used
> - Alexey's proposed enhancement
Changes requested by aivanov (Reviewer).
Changes requested by aivanov (Reviewer).
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 38:
> 36: * @key headful
> 37: * @summary Verifies clicking JComboBox during frame closure causes
> Exception
> 38: * @library /javax/swing/regtesthelpers
The library isn't used any more.
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 62:
> 60: if (frame != null) {
> 61: SwingUtilities.invokeAndWait(() -> frame.dispose());
> 62: }
The null-check should be on EDT too.
This comment is still not resolved.
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 63:
> 61: if (frame != null) {
> 62: SwingUtilities.invokeAndWait(() -> frame.dispose());
> 63: }
The null-check should be on EDT too, all the tests follow this pattern.
Suggestion:
SwingUtilities.invokeAndWait(() -> {
if (frame != null) {
frame.dispose();
}
});
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 94:
> 92: frame.getContentPane().add(closeButton, "South");
> 93: frame.setSize(200, 200);
> 94: frame.setVisible(true);
Suggestion:
frame.setSize(200, 200);
frame.setLocationRelativeTo(null);
frame.setVisible(true);
Put the frame in the centre of the screen.
-------------
PR Review: https://git.openjdk.org/jdk/pull/24624#pullrequestreview-2791603059
PR Review: https://git.openjdk.org/jdk/pull/24624#pullrequestreview-2794945992
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2058690451
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2060699938
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2058699525
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2060704263
