8339728 : [Accessibility,Windows,JAWS] Bug in the getKeyChar method of the AccessBridge class #22822

kumarabhi006 · 2024-12-19T05:17:07Z

For a JMenuItem with a shortcut like Ctrl + Comma, the getKeyChar method of the AccessBridge class cuts the Comma text to the first character and hence transfers C (instead of Comma) via the AccessBridge API. For a shortcut Ctrl + Comma in a menu item, screen readers announce Ctrl + C instead of Ctrl + Comma as shortcut. Same issue exists for Enter, Period and other keys as well where only the first character is returned from the getKeyChar method.

Proposed fix is to ensure that the char representation of the shortcuts is returned by the getKeyChar method and it worked fine except for "Enter" key. It is unclear to me why it has not announced by AT (JAWS) but I think that may be due to the char representation of the Enter key which is a Line Feed.

AT was also not able to announce the Tab and Space key as a shortcut. Adding these keys in the control key list in AccessBridge and in supported control code list in AccessBridgePackages files enabled them to be announced by AT.

Manual test case is added to verify the shortcut for JMenuItems.

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-8339728: [Accessibility,Windows,JAWS] Bug in the getKeyChar method of the AccessBridge class (Bug - P3)

Reviewers

Alexander Zuev (@azuev-java - Reviewer) Review applies to b6169d3d
Alexey Ivanov (@aivanov-jdk - Reviewer)
Prasanta Sadhukhan (@prsadhuk - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 22822

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

Using diff file

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

Using Webrev

Link to Webrev Comment

bridgekeeper · 2024-12-19T05:17:49Z

👋 Welcome back abhiscxk! 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 · 2024-12-19T05:18:37Z

@kumarabhi006 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:

8339728: [Accessibility,Windows,JAWS] Bug in the getKeyChar method of the AccessBridge class

Reviewed-by: aivanov, psadhukhan, kizune

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 8 new commits pushed to the master branch:

12700cb: 8346264: "Total compile time" counter should include time spent in failing/bailout compiles
dd81f8d: 8344079: Minor fixes and cleanups to compiler lint-related code
ccf3d57: 8346985: Convert test/jdk/com/sun/jdi/ClassUnloadEventTest.java to Class-File API
594e519: 8346984: Remove ASM-based benchmarks from Class-File API benchmarks
c027f2e: 8346983: Remove ASM-based transforms from Class-File API tests
e0695e0: 8346981: Remove obsolete java.base exports of jdk.internal.objectweb.asm packages
dfaa891: 8346569: Shenandoah: Worker initializes ShenandoahThreadLocalData twice results in memory leak
f1d85ab: 8346773: Fix unmatched brackets in some misc files

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
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.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

openjdk · 2024-12-19T05:19:18Z

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

client

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 · 2024-12-19T05:22:16Z

Webrevs

kumarabhi006 · 2024-12-19T05:25:00Z

@azuev-java Please review.

azuev-java · 2024-12-24T19:43:14Z

test/jdk/javax/accessibility/TestJMenuItemShortcutAccessibility.java

+ * @bug 8339728
+ * @summary Tests that JAWS announce the shortcuts for JMenuItems.
+ * @library /java/awt/regtesthelpers
+ * @build PassFailJFrame


Since instructions mention JAWS i would either make this test Windows specific or if you want to test t on mac too i would rephrase the instructions so they are not OS specific. Either way will work for me.

azuev-java · 2024-12-24T19:51:12Z

src/jdk.accessibility/windows/classes/com/sun/java/accessibility/internal/AccessBridge.java

+
+        keyCode = keyStroke.getKeyCode();
+        debugString("[INFO]:   Shortcut is: " + Integer.toHexString(keyCode));
+        if (keyCode != 0) {


Why do we need this check? If keyCode is not zero we return keyCode but if it is zero we will still return keyCode because we return zero after the condition.

Yeah, the condition check is not needed. Updated.

azuev-java

This version looks good. Manually tested with both JAWS and NVDA haven't found no functional regressions.

victordyakov · 2025-01-02T20:41:26Z

@prsadhuk please review

aivanov-jdk

Looks good to me except for minor comments.

You may want to bump up the copyright year to 2025 now.

aivanov-jdk · 2025-01-03T19:26:57Z

src/jdk.accessibility/windows/classes/com/sun/java/accessibility/internal/AccessBridge.java

+
+        keyCode = keyStroke.getKeyCode();
+        debugString("[INFO]:   Shortcut is: " + Integer.toHexString(keyCode));
+        return (char) keyCode;


Suggested change

return (char) keyCode;

return (char)keyCode;

For consistency with the return statements above which have no space after the cast.

aivanov-jdk · 2025-01-03T19:29:54Z

test/jdk/javax/accessibility/TestJMenuItemShortcutAccessibility.java

+        menu.add(menuItem6);
+        menu.add(menuItem7);
+
+        frame.setDefaultCloseOperation(JFrame.EXIT_ON_CLOSE);


You should not define a default close operation for a manual test, the PassFailJFrame framework handles closing the test window and fails the test.

aivanov-jdk · 2025-01-03T19:31:33Z

test/jdk/javax/accessibility/TestJMenuItemShortcutAccessibility.java

+        JFrame frame = new JFrame("A Frame with Menu");
+
+        JMenuBar menuBar = new JMenuBar();
+        JMenu menu = new JMenu("Menu with Keystrokes");


Suggested change

JMenu menu = new JMenu("Menu with Keystrokes");

JMenu menu = new JMenu("Menu with shortcuts");

You refer to concept as “shortcuts” in the instructions.

aivanov-jdk · 2025-01-03T19:32:59Z

test/jdk/javax/accessibility/TestJMenuItemShortcutAccessibility.java

+                2. Press Alt + M to open application Menu
+                3. Navigate the Menu Items by using UP / DOWN arrow key
+                4. Press Pass if you are able to hear correct JAWS announcements
+                   for each menu item shorcut else Fail.


Suggested change

for each menu item shorcut else Fail.

for each menu item shortcut else Fail.

Typo.

You should probably remove the period… to be consistent. Either add ending punctuation to each instruction item, nor don't add it at all, including the last item.

prsadhuk · 2025-01-06T06:15:37Z

src/jdk.accessibility/windows/native/include/bridge/AccessBridgePackages.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2005, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved.


this copyright also needs to be updated..

prsadhuk · 2025-01-06T06:17:24Z

test/jdk/javax/accessibility/TestJMenuItemShortcutAccessibility.java

+public class TestJMenuItemShortcutAccessibility {
+    public static void main(String[] args) throws Exception {
+        String INSTRUCTIONS = """
+                1. Start the JAWS aplication


"application"

prsadhuk · 2025-01-06T06:26:07Z

test/jdk/javax/accessibility/TestJMenuItemShortcutAccessibility.java

+                2. Press Alt + M to open application Menu
+                3. Navigate the Menu Items by using UP / DOWN arrow key
+                4. Press Pass if you are able to hear correct JAWS announcements
+                   for each menu item shortcut else Fail


should we need not expand what is "correct" JAWS announcements? like mentioning it should read aloud "full" shortcut text and not only the 1st character of shortcut

Updated test instruction.

aivanov-jdk · 2025-01-06T17:54:13Z

I ran the test with Java 21 and with the fix, and I confirm JAWS announces the shortcuts correctly now.

…it worked fine except for Enter key…

Should we submit a bug? It's a known problem now, but there's no fix for it. It could be a limitation that may be worth calling out in a release note.

kumarabhi006 · 2025-01-07T08:34:56Z

…it worked fine except for Enter key…

Should we submit a bug? It's a known problem now, but there's no fix for it. It could be a limitation that may be worth calling out in a release note.

Yeah, as of now I don't have a fix for "Enter" key shortcut or it may be a limitation.
I am not sure if filing a bug is a good idea as it may just increase the backlog with no potential solution.

kumarabhi006 · 2025-01-09T04:11:32Z

/integrate

openjdk · 2025-01-09T04:11:54Z

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

97dd06c: 8347299: Add annotations to test cases in LicenseTest
0a35ebf: 8347297: Skip the RuntimeImageSymbolicLinksTest test on Windows when it is executed outside of the jtreg
b3e8736: 8347296: WinInstallerUiTest fails in local test runs if the path to test work directory is longer that regular
1ade96b: 8347042: Remove an extra parenthesis in macro definition in jfrTraceIdMacros.hpp
db7fa6a: 8347289: HKDF delayed provider selection failed with non-extractable PRK
0a5ef82: 8347162: Update problemlist CR for vmTestbase/nsk/jdi/VMOutOfMemoryException
ea49537: 8310340: assert(_thread->is_interp_only_mode() || stub_caller) failed: expected a stub-caller
2e00816: 8346671: java/nio/file/Files/probeContentType/Basic.java fails on Windows 2025
88fa3b2: 8346998: Test nsk/jvmti/ResourceExhausted/resexhausted003 fails with java.lang.OutOfMemoryError when CDS is off
6ee2bd2: 8347147: [REDO] AccessFlags can be u2 in metadata
... and 48 more: https://git.openjdk.org/jdk/compare/9393897817a3280e08a8278b86df71f59608be9c...master

Your commit was automatically rebased without conflicts.

openjdk · 2025-01-09T04:12:03Z

@kumarabhi006 Pushed as commit a46ae70.

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

kumarabhi006 · 2025-01-10T05:25:33Z

/backport :jdk24

openjdk · 2025-01-10T05:26:26Z

@kumarabhi006 Could not automatically backport a46ae703 to openjdk/jdk due to conflicts in the following files:

src/jdk.accessibility/windows/classes/com/sun/java/accessibility/internal/AccessBridge.java

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jdk. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jdk.git jdk24:jdk24

# Check out the target branch and create your own branch to backport
$ git checkout jdk24
$ git checkout -b backport-kumarabhi006-a46ae703-jdk24

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk.git a46ae7031e30eb4940e43012a42f1b7fa5d942ef

# Backport the commit
$ git cherry-pick --no-commit a46ae7031e30eb4940e43012a42f1b7fa5d942ef
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport a46ae7031e30eb4940e43012a42f1b7fa5d942ef'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk with the title Backport a46ae7031e30eb4940e43012a42f1b7fa5d942ef.

Below you can find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit a46ae703 from the openjdk/jdk repository.

The commit being backported was authored by Abhishek Kumar on 9 Jan 2025 and was reviewed by Alexey Ivanov, Prasanta Sadhukhan and Alexander Zuev.

Thanks!

kumarabhi006 · 2025-01-10T06:23:58Z

/backport :jdk24

openjdk · 2025-01-10T06:24:58Z

@kumarabhi006 Could not automatically backport a46ae703 to openjdk/jdk due to conflicts in the following files:

src/jdk.accessibility/windows/classes/com/sun/java/accessibility/internal/AccessBridge.java

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jdk. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jdk.git jdk24:jdk24

# Check out the target branch and create your own branch to backport
$ git checkout jdk24
$ git checkout -b backport-kumarabhi006-a46ae703-jdk24

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk.git a46ae7031e30eb4940e43012a42f1b7fa5d942ef

# Backport the commit
$ git cherry-pick --no-commit a46ae7031e30eb4940e43012a42f1b7fa5d942ef
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport a46ae7031e30eb4940e43012a42f1b7fa5d942ef'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk with the title Backport a46ae7031e30eb4940e43012a42f1b7fa5d942ef.

Below you can find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit a46ae703 from the openjdk/jdk repository.

The commit being backported was authored by Abhishek Kumar on 9 Jan 2025 and was reviewed by Alexey Ivanov, Prasanta Sadhukhan and Alexander Zuev.

Thanks!

kumarabhi006 · 2025-01-10T06:47:11Z

/backport jdk:jdk24

openjdk · 2025-01-10T06:47:54Z

@kumarabhi006 Could not automatically backport a46ae703 to openjdk/jdk due to conflicts in the following files:

src/jdk.accessibility/windows/classes/com/sun/java/accessibility/internal/AccessBridge.java

Please fetch the appropriate branch/commit and manually resolve these conflicts by using the following commands in your personal fork of openjdk/jdk. Note: these commands are just some suggestions and you can use other equivalent commands you know.

# Fetch the up-to-date version of the target branch
$ git fetch --no-tags https://git.openjdk.org/jdk.git jdk24:jdk24

# Check out the target branch and create your own branch to backport
$ git checkout jdk24
$ git checkout -b backport-kumarabhi006-a46ae703-jdk24

# Fetch the commit you want to backport
$ git fetch --no-tags https://git.openjdk.org/jdk.git a46ae7031e30eb4940e43012a42f1b7fa5d942ef

# Backport the commit
$ git cherry-pick --no-commit a46ae7031e30eb4940e43012a42f1b7fa5d942ef
# Resolve conflicts now

# Commit the files you have modified
$ git add files/with/resolved/conflicts
$ git commit -m 'Backport a46ae7031e30eb4940e43012a42f1b7fa5d942ef'

Once you have resolved the conflicts as explained above continue with creating a pull request towards the openjdk/jdk with the title Backport a46ae7031e30eb4940e43012a42f1b7fa5d942ef.

Below you can find a suggestion for the pull request body:

Hi all,

This pull request contains a backport of commit a46ae703 from the openjdk/jdk repository.

The commit being backported was authored by Abhishek Kumar on 9 Jan 2025 and was reviewed by Alexey Ivanov, Prasanta Sadhukhan and Alexander Zuev.

Thanks!

MenuItem shortcut accessibility fix

6ab3dbd

openjdk bot added the rfr Pull request is ready for review label Dec 19, 2024

openjdk bot added the client client-libs-dev@openjdk.org label Dec 19, 2024

Copyright year and test update

a83aa98

azuev-java reviewed Dec 24, 2024

View reviewed changes

Review comment update

b6169d3

azuev-java approved these changes Jan 2, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 2, 2025

aivanov-jdk approved these changes Jan 3, 2025

View reviewed changes

Copyright year and minor fix

7e5b00d

openjdk bot removed the ready Pull request is ready to be integrated label Jan 6, 2025

Merge branch 'master' into JDK-8339728

64ecaf0

prsadhuk reviewed Jan 6, 2025

View reviewed changes

Instruction and missed copyright year update

705fb4c

prsadhuk approved these changes Jan 6, 2025

View reviewed changes

openjdk bot added the ready Pull request is ready to be integrated label Jan 6, 2025

aivanov-jdk approved these changes Jan 6, 2025

View reviewed changes

openjdk bot added the integrated Pull request has been integrated label Jan 9, 2025

openjdk bot closed this Jan 9, 2025

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Jan 9, 2025

	JMenu menu = new JMenu("Menu with Keystrokes");
	JMenu menu = new JMenu("Menu with shortcuts");

	for each menu item shorcut else Fail.
	for each menu item shortcut else Fail.

8339728 : [Accessibility,Windows,JAWS] Bug in the getKeyChar method of the AccessBridge class #22822

8339728 : [Accessibility,Windows,JAWS] Bug in the getKeyChar method of the AccessBridge class #22822

Uh oh!

Conversation

kumarabhi006 commented Dec 19, 2024 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper bot commented Dec 19, 2024

Uh oh!

openjdk bot commented Dec 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openjdk bot commented Dec 19, 2024

Uh oh!

mlbridge bot commented Dec 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

kumarabhi006 commented Dec 19, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

azuev-java left a comment

Choose a reason for hiding this comment

Uh oh!

victordyakov commented Jan 2, 2025

Uh oh!

aivanov-jdk left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aivanov-jdk commented Jan 6, 2025

Uh oh!

kumarabhi006 commented Jan 7, 2025

Uh oh!

kumarabhi006 commented Jan 9, 2025

Uh oh!

openjdk bot commented Jan 9, 2025

Uh oh!

openjdk bot commented Jan 9, 2025

Uh oh!

kumarabhi006 commented Jan 10, 2025

Uh oh!

openjdk bot commented Jan 10, 2025

Uh oh!

kumarabhi006 commented Jan 10, 2025

Uh oh!

openjdk bot commented Jan 10, 2025

Uh oh!

kumarabhi006 commented Dec 19, 2024 •

edited by openjdk bot

Loading

openjdk bot commented Dec 19, 2024 •

edited

Loading

mlbridge bot commented Dec 19, 2024 •

edited

Loading