Hello, Anton.

Thank you for the review. The updated version of the fix is here:
http://cr.openjdk.java.net/~pchelko/8024170/webrev.02/

I've followed your comments mostly. Additionally the LightweightContent is now 
retrieved by the SwingAccessor.

With best regards. Petr.

On Sep 3, 2013, at 7:53 PM, Anton V. Tarasov <[email protected]> wrote:

> Hi Petr,
> 
> It looks fine to me in general. Some minor comments:
> 
> * LightweightFramePeer.java
> 
> - Would be better to spell LightWeight as Lightweight.
> 
> - 41      * Sets the window peer under mouse to null if the current value us 
> equal to this peer
> 
> Please, fix the typo: us -> is
> 
> 
> * LightweightContent.java
> 
> - 187    public void invokeOnContentsThread(Runnable r);
> 
> The method name sounds a little bit odd...
> Could we use the term "client" mentioned in the javadoc? And rename it to 
> something like: invokeOnClientToolkitThread?
> 
> - Also, please provide a "default" impl for this method in order to avoid the 
> jdk/jfx versions mismatch until they get synchronized.
> 
> 
> * WLightweightFramePeer.java
> 
> - 54             lightweightFramePeerUnderMouse = this;
> 
> Looks like it should be set to null here.
> 
> 
> * JLightweightFrame.java
> 
> - 185     public void invokeOnContentsThread(Runnable r) {
> 
> Would you rather introduce a JLF.getContent() getter in order to avoid such 
> wrappers? But I'm not sure...
> 
> Thanks,
> Anton.
> 
> 
> On 9/3/13 6:27 PM, Petr Pchelko wrote:
>> Hello, AWT Team.
>> 
>> Please review the fix for the issue:
>> http://bugs.sun.com/view_bug.do?bug_id=8024170
>> The issue might not be available on bugs.sun.com yet, so here's the FX 
>> counterpart:
>> https://javafx-jira.kenai.com/browse/RT-31957
>> 
>> The JDK part of the fix is available at:
>> http://cr.openjdk.java.net/~pchelko/8024170/webrev.00/
>> The FX part will be reviewed separately, but the fix is available here:
>> http://cr.openjdk.java.net/~pchelko/8024170/webrev.rt.00/
>> 
>> This fix adds support for SwingNode cursor change on Windows and Mac OS X. 
>> Linux is not supported yet, because the native FX window handle should 
>> passed to AWT and this functionality is not implemented yet.
>> 
>> With best regards. Petr.
> 

Reply via email to