On Wed, 9 Jul 2025 04:15:56 GMT, Alexander Zuev <kiz...@openjdk.org> wrote:
>> modules/javafx.graphics/src/main/native-glass/mac/a11y/AccessibleBase.m line
>> 50:
>>
>>> 48: [rolesMap setObject:@"JFXButtonAccessibility"
>>> forKey:@"SPLIT_MENU_BUTTON"];
>>> 49: [rolesMap setObject:@"JFXRadiobuttonAccessibility"
>>> forKey:@"RADIO_BUTTON"];
>>> 50: [rolesMap setObject:@"JFXRadiobuttonAccessibility"
>>> forKey:@"TAB_ITEM"];
>>
>> this change seems unrelated - is it a part of some other work?
>
> When i implemented radio buttons one of the roles this implementation was
> supposed to fulfill was tab button for the tabbed pane. But without the new
> implementation of Tab Group it did not work - focus was funky and
> announcement was not correct. So i commented out this role assignment. When i
> pushed Tab Group implementation i haven't uncommented this line so now since
> i do clean up and refactoring on this exact file i tested that now it works
> properly and uncommented it. Just trying not to spawn too many pull requests
> for the minimal changes.
does it mean we missed something during the testing, something was supposed to
work and did not?
since there will be more a-y work, I think it should be ok.
>> modules/javafx.graphics/src/main/native-glass/mac/a11y/AccessibleBase.m line
>> 191:
>>
>>> 189: id p = [self requestNodeAttribute:@"AXPosition"];
>>> 190: id s = [self requestNodeAttribute:@"AXSize"];
>>> 191: if (p == NULL || s == NULL) {
>>
>> not a big issue, but would it be (marginally) better to bail out early if p
>> == NULL ?
>
> But that would require two conditions - one for position and one for size -
> with the same logic which would look not that compact and pretty.
but it will save a couple of nanoseconds!
the code is ok. one question though: what are the circumstances when these
calls would return NULL?
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1840#discussion_r2195249304
PR Review Comment: https://git.openjdk.org/jfx/pull/1840#discussion_r2195244238
