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