On Wed, 17 May 2023 20:31:27 GMT, Alexander Zuev <kiz...@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
