Hi, Petr.
The fix looks good.
On 26.08.2013 14:55, Petr Pchelko wrote:
Hello, AWT Team.
This is a reminder. Could I please get a second review on this fix.
With best regards. Petr.
On Aug 21, 2013, at 6:37 PM, Petr Pchelko wrote:
Hello, Anthony.
I've updated the fix according to your comments:
http://cr.openjdk.java.net/~pchelko/8015455/webrev.02/
Also, I suggest to run some DnD regression tests to ensure the fix is safe.
I've run all dnd regression tests on Windows and Mac. No new failures after the
fix.
With best regards. Petr.
On Aug 21, 2013, at 4:44 PM, Anthony Petrov wrote:
Hi Petr,
The new fix looks much better. Thanks!
I'm only concerned with the synchronization scheme. Since the DnD handlers are
synchronized methods, the new boolean flag should only be accessed under a
synchronized(this){} block in removeNotify() for consistency and thread safety.
Are you sure the cleanup() method needs to be made package-private? I believe
it should be accessible from dragExit() even if it stays private because the
methods belong to the same outer class.
Also, I suggest to run some DnD regression tests to ensure the fix is safe.
--
best regards,
Anthony
On 08/21/2013 04:11 PM, Petr Pchelko wrote:
Hello Anthony, Alexandr.
Thank you for the reviews.
Please hava a look on the new version of this fix:
http://cr.openjdk.java.net/~pchelko/8015455/webrev.01/
I have changed the approach according to your comments. DropTarget is tracking
if it is under the mouse and sends a synthetic dragExit on removeNotify if
necessary.
Also I've added a check in TransferHandler.SwingDropTarget.dragExit to make
sure that the cleanup would be called even if the DropTarget is inactive and
dragExit events are not dispatched to the dtListener.
With best regards. Petr.
On Aug 21, 2013, at 1:40 PM, Anthony Petrov wrote:
Hi Petr,
The java.awt.dnd.DropTarget is a public class, and as such adding any
non-private member to it is an API change. I'm not sure we want to expose the
dtListener field as a public (well, protected) API because it's too low-level
IMO.
Besides, it seems that in the case if the frame is disposed, an application
won't ever receive an event indicating that the DnD operation has ended (IIUC,
if it did, the normal processing of the dragExit would reset the timer).
So, how about overriding the removeNotify() in DropTarget() and sending an
appropriate DnD gesture termination event from there?
--
best regards,
Anthony
On 08/21/2013 01:21 PM, Petr Pchelko wrote:
Hello, AWT Team.
Please review the fix for the issue:
http://bugs.sun.com/view_bug.do?bug_id=8015455
The fix is available at:
http://cr.openjdk.java.net/~pchelko/8015455/webrev.00/
The SwingDropTarget initializes a SwingTimer to handle autoscroll. This timer
was stopped on dragExit and drop. However, if the frame was disposed in some
dropTarget listener the timer was never stopped and prevented the AWT
application from exiting.
With best regards. Petr,
--
Best regards, Sergey.
