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

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Manajit Halder
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

Reply | Threaded
Open this post in threaded view
|

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

Manajit Halder
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
>

Reply | Threaded
Open this post in threaded view
|

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

Dmitry Markov
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 <[hidden email]> 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
>>
>

Reply | Threaded
Open this post in threaded view
|

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

Manajit Halder
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 <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

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

Dmitry Markov
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 <[hidden email]> 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 <[hidden email]> 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
>

Reply | Threaded
Open this post in threaded view
|

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

Manajit Halder
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 <[hidden email]> 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 <[hidden email]> 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

Reply | Threaded
Open this post in threaded view
|

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

Dmitry Markov
Hi Manajit,

Looks good to me.

Thanks,
Dmitry

> On 26 Oct 2018, at 11:02, Manajit Halder <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
>

Reply | Threaded
Open this post in threaded view
|

Re: <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 <[hidden email]>
Cc: [hidden email]
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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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
>

Reply | Threaded
Open this post in threaded view
|

Re: <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 <[hidden email]>
> Cc: [hidden email]
> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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.
Reply | Threaded
Open this post in threaded view
|

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

Denis Fokin
Hi Manajit, Sergey,

we use a swing dialog-based implementation in our product. I believe the current terms and behaviour could be improved.

There are some collision in terms. To be a document-modal in AWT means to block "all windows from the same document except those from its child hierarchy". The MacOS X sheet, according to the Apple guide [1], " is a modal dialog that’s attached to a particular window—usually a document—and prevents further interaction with the window". So looks like from the UIX experience it should be a "Window Modal". 

Basically, if the sheet is shown on a window and it blocks the owner of the window, it is hard to understand why the owner is irresponsive. Any actions like switching to a "space" with the blocker, reordering windows look awful from the user perspective.

1 - https://developer.apple.com/design/human-interface-guidelines/macos/windows-and-views/sheets/

Thank you,
    Denis.

On Wed, Oct 31, 2018 at 4:15 AM Sergey Bylokhov <[hidden email]> wrote:
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 <[hidden email]>
> Cc: [hidden email]
> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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.
Reply | Threaded
Open this post in threaded view
|

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

Dmitry Markov
In reply to this post by Sergey Bylokhov
Hi Sergey,

You are right, the support for “apple.awt.documentModalShee” property is incomplete. If the property is set for a window, the window appears as a sheet but it is not attached to its owner. I have opened JDK-8213197 [1] for this.

Thanks,
Dmitry

[1] - https://bugs.openjdk.java.net/browse/JDK-8213197

> On 31 Oct 2018, at 01:14, Sergey Bylokhov <[hidden email]> wrote:
>
> 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 <[hidden email]>
>> Cc: [hidden email]
>> 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 <[hidden email]> 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 <[hidden email]> 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 <[hidden email]> 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.