Looks fine to me. Thanks.
--
best regards,
Anthony
On 07/12/2013 05: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.
