Hi Petr,
The fix looks fine to me. Thank you.
A tiny nit at src/share/classes/sun/swing/SwingAccessor.java:
79 * Updates the JLightweight frame that it needs to update a cursor
s/Updates/Notifies/ . Also, please add a closing dot in the sentence to
avoid any potential javadoc warnings.
No need for a new webrev with these changes.
--
best regards,
Anthony
On 09/12/13 16:58, 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<[email protected]> 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<[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.
