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