RFR: 8307325: Verify the focus owner when non focused windows requesting focus [v2]

Alexey Ivanov aivanov at openjdk.org
Wed Jun 28 19:33:00 UTC 2023


On Tue, 16 May 2023 05:04:51 GMT, Ravi Gupta <duke at openjdk.org> wrote:

>> Write a test Check, when the top level Window is not the focused Window requesting for focus and becoming the Focus Owner for any Component in that Window checking the following
>> 
>> 1.The Component is the Focus Owner and the Window becomes the focused Window If the platform supports cross requesting focus
>> across Windows.
>> 2.The request is remembered and  be granted when the Window is later focused If the platform does not support requesting focus
>> across Windows.
>> 
>> Testing:
>> Tested using Mach5(20 times per platform) in macos,linux and windows and got all pass.
>
> Ravi Gupta has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8307325: Review comments fixed

Changes requested by aivanov (Reviewer).

test/jdk/java/awt/Focus/CrossFocusRequestTest/CrossFocusRequestTest.java line 50:

> 48: 
> 49:     private static Frame frame1, frame2;
> 50:     private volatile static Button button;

Please use the [blessed modifier order](https://docs.oracle.com/javase/8/docs/api/java/lang/reflect/Modifier.html#toString-int-): `private static volatile`.

In fact, neither `button` nor `textField` need to be `volatile`.

test/jdk/java/awt/Focus/CrossFocusRequestTest/CrossFocusRequestTest.java line 95:

> 93:                 compAt.y + compSize.height - 20);
> 94:             robot.mousePress(InputEvent.BUTTON1_DOWN_MASK);
> 95:             robot.mouseRelease(InputEvent.BUTTON1_DOWN_MASK);

How does it test that the `requestFocus` works across frames?!

You click the text field in the first frame, which grants the focus to the text field irrespective whether `requestFocus` was called or not.

It does not make sense to me.

test/jdk/java/awt/Focus/CrossFocusRequestTest/CrossFocusRequestTest.java line 141:

> 139:     }
> 140: 
> 141:     public static void disposeFrame() {

Suggestion:

    private static void disposeFrames() {

`initializeGUI` is private, why is `disposeFrame` public?

Since you have two frames, using plural is better.

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

PR Review: https://git.openjdk.org/jdk/pull/13790#pullrequestreview-1503761835
PR Review Comment: https://git.openjdk.org/jdk/pull/13790#discussion_r1245628552
PR Review Comment: https://git.openjdk.org/jdk/pull/13790#discussion_r1245666425
PR Review Comment: https://git.openjdk.org/jdk/pull/13790#discussion_r1245672255



More information about the client-libs-dev mailing list