On Tue, 4 Feb 2025 13:24:29 GMT, Artem Semenov <aseme...@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. > > 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)) { > > At first glance, these conditions look like the same thing. equals() above > will return false if oldValue is null. oldValue.equals(newValue) and > newValue.equals(oldValue) are also the same thing. Try to rewrite this > condition more clearly. When CheckBox state is `checked`, `newValue is Checked` and `oldValue is null` and the existing condition `if (newValue != null && !newValue.equals(oldValue))` works well. But when CheckBox is `unchecked`, `newValue is null` and `oldValue is checked`, the existing condition returns `false` when `newValue != null` is evaluated. oldValue.equals(newValue) and newValue.equals(oldValue) looks same but when newValue is null, newValue.equals(oldValue) throws NPE. So, I think these conditions are not exactly same. We need to evaluate the `oldValue!=null` when CheckBox is unchecked for the correct behaviour. @aivanov-jdk suggested simpler way for comparing oldValue and newValue [here](https://github.com/openjdk/jdk/pull/23436#discussion_r1941484674). I will check and update. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/23436#discussion_r1942299553
