On Tue, 22 Apr 2025 16:11:28 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 one additional
> commit since the last revision:
>
> Update Copyright
Changes requested by aivanov (Reviewer).
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 62:
> 60:
> 61: private static JFrame createUI() {
> 62: JFrame frame = new JFrame("ComboPopup");
To reduce the number of changes compared to the previous version, I suggest
keeping `createUI` method, just change its return type to `void` and pass the
reference `ComboPopupBug::createUI` to `invokeAndWait`.
This way would be cleaner, I think.
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 66:
> 64: clickComboBox();
> 65: frame.setVisible(false);
> 66: });
Reduce indentation.
Suggestion:
closeButton.addActionListener((e) -> {
clickComboBox();
frame.setVisible(false);
});
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 80:
> 78: SwingUtilities.invokeAndWait(() -> closeButton.doClick());
> 79: }
> 80: finally {
Use Java code style.
Suggestion:
} finally {
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 85:
> 83: }
> 84:
> 85: public static void clickComboBox() {
Suggestion:
private static void clickComboBox() {
Not that important, yet I prefer having all the methods `private`.
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 87:
> 85: public static void clickComboBox() {
> 86: comboBoxLocation = comboBox.getLocationOnScreen();
> 87: comboBoxSize = comboBox.getSize();
Both `comboBoxLocation` and `comboBoxSize` should be local variables declared
here, in the `clickComboBox` method.
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 92:
> 90: comboBoxLocation.y + comboBoxSize.height / 2);
> 91: robot.mousePress(InputEvent.BUTTON1_MASK);
> 92: robot.mouseRelease(InputEvent.BUTTON1_MASK);
Use the recommended constants instead of deprecated ones.
Suggestion:
robot.mousePress(InputEvent.BUTTON1_DOWN_MASK);
robot.mouseRelease(InputEvent.BUTTON1_DOWN_MASK);
-------------
PR Review: https://git.openjdk.org/jdk/pull/24624#pullrequestreview-2786723132
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2055717632
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2055699136
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2055702144
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2055707240
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2055705815
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2055721502
