PS .. there should be some tests on the JDK side that don't use FX
Or at least a reasoned explanation as to why this isn't possible.
-phil.
On 5/10/18, 5:49 PM, Philip Race wrote:
I've looked over the Swing code. No time to actually try it out so I
can only comment on what I see.
I am not sure what I think about class docs like "This class wraps
<names internal interface> .."
I know no javadoc is generated but this module is exported and
probably should say something
less specific.
In general I really have to hold my nose when looking at this whole
module and I
find it distasteful to endorse it.
I really don't like all the things SwingInterOpUtils.java does.
I worry we are creating something we'll come to regret here.
The "unsupportedness" needs to be backed up by deprecated-for-removal
being included
today so we can get rid of it the next release if we want to.
No @since tags anywhere....
-phil.
On 5/10/18, 8:32 AM, Kevin Rushforth wrote:
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.
I am not a Swing "R"eviewer, but the .13 webrev gets a +1 from me.
-- Kevin
On 5/10/2018 8:20 AM, Prasanta Sadhukhan wrote:
Hi Kevin,All,
Please find the modified webrev fixing this #1 issue
http://cr.openjdk.java.net/~psadhukhan/fxswing.13/
via change in
jdk/swing/interop/DropTargetContextWrapper.java#setDropTargetContext
and FXDnD.java#postDropTargetEvent
For me, #2 works, #3 doesn't work even now due to JDK-8141391
<https://bugs.openjdk.java.net/browse/JDK-8141391> and #4 works for me.
Regards
Prasanta
On 5/9/2018 11:29 PM, Kevin Rushforth wrote:
Hi Prasanta,
The API looks good now.
All of our automated tests work (except for the ones with a
security manager due to JDK-8202451
<https://bugs.openjdk.java.net/browse/JDK-8202451>).
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:
1. Drag / drop onto a Swing component in a SwingNode
2. Drag / drop from a Swing component in a SwingNode
3. Drag / drop onto a JavaFX control in a JFXPanel
4. Drag / drop from a JavaFX control in a JFXPanel
So far I only tried the first one; the others still need to be
validated.
-- Kevin
On 5/9/2018 7:14 AM, Prasanta Sadhukhan wrote:
Modified webrev to cater to this
http://cr.openjdk.java.net/~psadhukhan/fxswing.12/
Regards
Prasanta
On 5/9/2018 5:58 PM, Kevin Rushforth wrote:
The following can also be abstract:
LightweightContentWrapper:
getComponent, createDragGestureRecognizer,
createDragSourceContextPeer
DropTargetContextWrapper:
getTargetActions, getDropTarget, getTransferDataFlavors,
getTransferable, isTransferableJVMLocal
DispatcherWrapper:
isDispatchThread, createSecondaryLoop
The rest looks good to me (although I still see two public
methods with "Peer" in the name, so Phil may want those renamed).
-- Kevin
On 5/9/2018 2:14 AM, Prasanta Sadhukhan wrote:
Modified webrev to cater to these 3 observations
http://cr.openjdk.java.net/~psadhukhan/fxswing.11/
Regards
Prasanta
On 5/9/2018 5:03 AM, Kevin Rushforth wrote:
The module definition for jdk.unsupported.desktop and the
changes to java.desktop look fine.
In reviewing the jdk.swing.interop API, I have the following
suggestions / observations:
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).
2. The addNotify method in LightweightFrameWrapper is unused.
Should be used somewhere? If not, then it can be removed.
The implementation of the new wrapper classes looks OK to me
with one observation that might or might not matter:
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.
-- Kevin
On 5/8/2018 3:31 AM, Alan Bateman wrote:
On 08/05/2018 06:51, Prasanta Sadhukhan wrote:
Modified webrev to rename to InteropProviderImpl
http://cr.openjdk.java.net/~psadhukhan/fxswing.10/
This looks okay to me.
-Alan
