On Wed, 27 Aug 2025 20:04:28 GMT, Alexey Ivanov <aiva...@openjdk.org> wrote:
>> Damon Nguyen has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Review comment update
>
> test/jdk/java/awt/List/MouseDraggedOriginatedByScrollBarTest.java line 124:
>
>> 122: } catch (Exception ex) {
>> 123: throw new RuntimeException("Can't create robot");
>> 124: }
>
> Suggestion:
>
> Robot robot = new Robot();
>
> The `test` method has the `throws` clause with `Exception`, just let
> `AWTException` propagate.
Done.
> test/jdk/java/awt/List/MouseDraggedOriginatedByScrollBarTest.java line 129:
>
>> 127: robot.setAutoWaitForIdle(true);
>> 128:
>> 129: // Focus default button and wait till it gets focus
>
> The comment is confusing, it doesn't correspond to the code below.
Removed.
> test/jdk/java/awt/List/MouseDraggedOriginatedByScrollBarTest.java line 132:
>
>> 130: EventQueue.invokeAndWait(() -> {
>> 131: loc = list.getLocationOnScreen();
>> 132: width = list.getWidth();
>
> Suggestion:
>
> Point p = list.getLocationOnScreen();
> p.translate(list.getWidth(), 20);
> loc = p;
>
> Then `width` becomes redundant.
Edited!
> test/jdk/java/awt/List/MouseDraggedOriginatedByScrollBarTest.java line 139:
>
>> 137: Point p = MouseInfo.getPointerInfo().getLocation();
>> 138: robot.mouseMove(p.x, p.y + 1);
>> 139: }
>
> Suggestion:
>
> for (int i = 0; i < 30; i++) {
> robot.mouseMove(loc.x, loc.y + 1);
> }
>
> You may copy `loc` into a non-volatile local variable (micro-optimisation).
Sure, thanks for the suggestion!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26636#discussion_r2306247014
PR Review Comment: https://git.openjdk.org/jdk/pull/26636#discussion_r2306243803
PR Review Comment: https://git.openjdk.org/jdk/pull/26636#discussion_r2306246739
PR Review Comment: https://git.openjdk.org/jdk/pull/26636#discussion_r2306244789
