<AWT Dev> [8] Review Request for JDK-8024170, [SwingNode] Implement cursor change

[Anton Tarasov]

Hi Petr,

Great! At least, you no longer need to kick the awt cursor machinery =)

Thanks!
Anton.

On 9/12/2013 4:58 PM, Petr Pchelko wrote:
> Hello, AWT Team.
>
> Please disregard the previous version of the fix. We've decided to change the approach completely.
> The new version of the JDK part is available at:
> http://cr.openjdk.java.net/~pchelko/8024170/webrev.03/
>
> The SwingAccessor is used because Component.updateCursorImmediately is a final method. It's a part of the public API, so the only way to introduce the new behaviour is to override updateCursorImmediately inside peer classes.
>
> With best regards. Petr.
>
> On Sep 5, 2013, at 10:13 AM, Anton V. Tarasov <anton.tarasov at oracle.com> wrote:
>
>> Hi Petr,
>>
>> On 04.09.2013 13:19, Petr Pchelko wrote:
>>> 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.
>> I'm fine with it!
>>
>> Thanks,
>> Anton.
>>
>>> With best regards. Petr.
>>>
>>> On Sep 3, 2013, at 7:53 PM, Anton V. Tarasov <anton.tarasov at oracle.com> 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.