On Tue, 16 May 2023 05:04:51 GMT, Ravi Gupta <d...@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
