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

[Sergey Bylokhov]

Hello,
Can somebody explain how the dialog which have the "documentModalSheet" property should behave?
I thought this dialogs should looks like this:
https://stackoverflow.com/questions/13777067/swing-native-look-and-feel-for-jdialog-in-macos

But when I run the manual testcase which is attached to the fix, it looks differently. It shows the undecorated dialog in the top/left corner, which is not attached to the window, and which cannot be moved.

For example in the code below, should the dialog be inside the frame(I guess this can be checked by the automated test)?
===>
         Window frame = new JFrame();
         frame.setSize(300,300);
         frame.setLocationRelativeTo(null);
         frame.setVisible(true);
         JDialog dialog =
                 new JDialog(frame, null, Dialog.ModalityType.DOCUMENT_MODAL);
         dialog.getRootPane().putClientProperty("apple.awt.documentModalSheet",
                                                Boolean.TRUE);
         dialog.add(new JLabel("Hello world!"));
         dialog.pack();
         dialog.setVisible(true);
===>

On 30/10/2018 04:16, Krishna Addepalli wrote:
> +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
>>
> 


-- 
Best regards, Sergey.