On Fri, 29 Jan 2021 07:08:47 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:

>> Kevin Rushforth has updated the pull request incrementally with one 
>> additional commit since the last revision:
>> 
>>   Use XkbGetNamedIndicator on Linux instead of relying on the (unreliable) 
>> GDK method.
>
> modules/javafx.graphics/src/main/java/com/sun/javafx/tk/Toolkit.java line 878:
> 
>> 876: 
>> 877:     /**
>> 878:      * Return the lock state for the given keyCode.
> 
> Minor typo: Return -> Returns

fixed

> modules/javafx.graphics/src/main/native-glass/mac/GlassKey.m line 407:
> 
>> 405:   (JNIEnv * env, jobject obj, jint keyCode)
>> 406: {
>> 407:     NSUInteger modifierFlags = [NSEvent modifierFlags];
> 
> If this line is moved just before return statement(Line#418),  then we can 
> avoid executing this call in case of invalid keyCode.

fixed

> modules/javafx.graphics/src/main/native-glass/gtk/glass_key.cpp line 381:
> 
>> 379:     }
>> 380: 
>> 381:     Atom atom = None;
> 
> The documentation of 
> [XInternAtom()](https://www.x.org/releases/X11R7.5/doc/man/man3/XInternAtom.3.html)
>  does mention that this function returns an atom identifier associated with 
> the provided atom name. So the variable name 'atom' does sound good enough. 
> But would it be good to rename this variable to something like 'keyCodeAtom'. 
> I am Ok either way.

fixed

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

PR: https://git.openjdk.java.net/jfx/pull/385

Reply via email to