On Mon, 9 Feb 2026 23:05:51 GMT, Sergey Bylokhov <[email protected]> wrote:
>> Jeremy Wood has updated the pull request incrementally with two additional >> commits since the last revision: >> >> - 8377428: simplify test instructions >> >> Now that there are other interesting components in the UI: we don't need >> to press CTRL + ALT + UP. >> - 8377428: test condition where axComp is null >> >> If AccessibleComponent is null, when the new CAccessibility.isShowing(..) >> method falls back to consulting the AccessibleStateSet. >> >> This change makes sure we follow that code path, too. >> >> When I checked CAccessibility.isShowing() in the debugger, I observed: >> >> For "row 1": we had an AccessibleComponent where isShowing() is false >> >> For "row 2": we had a no AccessibleComponent. We checked the >> AccessibleStateSelection and did not see SHOWING, so >> CAccessibility.isShowing(context) returned TRUE. This is not accurate, but >> it's working as designed. We had incomplete information, and we're supposed >> to err on the side of returning true (to minimize invasive risk). Meanwhile: >> the test still passes, because then _addChildren recursively inspects the >> JButton, and CAccessibility.isShowing(buttonContext) returns false. >> >> For "row 3": we had an AccessibleComponent where isShowing() is true. >> >> For "row 4": we had no AccessibleComponent. We checked >> AccessibleStateSelection and DID see SHOWING, so we returned true. >> >> This is in response to: >> https://github.com/openjdk/jdk/pull/29630#discussion_r2784969123 > > src/java.desktop/macosx/classes/sun/lwawt/macosx/CAccessibility.java line > 1077: > >> 1075: } >> 1076: >> 1077: AccessibleStateSet ass = context.getAccessibleStateSet(); > > Could you please add a test to verify that these methods from > AccessibleContext are actually working? I doubt we currently have any tests > that cover this code path. I expanded the test to include 4 rows: two visible, and two with no AccessibleComponent. I confirmed in IntelliJ's debugger that each of the 4 predicted scenarios are addressed. I detailed what I observed here: https://github.com/openjdk/jdk/pull/29630/changes/1a6b53be73e6445d79216abe94a73922b2d17a66 I'm also fine with removing these new references to AccessibleStateSet in `isShowing(..)` entirely. It's intended to be a fallback. I have seen several examples of incomplete accessibility info, but I don't think I've seen a case where the AccessibleComponent is missing. (I have seen cases where the AccessibleStateSet contains incomplete data, though. That's why I only look for the presence of the SHOWING state, but I don't interpret the absence of the SHOWING state as a clear guarantee that we're not showing...) ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29630#discussion_r2785294585
