Looks good to me. Regards, Rajeev Chamyal
-----Original Message----- From: Alexandr Scherbatiy Sent: 10 June 2016 13:06 To: Ajit Ghaisas; Sergey Bylokhov; Rajeev Chamyal; swing-dev@openjdk.java.net Subject: Re: Fix for JDK-8065861 : Pressing Esc does not set 'canceled' property of ProgressMonitor The fix looks good to me. Thanks, Alexandr. On 6/10/2016 8:50 AM, Ajit Ghaisas wrote: > Hi, > > Thanks Alex for spotting probable exceptions in code changes. > I have corrected the code to address them. > > Here is the updated webrev. Request you to review. > http://cr.openjdk.java.net/~aghaisas/8065861/webrev.01/ > > Regards, > Ajit > > > -----Original Message----- > From: Alexandr Scherbatiy > Sent: Thursday, June 09, 2016 8:56 PM > To: Ajit Ghaisas; Sergey Bylokhov; Rajeev Chamyal; swing-dev@openjdk.java.net > Subject: Re: Fix for JDK-8065861 : Pressing Esc does not set 'canceled' > property of ProgressMonitor > > On 6/8/2016 5:35 PM, Ajit Ghaisas wrote: >> Hi, >> >> Bug : >> https://bugs.openjdk.java.net/browse/JDK-8065861 >> >> Issue : >> Pressing Esc does not set 'canceled' property of ProgressMonitor >> >> Analysis : >> ProgressMonitor option pane only gets hidden on pressing Escape key. >> It is not truly canceled as isCanceled() method continues to return false. >> >> Fix : >> On Pressing Escape key JOptionPane.CLOSED_OPTION value is set in JOptionPane >> from BasicOptionPaneUI.java. >> This value is used as a condition in isCanceled() to identify >> ProgressMonitor is canceled by pressing Escape key. >> >> Please review the webrev: >> http://cr.openjdk.java.net/~aghaisas/8065861/webrev.00/ > The updated code calls methods from 'v' object before it is checked to > the null which can lead to the NPE. The cancelOption[0] also can throw the > IOBE before the arrays length checking. > > Thanks, > Alexandr. >> On review completion, I will raise a CCC request for documentation change. >> >> Regards, >> Ajit