+1 Krishna
-----Original Message----- From: Dmitry Markov Sent: Friday, October 26, 2018 4:57 PM To: Manajit Halder <manajit.hal...@oracle.com> Cc: awt-dev@openjdk.java.net Subject: Re: <AWT Dev> <AWT dev>[12] Review request for JDK-8208543: [macos] Support for apple.awt.documentModalSheet incomplete Hi Manajit, Looks good to me. Thanks, Dmitry > On 26 Oct 2018, at 11:02, Manajit Halder <manajit.hal...@oracle.com> wrote: > > Hi Dmitry, > > I have corrected the test case, now it fails if timeout takes place. Please > review the webrev: > http://cr.openjdk.java.net/~mhalder/8208543/webrev.02/ > <http://cr.openjdk.java.net/%7Emhalder/8208543/webrev.02/> > > Regards, > Manajit > > > > On 25/10/18 7:18 PM, Dmitry Markov wrote: >> Hi Manajit, >> >> Currently the test is marked as ‘passed’ if timeout takes place. I think we >> should indicate an error or mark it as ‘failed’ in such case. >> >> Thanks, >> Dmitry >> >>> On 25 Oct 2018, at 11:35, Manajit Halder <manajit.hal...@oracle.com> wrote: >>> >>> Hi Dmitry, >>> >>> Thanks for your comments. I have addressed all your review comments in the >>> new webrev. >>> Additional changes: >>> NSDocModalWindowMask is deprecated and hence changed it to >>> NSWindowStyleMaskDocModalWindow. >>> Window is created a Panel, required for style mask >>> NSWindowStyleMaskDocModalWindow. >>> Test case was modified to add a case for the failed scenario "Dialog >>> without owner". >>> >>> Please review the modified webrev: >>> http://cr.openjdk.java.net/~mhalder/8208543/webrev.01/ >>> <http://cr.openjdk.java.net/%7Emhalder/8208543/webrev.01/> >>> >>> Regards, >>> Manajit >>> >>> On 13/10/18 12:14 AM, Dmitry Markov wrote: >>>> Hi Manajit, >>>> >>>> There is an inconsistency between the proposed implementation and Apple >>>> JDK: if the property applied to the dialog which does not have an owner on >>>> the build with your changes it appears as sheet, but on Apple JDK it >>>> appears as a window. >>>> >>>> I think every frame/dialog inside dispose() method in the regression test >>>> should be checked for null-value before usage. >>>> >>>> I noticed that the regression test uses Timer API (see >>>> createAndShowModalSheet() method). Shall we stop/cancel the timer when >>>> “Pass”/“Fail” button is press? >>>> >>>> I suppose it is better to declare createAndShowModalSheet() and >>>> createAndShowInstructionFrame() as static. In such case the creation of >>>> class instance may be omitted. >>>> >>>> Thanks, >>>> Dmitry >>>> >>>>> On 12 Oct 2018, at 05:36, Manajit Halder <manajit.hal...@oracle.com> >>>>> wrote: >>>>> >>>>> Hi Dmitry, >>>>> >>>>> Could you please review this fix related to Modal sheet on Mac OS? >>>>> >>>>> Regards, >>>>> Manajit >>>>> >>>>> >>>>> On 10/10/18 3:33 PM, Manajit Halder wrote: >>>>>> Hi All, >>>>>> >>>>>> Kindly review the fix for JDK12. >>>>>> >>>>>> Bug: >>>>>> https://bugs.openjdk.java.net/browse/JDK-8208543 >>>>>> >>>>>> >>>>>> Webrev: >>>>>> http://cr.openjdk.java.net/~mhalder/8208543/webrev.00/ >>>>>> <http://cr.openjdk.java.net/%7Emhalder/8208543/webrev.00/> >>>>>> >>>>>> Problem: >>>>>> "apple.awt.documentModalSheet" was getting set on the Dialog while its >>>>>> creations, but appearance of Dialog was not changing. >>>>>> >>>>>> Fix: >>>>>> Setting "apple.awt.documentModalSheet" on Window after its creation. >>>>>> >>>>>> Regards, >>>>> Manajit >
