On Mon, 14 Apr 2025 13:50:54 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.
Changes requested by aivanov (Reviewer).
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 1:
> 1: /*
Please update the copyright year to `2024, 2025,`
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 51:
> 49:
> 50: public static void main(String[] args) throws Exception {
> 51: robot = JRobot.getRobot();
You don't use any of `JRobot` features, just use the `Robot` class: `new
Robot()`.
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 71:
> 69: frame.setSize(200, 200);
> 70: frame.setVisible(true);
> 71: });
There must be `robot.waitForIdle` to ensure the UI becomes visible on the
screen before you get the location of the combo box on the screen.
It's also common to add `robot.delay(1000)`.
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 80:
> 78: }
> 79: catch (Exception e) {
> 80: throw new RuntimeException(e);
Swing is not thread-safe, you should get the location and size on EDT too.
Suggestion:
try {
SwingUtilities.invokeAndWait(() ->{
comboBoxLocation = comboBox.getLocationOnScreen();
comboBoxSize = comboBox.getSize();
closeButton.doClick();
});
}
catch (Exception e) {
throw new RuntimeException(e);
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 80:
> 78: }
> 79: catch (Exception e) {
> 80: throw new RuntimeException(e);
There's no need to catch the exception, just let it propagate.
test/jdk/javax/swing/JComboBox/ComboPopupBug.java line 89:
> 87: public static void clickComboBox() {
> 88: int padding = 10;
> 89: robot.mouseMove(comboBoxLocation.x + comboBoxSize.width -
> padding, comboBoxLocation.y + comboBoxSize.height / 2);
I'm sure this line doesn't fit into 80-column limit, I recommend wrapping it.
Suggestion:
robot.mouseMove(comboBoxLocation.x + comboBoxSize.width - padding,
comboBoxLocation.y + comboBoxSize.height / 2);
Padding or offset could be a constant declared statically.
-------------
PR Review: https://git.openjdk.org/jdk/pull/24624#pullrequestreview-2764590444
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2042302246
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2042300846
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2042306252
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2042290016
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2042292563
PR Review Comment: https://git.openjdk.org/jdk/pull/24624#discussion_r2042297780
