RFR: 8361379: [macos] Refactor accessibility code to retrieve attribute by name [v3]

Alexander Zuev kizune at openjdk.org
Wed Jul 9 05:01:15 UTC 2025


On Tue, 8 Jul 2025 21:33:00 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Alexander Zuev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Remove explicit jresult declaration
>
> modules/javafx.graphics/src/main/native-glass/mac/a11y/AccessibleBase.m line 113:
> 
>> 111:     GET_MAIN_JENV;
>> 112:     if (env == NULL) return NULL;
>> 113:     jresult = (jobject)(*env)->CallLongMethod(env, [self getJAccessible],
> 
> not an expert, but do we really need to initialize `jresult` to NULL in line 110?
> can the variable be declared in line 113 ?

Well this pattern was used to initialize the result value to the default return value so if assignment does not happen due to the Java exception we have something meaningful to return but in case of NULL it is not really required because it will produce the same result anyways.

> 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.

-------------

PR Review Comment: https://git.openjdk.org/jfx/pull/1840#discussion_r2194014740
PR Review Comment: https://git.openjdk.org/jfx/pull/1840#discussion_r2194018393


More information about the openjfx-dev mailing list