8307325: Verify the focus owner when non focused windows requesting focus#13790
8307325: Verify the focus owner when non focused windows requesting focus#13790ravigupta30 wants to merge 2 commits intoopenjdk:masterfrom
Conversation
|
👋 Welcome back ravigupta30! A progress list of the required criteria for merging this PR into |
|
@ravigupta30 The following label will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command. |
ravigupta30
changed the title
Webrevs
|
|
Any volunteers for a review? |
test/jdk/java/awt/Focus/CrossFocusRequestTest/CrossFocusRequestTest.java
Outdated
Show resolved
Hide resolved
test/jdk/java/awt/Focus/CrossFocusRequestTest/CrossFocusRequestTest.java
Outdated
Show resolved
Hide resolved
test/jdk/java/awt/Focus/CrossFocusRequestTest/CrossFocusRequestTest.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| private static void initializeGUI() { | ||
| frame1 = new Frame("Test Frame1"); |
There was a problem hiding this comment.
Should these frames be named something specific to the test?
There was a problem hiding this comment.
Should these frames be named something specific to the test?
I agree, more specific caption could help diagnose which test left frames open.
|
Any volunteers for a review? |
| public class CrossFocusRequestTest { | ||
|
|
||
| private static Frame frame1, frame2; | ||
| private volatile static Button button; |
There was a problem hiding this comment.
Please use the blessed modifier order: private static volatile.
In fact, neither button nor textField need to be volatile.
| robot.mouseMove(compAt.x + compSize.width / 2, | ||
| compAt.y + compSize.height - 20); | ||
| robot.mousePress(InputEvent.BUTTON1_DOWN_MASK); | ||
| robot.mouseRelease(InputEvent.BUTTON1_DOWN_MASK); |
There was a problem hiding this comment.
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.
| } | ||
|
|
||
| private static void initializeGUI() { | ||
| frame1 = new Frame("Test Frame1"); |
There was a problem hiding this comment.
Should these frames be named something specific to the test?
I agree, more specific caption could help diagnose which test left frames open.
| frame2.setVisible(true); | ||
| } | ||
|
|
||
| public static void disposeFrame() { |
There was a problem hiding this comment.
| public static void disposeFrame() { | |
| private static void disposeFrames() { |
initializeGUI is private, why is disposeFrame public?
Since you have two frames, using plural is better.
|
@ravigupta30 This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration! |
|
@ravigupta30 This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the |
5 participants
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.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/13790/head:pull/13790$ git checkout pull/13790Update a local copy of the PR:
$ git checkout pull/13790$ git pull https://git.openjdk.org/jdk.git pull/13790/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 13790View PR using the GUI difftool:
$ git pr show -t 13790Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/13790.diff
Webrev
Link to Webrev Comment