RFR: 8301312: Create implementation of NSAccessibilityButton protocol [v4]
Kevin Rushforth
kcr at openjdk.org
Mon May 22 23:27:56 UTC 2023
On Wed, 17 May 2023 20:31:27 GMT, Alexander Zuev <kizune at openjdk.org> wrote:
>> Add the common base component for all the new implementing native classes Change native peer creation to use the new base component The new code will instantiate new protocol implementation for the given role if it exists or an old one if it does not exist
>> Added BUTTON role implementing class
>
> Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision:
>
> Properly use NSObject instead of the GlassAccessible type.
Initial testing looks good. I'll do more testing soon. I did get a one-time exception while playing around with Ensemble8, I can't reproduce it. It was in the existing code, so it might be unrelated to your change.
I left a couple inline questions.
I noticed that your branch is about 3 months out of date. Can you merge the latest master.
modules/javafx.graphics/src/main/native-glass/mac/a11y/AccessibleBase.m line 124:
> 122:
> 123: // Actions support
> 124: - (BOOL)performAccessibleAction:(jlong)actionId
Would it make sense to have this parameter be an `NSString` (like the similar method in `GlassAcessible`) and do the cast to `jlong` in this method rather than having all the callers do it? Of course, that only works if all of the calls will use NSString, so what you have is more flexible.
modules/javafx.graphics/src/main/native-glass/mac/a11y/AccessibleBase.m line 206:
> 204: NSObject* accessible = (NSObject*)jlong_to_ptr(macAccessible);
> 205: return ptr_to_jlong(NSAccessibilityUnignoredAncestor(accessible));
> 206: }
Why was this method moved from GlassAccessible.m to here? It seems fine, I was just curious.
-------------
PR Review: https://git.openjdk.org/jfx/pull/1084#pullrequestreview-1437812622
PR Review Comment: https://git.openjdk.org/jfx/pull/1084#discussion_r1201258731
PR Review Comment: https://git.openjdk.org/jfx/pull/1084#discussion_r1201254372
More information about the openjfx-dev
mailing list