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

[Artem Ananiev]

Still looks fine to me.

Thanks,

Artem

On 7/12/2013 5:16 PM, Petr Pchelko wrote:
> 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.
>>>>>
>>>
>