[11] RFR JDK-8202199 : Provide public, unsupported API for FX Swing interop

Philip Race philip.race at oracle.com
Fri May 11 00:49:51 UTC 2018


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
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>


More information about the openjfx-dev mailing list