<html>
<head>
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
PS .. there should be some tests on the JDK side that don't use FX<br>
Or at least a reasoned explanation as to why this isn't possible.<br>
<br>
-phil.<br>
<br>
On 5/10/18, 5:49 PM, Philip Race wrote:
<blockquote cite="mid:5AF4E8AF.60202@oracle.com" type="cite">
<meta content="text/html; charset=UTF-8" http-equiv="Content-Type">
I've looked over the Swing code. No time to actually try it out so
I can only comment on what I see.<br>
<br>
I am not sure what I think about class docs like "This class wraps
<names internal interface> .."<br>
I know no javadoc is generated but this module is exported and
probably should say something<br>
less specific.<br>
<br>
In general I really have to hold my nose when looking at this
whole module and I<br>
find it distasteful to endorse it.<br>
<br>
I really don't like all the things SwingInterOpUtils.java does.<br>
<br>
I worry we are creating something we'll come to regret here.<br>
The "unsupportedness" needs to be backed up by
deprecated-for-removal being included<br>
today so we can get rid of it the next release if we want to.<br>
<br>
No @since tags anywhere....<br>
<br>
-phil.<br>
<br>
On 5/10/18, 8:32 AM, Kevin Rushforth wrote:
<blockquote
cite="mid:fe5995d5-d8ed-6492-a1f2-3d242424b06f@oracle.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html;
charset=UTF-8">
I confirm that this fixes the issue. The interface change to
make the two static methods e instance methods is a good change
in any case.<br>
<br>
I am not a Swing "R"eviewer, but the .13 webrev gets a +1 from
me.<br>
<br>
-- Kevin<br>
<br>
<br>
<div class="moz-cite-prefix">On 5/10/2018 8:20 AM, Prasanta
Sadhukhan wrote:<br>
</div>
<blockquote type="cite"
cite="mid:bc3b6dd5-6ae2-acaa-81f0-7eef168f271d@oracle.com">
<meta http-equiv="Content-Type" content="text/html;
charset=UTF-8">
<p>Hi Kevin,All,<br>
</p>
Please find the modified webrev fixing this #1 issue<br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Epsadhukhan/fxswing.13/"
moz-do-not-send="true">http://cr.openjdk.java.net/~psadhukhan/fxswing.13/</a><br>
via change in
jdk/swing/interop/DropTargetContextWrapper.java#setDropTargetContext
and FXDnD.java#postDropTargetEvent<br>
<br>
For me, #2 works, #3 doesn't work even now due to <a
class="issue-link" data-issue-key="JDK-8141391"
href="https://bugs.openjdk.java.net/browse/JDK-8141391"
id="key-val" rel="4851839" moz-do-not-send="true">JDK-8141391</a>
and #4 works for me.<br>
<br>
Regards<br>
Prasanta<br>
<div class="moz-cite-prefix">On 5/9/2018 11:29 PM, Kevin
Rushforth wrote:<br>
</div>
<blockquote type="cite"
cite="mid:5b57623c-bd12-89af-17e0-1a740d72031c@oracle.com">
<meta http-equiv="Content-Type" content="text/html;
charset=UTF-8">
Hi Prasanta,<br>
<br>
The API looks good now.<br>
<br>
All of our automated tests work (except for the ones with a
security manager due to <a moz-do-not-send="true"
href="https://bugs.openjdk.java.net/browse/JDK-8202451">JDK-8202451</a>).<br>
<br>
The only functional problem that I see is that Drag and Drop
onto a SwingNode doesn't work. We need to make sure that we
test the following four cases:<br>
<br>
1. Drag / drop onto a Swing component in a SwingNode<br>
2. Drag / drop from a Swing component in a SwingNode<br>
3. Drag / drop onto a JavaFX control in a JFXPanel<br>
4. Drag / drop from a JavaFX control in a JFXPanel<br>
<br>
So far I only tried the first one; the others still need to
be validated.<br>
<br>
-- Kevin<br>
<br>
<br>
<br>
<div class="moz-cite-prefix">On 5/9/2018 7:14 AM, Prasanta
Sadhukhan wrote:<br>
</div>
<blockquote type="cite"
cite="mid:1b7653d1-9c9a-7ad8-8f56-674760136d22@oracle.com">Modified
webrev to cater to this <br>
<br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Epsadhukhan/fxswing.12/"
moz-do-not-send="true">http://cr.openjdk.java.net/~psadhukhan/fxswing.12/</a>
<br>
<br>
Regards <br>
Prasanta <br>
On 5/9/2018 5:58 PM, Kevin Rushforth wrote: <br>
<blockquote type="cite">The following can also be
abstract: <br>
<br>
LightweightContentWrapper: <br>
 getComponent, createDragGestureRecognizer,
createDragSourceContextPeer <br>
<br>
DropTargetContextWrapper: <br>
 getTargetActions, getDropTarget,
getTransferDataFlavors, getTransferable,
isTransferableJVMLocal <br>
<br>
DispatcherWrapper: <br>
 isDispatchThread, createSecondaryLoop <br>
<br>
The rest looks good to me (although I still see two
public methods with "Peer" in the name, so Phil may want
those renamed). <br>
<br>
-- Kevin <br>
<br>
<br>
On 5/9/2018 2:14 AM, Prasanta Sadhukhan wrote: <br>
<blockquote type="cite">Modified webrev to cater to
these 3 observations <br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Epsadhukhan/fxswing.11/"
moz-do-not-send="true">http://cr.openjdk.java.net/~psadhukhan/fxswing.11/</a>
<br>
<br>
Regards <br>
Prasanta <br>
<br>
On 5/9/2018 5:03 AM, Kevin Rushforth wrote: <br>
<blockquote type="cite">The module definition for
jdk.unsupported.desktop and the changes to
java.desktop look fine. <br>
<br>
In reviewing the jdk.swing.interop API, I have the
following suggestions / observations: <br>
<br>
1. DispatcherWrapper, DragSourceContextWrapper,
DropTargetContextWrapper, and
LightweightContentWrapper can all be abstract, along
with most of the methods (rather than having an
empty body return value that is never used). <br>
<br>
2. The addNotify method in LightweightFrameWrapper
is unused. Should be used somewhere? If not, then it
can be removed. <br>
<br>
The implementation of the new wrapper classes looks
OK to me with one observation that might or might
not matter: <br>
<br>
3. The behavior of getDefaultScaleX/Y (which is now
in SwingInteropUtils) has changed in the case where
the Graphics is not an instance of SunGraphics2D.
The former behavior was to leave the instance
variables X and Y unchanged. The new behavior will
set them back to 1.0. Maybe this can't happen in
practice, but it is something to consider. <br>
<br>
-- Kevin <br>
<br>
<br>
On 5/8/2018 3:31 AM, Alan Bateman wrote: <br>
<blockquote type="cite">On 08/05/2018 06:51,
Prasanta Sadhukhan wrote: <br>
<blockquote type="cite">Modified webrev to rename
to InteropProviderImpl <br>
<br>
<a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Epsadhukhan/fxswing.10/"
moz-do-not-send="true">http://cr.openjdk.java.net/~psadhukhan/fxswing.10/</a>
<br>
</blockquote>
This looks okay to me. <br>
<br>
-Alan <br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
</blockquote>
</body>
</html>