JDK-8311893: Interactive component with ARIA role 'tabpanel' does not have a programmatically associated name

[psoujany]

Interactive component with ARIA role 'tabpanel' does not have a programmatically associated name in javadoc which is failing Accessibility checks.
https://bugs.openjdk.org/browse/JDK-8311893
https://bugs.openjdk.org/browse/JDK-8311894

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8311893: Interactive component with ARIA role 'tabpanel' does not have a programmatically associated name (Enhancement - P4)

Reviewers

• Hannes Wallnöfer (@hns - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/16148/head:pull/16148
$ git checkout pull/16148

Update a local copy of the PR:
$ git checkout pull/16148
$ git pull https://git.openjdk.org/jdk.git pull/16148/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 16148

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/16148.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back psoujany! 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]

@psoujany The following label will be automatically applied to this pull request:

• javadoc

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.

[mlbridge]

Webrevs

• 11: Full - Incremental (e530c7ca)
• 10: Full - Incremental (5b7327a0)
• 09: Full - Incremental (2d72e162)
• 08: Full - Incremental (033c2242)
• 07: Full - Incremental (dca9d95a)
• 06: Full - Incremental (50b36258)
• 05: Full - Incremental (f51cbb85)
• 04: Full - Incremental (0b6876ac)
• 03: Full - Incremental (0bac8588)
• 02: Full - Incremental (7bd95ee6)
• 01: Full - Incremental (3be80dcd)
• 00: Full (c864342a)

[psoujany]

@RealCLanger Could you please review my changes. Thank you.

[RealCLanger]

@RealCLanger Could you please review my changes. Thank you.

Sorry, that's not my turf. Somebody from javadoc area should pick this up. However, could you please enable Github Actions testing for your repo? Thanks.

[psoujany]

@jonathan-gibbons Could you please review my changes. Thank you.

[bridgekeeper]

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

[psoujany]

Can anyone review this PR.

[bridgekeeper]

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

[psoujany]

Can anyone review this PR. Thanks.

[jonathan-gibbons]

@hns ?

[jonathan-gibbons]

Thank you for your contribution.

As a general rule, whenever a change like this is made, we provide or update a regression test, to verify the fix works as expected, and to ensure that it stays fixed. If there are reasons why it is not necessary to write a test, you should put a suitable noreg-* label on the JBS issue. You can find the set of acceptable noreg-* labels in the JBS Label Dictionary in the OpenJDK Developers Guide.

[psoujany]

Thank you @jonathan-gibbons. I'll check on the regression tests.

[hns]

I do agree that the aria-labelledby attribute should be moved to the <div role="tab-panel"> element. However, it is certainly not correct to set the attribute to the tab-panel's own id as it does not provide a human-readable label. I think the current value for the attribute is correct, but it needs to be updated when a different tab is selected.

Looking at this example as a blueprint, I think the solution should be the following:

• Add an aria-label attribute to the <div role="tablist"> element explaining its purpose, e.g. "Tabs for selecting table contents".
• Move the aria-labelledby attribute from the embedded <div> to the <div role="tab-panel"> as proposed, but maintain its current default value (the id of the default tab).
• Add code in script.js to update the value of the aria-labelledby attribute when a different tab is selected.

Note: I have up updated this comment as I initially misunderstood the diff and how it related to generated HTML.

[hns]

There are quite a few tests failing because of this change, which need to be updated once we agree on the proper solution.

[psoujany]

@hns Thanks for your review. Will look into the changes.

[hns]

@psoujany some of the additional changes such as updating script.js and the failing tests are probably quite involved and easier to do by someone familiar with the code, so I could contribute these changes if that's ok with you.

[psoujany]

@hns As this PR addresses below comment. I want this PR to be contributed with the below proposal.
Move the aria-labelledby attribute from the embedded <div> to the <div role="tab-panel"> as proposed, but maintain its current default value (the id of the default tab).

Others you can handle. Please let me know if this is fine with you. I'll update code and testcases involved with this change as well. Thank you.

[hns]

@psoujany just to make sure we're on the same page, you'll also update the tests required by this change?

[psoujany]

Hi @hns , yes I'm trying to run jtreg testcases with this code changes. I would like to fix the testcase as well, I'll reach out to you for help in case of any queries. Thank you.

[psoujany]

@hns I've modified regression tests with the code changes and see langtools jtreg tests passing with the new fixes. Could you please review this PR. Thank you.

[hns]

@psoujany I still get two failing tests, possibly because I updated my local repository by merging the current master branch before running tests. This PR is based on a very outdated branch, so we should definitely merge the master branch before integrating. The failing tests I get on my up-to-date branch are these:

jdk/javadoc/doclet/testPreview/TestPreview.java
jdk/javadoc/doclet/testSpecTag/TestSpecTag.java

[psoujany]

@hns I've updated my branch and modified the failing tests. Now all tests seems passing with the changes. Thank you.

[hns]

Thanks @psoujany, this looks good now. Although script.js only requires a very small change to set the aria-labelledby attribute in the new element, I unfortunately detected an additional bug where the attribute is set to the wrong value when switching back to the default tab. I'm currently deciding whether to fix this in this PR or file a new issue.

[hns]

@psoujany the last thing that needs to be updated is to change script.js to update the aria-labelledby attribute in the correct element as I did in this pull request. Feel free to make the change yourself if there are problems with the pull request (github complains that I haven't signed the Eclipse Contributors Agreement, but that shouldn't be an issue for this OpenJDK change).

[psoujany]

@hns Thank you for the help. I've updated your suggested change to script.js.

[hns]

Thanks for fixing the tests, @psoujany! This looks good now.

[openjdk]

@psoujany This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8311893: Interactive component with ARIA role 'tabpanel' does not have a programmatically associated name

Reviewed-by: hannesw

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 31 new commits pushed to the master branch:

• d3c3194: 6285888: ChoiceFormat can support unescaped relational symbols in the Format segment
• 144a08e: 8325078: Better escaping of single and double quotes in javac annotation toString() results
• b3ecd55: 8324679: Replace NULL with nullptr in HotSpot .ad files
• 192349e: 8324066: "clhsdb jstack" should not by default scan for j.u.c locks because it can be very slow
• 6b09a79: 8324974: JFR: EventCompilerPhase should be created as UNTIMED
• 70e7cdc: 8323670: A few client tests intermittently throw ConcurrentModificationException
• ac1cd31: 8325096: Test java/security/cert/CertPathBuilder/akiExt/AKISerialNumber.java is failing
• 8e45182: 8324834: Use _LARGE_FILES on AIX
• cab74b0: 8324287: Record total and free swap space in JFR
• 6b84f9b: 8325001: Typo in the javadocs for the Arena::ofShared method
• ... and 21 more: https://git.openjdk.org/jdk/compare/577de17d24e83c55ab10a5794f381243a298fc68...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@hns) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[psoujany]

/integrate

[openjdk]

@psoujany
Your change (at version e530c7c) is now ready to be sponsored by a Committer.

[psoujany]

@hns Could you please sponsor this PR. Thank you.

[hns]

/sponsor

[openjdk]

Going to push as commit 783ae56.
Since your change was applied there have been 31 commits pushed to the master branch:

• d3c3194: 6285888: ChoiceFormat can support unescaped relational symbols in the Format segment
• 144a08e: 8325078: Better escaping of single and double quotes in javac annotation toString() results
• b3ecd55: 8324679: Replace NULL with nullptr in HotSpot .ad files
• 192349e: 8324066: "clhsdb jstack" should not by default scan for j.u.c locks because it can be very slow
• 6b09a79: 8324974: JFR: EventCompilerPhase should be created as UNTIMED
• 70e7cdc: 8323670: A few client tests intermittently throw ConcurrentModificationException
• ac1cd31: 8325096: Test java/security/cert/CertPathBuilder/akiExt/AKISerialNumber.java is failing
• 8e45182: 8324834: Use _LARGE_FILES on AIX
• cab74b0: 8324287: Record total and free swap space in JFR
• 6b84f9b: 8325001: Typo in the javadocs for the Arena::ofShared method
• ... and 21 more: https://git.openjdk.org/jdk/compare/577de17d24e83c55ab10a5794f381243a298fc68...master

Your commit was automatically rebased without conflicts.

[openjdk]

@hns @psoujany Pushed as commit 783ae56.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.