<AWT Dev> <AWT dev>[12] Review request for JDK-8208543: [macos] Support for apple.awt.documentModalSheet incomplete

[Krishna Addepalli]

+1

Krishna

-----Original Message-----
From: Dmitry Markov 
Sent: Friday, October 26, 2018 4:57 PM
To: Manajit Halder <manajit.halder at oracle.com>
Cc: awt-dev at 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.halder at 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.halder at 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.halder at 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
>