Thank you for the reviews.
Please have a look on the updated version here:
http://cr.openjdk.java.net/~pchelko/8006941/webrev.04/
The CDropTarget is now also ready to work with a view-based embedded frame.
Also A native crash was discovered, it will be fixed with the current fix.
I have discovered 2 additional DnD issues while testing, I would file separate
issues.
With best regards. Petr.
On Feb 28, 2013, at 4:47 PM, Denis S. Fokin wrote:
> Hi Petr,
>
> I would make these lines shorter...
> CDropTarget.m:495
> CDropTarget.m:609
>
> Ok, I see that in the next line I did not noticed that the parentheses
> are related to the whole statement.
> 457 if (!(root.contains(x, y) && root.isEnabled() &&
> root.isVisible())) {
>
> Therefor, I would prefer to see || instead of &&. But it is just my personal
> observation.
>
> Thank you,
> Denis.
>
> On 2/28/2013 4:36 PM, Petr Pchelko wrote:
>> Hello, Denis, Sergey. Thank you for the reviews.
>>
>> The updated version of the fix is available here:
>> http://cr.openjdk.java.net/~pchelko/8006941/webrev.03/
>>
>> Sergey wrote:
>>> getViewPtr() is not a good method in PlatformWindow interface because
>>> pointer to NSView is macosx specific implementation.
>> Removed it, replace with if's with instanceof
>>
>>> What does it mean "Returns the first " in the getDropTargetAt. Should it be
>>> hw/lw?
>> It should return any component which would be a valid drop target for the
>> drag. It does not matter, lightweight or heavyweight. I have updated a
>> comment a bit.
>>
>>> Check your fix with the example from the CR:8007155. Just modify for dnd.
>> Works fine.
>>
>> Denis wrote:
>>> Where we are posting dragExit and dragEnter messages now? Is it the event
>>> dispatching issue that you have mentioned?
>>> CDropTarget.m:516,614
>>
>> The DragSource Enter/Exit messages are posted in Java now. The issue is that
>> native dropTarget bounds are not the same as in Java because a native peer
>> for DnD is a contentView of the window. So these events are posted in the
>> wrong places or were not posted when they should have been. For DropTarget
>> events this issue also exist and the Enter/Exit events are modified on the
>> Java level. But for the dragSource I've decided not to use native events at
>> all, because they are not very useful, it's easier to synthesize them in
>> Java.
>>
>>> I would say that visibility and isEnabled() value do not make sense here.
>>> CDragSourceContextPeer.java
>>> 449 if (!(root.contains(x, y) && root.isVisible() &&
>>> root.isEnabled())) {
>>
>> We should find only the dropTarget which would be a valid target for the
>> current drag. This implies that the component should be enabled, visible,
>> and should have a valid drop target. This would produce the same behavior as
>> on Windows.
>>
>> With best regards. Petr.
>>
>> On Feb 28, 2013, at 1:24 PM, Sergey Bylokhov wrote:
>>
>>> Hi, Petr.
>>> getViewPtr() is not a good method in PlatformWindow interface because
>>> pointer to NSView is macosx specific implementation.
>>> Could you please split VERY long line in the CDropTarget.m
>>> What does it mean "Returns the first " in the getDropTargetAt. Should it be
>>> hw/lw?
>>> Check your fix with the example from the CR:8007155. Just modify for dnd.
>>>
>>> 27.02.2013 17:43, Petr Pchelko wrote:
>>>> Hello, AWT Team.
>>>>
>>>> Please, review the fix for 2 issues:
>>>> http://bugs.sun.com/view_bug.do?bug_id=7199783 Setting cursor on
>>>> DragSourceContext does not work on OSX
>>>> http://bugs.sun.com/view_bug.do?bug_id=8006941 Deadlock in drag and drop
>>>> The fix is available at:
>>>> http://cr.openjdk.java.net/~pchelko/8006941/webrev.02/
>>>>
>>>> I have made a single fix for these 2 issues, because they are quite
>>>> closely related, and the same methods need to be changed. And they depend
>>>> on one another quite a bit.
>>>>
>>>> 1. The deadlock occurred because in CDragSourceContextPeer.dragMouseMoved
>>>> methods on components were invoke on the Appkit thread. They blocked on an
>>>> AWTTreeLock if EDT had already took it. EDT trying to perform a sync
>>>> selector on the Appkit thread lead to a deadlock. So the logic of working
>>>> with components are moved to the EDT now.
>>>> 2. DragSource events were dispatched absolutely incorrectly. Now we
>>>> dispatch them the same way as on other platforms.
>>>> 3. CCursorManager contained a workaround for the issue that we were not
>>>> able to perform sync selectors during the dnd. Now sync selectors are
>>>> processed during drag, so this workaround is not needed any more.
>>>> 4. The functionality to set a cursor in DragSourceListeners is
>>>> implemented. It works, however it still has a couple of issues:
>>>> a. Sometimes mouse events are dispatched during the dnd, which might
>>>> reset a cursor. That is wrong, mouse events should not be dispatched. It
>>>> is a separate issue, so this problem will disappear as mouseEvent
>>>> dispatching would be fixed.
>>>> b. If the DropTarget supports NSDragOperationCopy cocoa sets an
>>>> NSDragCopyCursor. There is no API to disable this and I have found no
>>>> suitable workaround.
>>>> c. On the first drag in the lifetime of the application cursor is not
>>>> set. This is due to a bug in cocoa. They reset a cursor in some setter
>>>> which looks absolutely unrelated and called when the first dragging
>>>> session is initialized. I am thinking about filing a bug against apple.
>>>> 5. Some cleanup: removed unused variables.
>>>>
>>>> With best regards. Petr.
>>>
>>>
>>> --
>>> Best regards, Sergey.
>>>
>>
>
