8154043: Fields not reachable anymore by tab-key, because of new tabbing behaviour of radio button groups.

[ktakakuri]

This is a backport of JDK-8154043: Fields not reachable anymore by tab-key, because of new tabbing behaviour of radio button groups.

Applying the JDK-8154043 fix as is will result in a regression of JDK-8182577.
The fix of JDK-8182577 adds an interface for JDK10, therefore this fix cannot be backported simply for JDK8u.
So, I propose to judge the buttonModel is an instance of DefaultButtonModel.

Testing:
java/awt
javax/swing
ButtonGroupLayoutTraversalTest.java
bug8033699.java
DefaultButtonModelCrashTest.java
on Windows x86_64

---

Progress

• JDK-8154043 needs maintainer approval
• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change must be properly reviewed (2 reviews required, with at least 2 Reviewers)
• JDK-8182577 needs maintainer approval

Issues

• JDK-8154043: Fields not reachable anymore by tab-key, because of new tabbing behaviour of radio button groups. (Bug - P3 - Rejected)
• JDK-8182577: Exception when Tab key moves focus to a JCheckbox with a custom ButtonModel (Bug - P3)

Reviewers

• Paul Hohensee (@phohensee - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk8u-dev.git pull/285/head:pull/285
$ git checkout pull/285

Update a local copy of the PR:
$ git checkout pull/285
$ git pull https://git.openjdk.org/jdk8u-dev.git pull/285/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 285

View PR using the GUI difftool:
$ git pr show -t 285

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk8u-dev/pull/285.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back ktakakuri! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

This backport pull request has now been updated with issue from the original commit.

[ktakakuri]

Could someone please review this backport?

[ktakakuri]

@mrserb
I issued this PR in relation to #212.
Could you please review this backport?

[bridgekeeper]

@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]

Could someone please review this backport?

[mlbridge]

Webrevs

• 02: Full - Incremental (a1eac9d6)
• 01: Full - Incremental (cc8ef7ee)
• 00: Full (6a8e5fe6)

[bridgekeeper]

@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!

[phohensee]

Copyright date updates should be taken from the original commit rather than use the current year. If the existing file has a later copyright date than the file in the original commit, leave the copyright date alone. Other than that, lgtm.

[mrserb]

Sorry I did not have a time to look at this PR, will try to look soon.

[ktakakuri]

I corrected the copyright date.

[openjdk]

@ktakakuri This change is no longer ready for integration - check the PR body for details.

[openjdk]

⚠️ @ktakakuri This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

[jerboaa]

As this fix includes the test from JDK-8182577, please use /isssue add JDK-8182577.

[bridgekeeper]

@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]

What is the status of this backport now?

[bridgekeeper]

@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!

[phohensee]

I filed a backport JBS issue JDK-8337619, and an associated backport CSR JDK-8337620. Follow the standard CSR process to get it reviewed and approved. It may not be approved, since it add a public method to an existing API, which afaik can't be done without an update JSR.

[bridgekeeper]

@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]

Is there anything I should do now?
Should I just wait for a review?

[phohensee]

You need a backport CSR approved for https://bugs.openjdk.org/browse/JDK-8182577. The backport CSR https://bugs.openjdk.org/browse/JDK-8337619 is linked from the backport JBS issue https://bugs.openjdk.org/browse/JDK-8337619.

[ktakakuri]

Unlike jdk10, this backport does not add any interfaces.
Is it still necessary for the CSR to be approved?

[theRealAph]

Unlike jdk10, this backport does not add any interfaces. Is it still necessary for the CSR to be approved?

I don't think so. But there are some complex issues with the jdk 8 backport described in the comments to https://bugs.openjdk.org/browse/JDK-8154043 , which I think is why the 8 bakport was denied in 2016. What I do not understand is why, when the backport was denied 8 years ago, it should be allowed today. If someone can explain that, we can have a meningful discussion.

[ktakakuri]

Why was the jdk8 backport denied in 2016?
I tried to find it through JBS and email but could not find it.
If adding an interface is the problem, this backport is fine.

[bridgekeeper]

@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]

@theRealAph If you have any information, I would appreciate it if you could share it with me.

[bridgekeeper]

@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]

I comment to not close.

[bridgekeeper]

@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]

I comment to not close.

[bridgekeeper]

@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]

I comment to not close.

[gnu-andrew]

The change to use DefaultButtonModel makes sense as this is the approach used in JDK-8074883 prior to being replaced in JDK-8182577

If we are including the relevant parts of JDK-8182577 in this change (the test case and the indentation fix to the member isEnabled if statement) then we should also include the correction to the indentation of group.getElements() and presumably the instanceof test?

There is also a closing brace with bad indentation in ButtonGroupLayoutTraversalTest.java (line 78). I'm not sure how that crept in.

[gnu-andrew]

Why was the jdk8 backport denied in 2016? I tried to find it through JBS and email but could not find it. If adding an interface is the problem, this backport is fine.

There is a comment on the bug from 2016 that says "No plans to fix for JDK 7 and 8", but it doesn't go into details as to whether this is due to risk or resources, so it's not clear if that is actually a denial or not. Given this was a fix made during the development of 9, the code it was developed against should be fairly close to 8u's and indeed, this backport only really diverges code-wise in that it also incorporates 8182577.

That comment also seems to have been superseded by the backport of this change (and then 8182577) to Oracle's 8u311 by Alexey Ivanov. I think it's also worth noting that this is a regression from 8 GA due to JDK-8033699 being introduced in 8u45. If that backport was suitable for an update release to break this, then presumably the fix should be too.

This brings us back to @theRealAph's question as to why to do this now if it has been broken for so long. What piqued your interest in this bug? Are you or others hitting it in real use? Or was it just a case of Oracle fixed it, so we should?

[gnu-andrew]

/reviewers 2 reviewer

Bumping to make sure the requested changes are made if we do decide to go ahead with this.

[openjdk]

@gnu-andrew
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 2 Reviewers).

[gnu-andrew]

/csr unneeded

The associated CSR JDK-8182695 is for adding the getModel() method in JDK-8182577 but this patch does not add that method (and should not). It is just bringing over the test case and some minor code fixes.

[openjdk]

@gnu-andrew determined that a CSR request is not needed for this pull request.

[bridgekeeper]

@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!

[bridgekeeper]

@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 pull request command.

[ktakakuri]

/open

[openjdk]

@ktakakuri This pull request is now open