<AWT Dev> [8] Review Request for CR JDK-8020371 [macosx] applets with Drag and Drop fail with IllegalArgumentException

[Petr Pchelko]

Hello, Anthony.

> The fix looks good to me. BTW, could we provide some meaningful message with the InvalidDnDOperationException ? E.g. state that it's unsupported for this window or something like this? Also, the comments "// Unsupported" after the if()return; could be better worded as "Unsupported for a window w/o a native view (plugin)".
I've updated the fix. The new version is available here:
http://cr.openjdk.java.net/~pchelko/8020371/webrev.02/

With best regards. Petr.

On Jul 11, 2013, at 6:11 PM, Anthony Petrov wrote:

> Hi Petr,
> 
> The fix looks good to me. BTW, could we provide some meaningful message with the InvalidDnDOperationException ? E.g. state that it's unsupported for this window or something like this? Also, the comments "// Unsupported" after the if()return; could be better worded as "Unsupported for a window w/o a native view (plugin)".
> 
> --
> best regards,
> Anthony
> 
> On 07/11/13 17:50, Petr Pchelko wrote:
>> Hello, Artem.
>> 
>> I've updated the fix, the new version is here:
>> http://cr.openjdk.java.net/~pchelko/8020371/webrev.01/
>> 
>>> 1. Are CDropTarget and CDragSourceContextPeer ready to handle the case, when their nativePtr is 0?
>> 	1. CDropTarget: it's absolutely ready, it has only create and release methods which are ready to have a nativePtr=0.
>> 	In native we create a delegate and set it as the DropTarget to the View. This DropTarget get's notified when NSView see some DnD operation.
>> 	So we cannot and should not set it for applets.
>> 
>> 	2. CDragSourceContextPeer - it has only one public method: startDrag which could be called when a DragGestureRecognizer recognizes the Drag. In applets it could never happen,
>> 	so the CDragSourceContextPeer wouldn't event be created. I'be updated it just to make sure we would never get a native crash with 0 viewPtr.
>> 
>>> 2. Lines 110-113 in CDragSourceContextPeer should be better moved to before line 140.
>> 	I've updated the fix.
>> 
>>> 3. It's unclear if CDropTarget and CDragSourceContextPeer should be used for lightweight components at all. How lightweight components are handled on other platforms.?
>> Not sure about other platforms, but on Mac it works as follows:
>> 	When a DropTarget is set to a lightweight component we set to it's peer and traverse the tree up until we get a non-lightweight peer. And create, update (or don't update) the
>> 	CDropTarget for that peer. The CDragSourceContextPeer is a singleton. When DnD is started we update the trigger in it. The trigger could be any component. To get native support
>> 	we traverse up the tree from the trigger t get a first heavyweight component and use it as a native peer for DnD.
>> 
>> I\m not sure I've answered your question thought.
>> 	
>> With best regards. Petr.
>> 
>> On Jul 11, 2013, at 4:15 PM, Artem Ananiev wrote:
>> 
>>> Hi, Petr,
>>> 
>>> I'm not an expert in this code, so a few questions about the fix:
>>> 
>>> 1. Are CDropTarget and CDragSourceContextPeer ready to handle the case, when their nativePtr is 0?
>>> 
>>> 2. Lines 110-113 in CDragSourceContextPeer should be better moved to before line 140.
>>> 
>>> 3. It's unclear if CDropTarget and CDragSourceContextPeer should be used for lightweight components at all. How lightweight components are handled on other platforms.?
>>> 
>>> Thanks,
>>> 
>>> Artem
>>> 
>>> On 7/11/2013 1:51 PM, Petr Pchelko wrote:
>>>> Hello, AWT Team.
>>>> 
>>>> Please review the fix for the issue:
>>>> http://bugs.sun.com/view_bug.do?bug_id=8020371
>>>> (It's recently created, so it might not be visible on bugs.sum.com yet)
>>>> The fix is available at:
>>>> http://cr.openjdk.java.net/~pchelko/8020371/webrev/
>>>> 
>>>> The problem:
>>>> We currently do not support DnD in applets on Mac, so IllegalArgumentException was thrown. If you explicitly call component.setDropTarget(new DropTarget()) the applet would fail to start due to this exception.
>>>> 
>>>> The solution:
>>>> The fix removes the exception and silently ignores unsupported DnD initialization.
>>>> DnD does not start to work, however at least an applet could be started.
>>>> However, I'm not sure if we should print a warning about unsupported DnD. What do you think?
>>>> 
>>>> With best regards. Petr.
>>>> 
>>