RFR: 8348936: [Accessibility,macOS,VoiceOver] VoiceOver doesn't announce untick on toggling the checkbox with "space" key on macOS

[Alexey Ivanov]

On Tue, 4 Feb 2025 10:58:20 GMT, Abhishek Kumar <abhiscxk at openjdk.org> wrote:

> VoiceOver doesn't announce the _untick state_ when Checkbox is `deselected` using **space** key. When CheckBox is deselected, the state change is not notified to a11y client (VoiceOver) and so the state is not announced by VO. 
> 
> Screen Magnifier is also unable to magnify the unchecked state of JCheckBox due to same reason and is captured as separate bug [JDK-8345728](https://bugs.openjdk.org/browse/JDK-8345728).
> 
> Proposed fix is to send the state change notification to a11y client when checkbox is deselected, this resolves the problem for VoiceOver and Screen Magnifier.
> 
> Similar issue observed for JToggleButton. So, I extended the fix for JToggleButton as well.
> 
> The proposed change can be verified the manual test in the PR.
> 
> CI pipeline testing is `OK`, link posted in JBS.

Changes requested by aivanov (Reviewer).

src/java.desktop/macosx/classes/sun/lwawt/macosx/CAccessible.java line 186:

> 184:                     if (thisRole == AccessibleRole.CHECK_BOX) {
> 185:                         if ((newValue != null && !newValue.equals(oldValue)) ||
> 186:                                 oldValue != null && !oldValue.equals(newValue)) {

Suggestion:

                        if (!Objects.equals(newValue, oldValue)) {

This handles all the cases, including `null` values.

src/java.desktop/macosx/classes/sun/lwawt/macosx/CAccessible.java line 212:

> 210:                     // Do send toggle button state changes to native side
> 211:                     if (thisRole == AccessibleRole.TOGGLE_BUTTON) {
> 212:                         if ((newValue != null && !newValue.equals(oldValue)) ||

I believe the new condition should be used for `RADIO_BUTTON` too:

https://github.com/openjdk/jdk/blob/b985347c2383a7a637ffa9a4a8687f7f7cde1369/src/java.desktop/macosx/classes/sun/lwawt/macosx/CAccessible.java#L197-L200

test/jdk/javax/accessibility/TestJCheckBoxToggleAccessibility.java line 34:

> 32: /*
> 33:  * @test
> 34:  * @bug 8348936

Suggestion:

 * @bug 8348936 8345728

Two bugs are fixed by one changeset, and your summary implies either issue is tested by this single test.

test/jdk/javax/accessibility/TestJCheckBoxToggleAccessibility.java line 58:

> 56:                 7. Press Tab to move focus to ToggleButton
> 57:                 8. Repeat steps 3 to 6 and listen the announcement
> 58:                 9. If announcements are incorrect, Press Fail

Suggestion:

                9. If announcements are incorrect, press Fail

test/jdk/javax/accessibility/TestJCheckBoxToggleAccessibility.java line 87:

> 85: 
> 86:                 Press Pass if you are able to hear correct VoiceOver announcements and
> 87:                 able to see the correct screen magnifier behaviour. """;

These long instructions may benefit from using HTML for formatting instructions.

-------------

PR Review: https://git.openjdk.org/jdk/pull/23436#pullrequestreview-2593380133
PR Review Comment: https://git.openjdk.org/jdk/pull/23436#discussion_r1941484674
PR Review Comment: https://git.openjdk.org/jdk/pull/23436#discussion_r1941487923
PR Review Comment: https://git.openjdk.org/jdk/pull/23436#discussion_r1941492200
PR Review Comment: https://git.openjdk.org/jdk/pull/23436#discussion_r1941493443
PR Review Comment: https://git.openjdk.org/jdk/pull/23436#discussion_r1941495810