-
Notifications
You must be signed in to change notification settings - Fork 143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
8074883: Tab key should move to focused button in a button group #212
base: master
Are you sure you want to change the base?
Conversation
👋 Welcome back ktakakuri! A progress list of the required criteria for merging this PR into |
This backport pull request has now been updated with issue from the original commit. |
Could someone please review this backport? |
Looking into this fiix |
It looks like this change caused a regression fixed by the JDK-8182577(probably some ofthe issues were fixed as well?), please chech that it is possible to backport it as well. |
DefaultButtonModelCrashTest passed even before this backport was applied. |
One of the comment in the JDK-8182577 mentioned that that test works fine in jdk8 and failed since jdk9. So that was a regression, possibly caused by the change you requested to backport or maybe some other. |
DefaultButtonModelCrashTest passed even after applying this backport. |
@ktakakuri 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! |
@mrserb |
It needs to check why the same patch caused regression in jdk9, something is missing(not backported) in jdk8? If some other patch is missing we should try to backport it first, then backport this one, and then backport the JDK-8182577. |
I issued a PR for JDK-8154043 and JDK-8182577. |
@ktakakuri 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! |
@ktakakuri 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! |
@ktakakuri 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 |
/open |
@ktakakuri This pull request is now open |
@ktakakuri 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! |
/open |
@ktakakuri This pull request is now open |
@ktakakuri 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! |
@ktakakuri 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 |
/open |
@ktakakuri This pull request is now open |
@ktakakuri 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! |
@ktakakuri 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 |
/open |
@ktakakuri This pull request is now open |
❗ This change is not yet ready to be integrated. |
@ktakakuri 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! |
@ktakakuri 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 |
/open |
@ktakakuri This pull request is now open |
@ktakakuri 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! |
@ktakakuri 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 |
/open |
@ktakakuri This pull request is now open |
@ktakakuri 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! |
This pull request is pending review of #285. |
This is a backport of JDK-8074883: Tab key should move to focused button in a button group.
I would like to backport the patch to OpenJDK8u.
Original patch does not apply cleanly to 8u, because the fix uses a new API published in JDK9.
Since RequestFocusController only determines whether or not to set focus, I modified it so that requestFocus/requestFocusInWindow is called by SwingUtilities.invokeLater() and re-set focus and return false.
Without invokeLater(), the focus returns to the first button in case of Cause.ACTIVATION.
ToggleButton.getGroupSelection() is defined as a package private method, because it must be called by JCompoennt.focusController.
Calling requestFocus()/requestFocusInWindow() will be processed as Cause.UNKNOWN.
ToggleButton.getGroupSelection() returns itself, so no circular call occurs.
I moved Component.requestFocusController.acceptRequestFocus because RequestFocusContoroller is not called when Cause.ACTIVATION.
Only Swing components replace default RequestFocusController to JComponent.focusController.
The focusController returns true except for Swing, so this change does not affect other components.
Testing:
build on Windows x86_64
java/awt, javax/swing and ButtonGroupFocusTest.java on Windows x86_64
Progress
Issue
Reviewing
Using
git
Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u-dev.git pull/212/head:pull/212
$ git checkout pull/212
Update a local copy of the PR:
$ git checkout pull/212
$ git pull https://git.openjdk.org/jdk8u-dev.git pull/212/head
Using Skara CLI tools
Checkout this PR locally:
$ git pr checkout 212
View PR using the GUI difftool:
$ git pr show -t 212
Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk8u-dev/pull/212.diff
Webrev
Link to Webrev Comment