<AWT Dev> Subject: <AWT dev>[11] Review request for JDK-7158623: [macosx] Should an unfocusable maximized Frame be resizable?

[Manajit Halder]

Hi Phil,

Please find my answers inline:

Before: if its unfocusable, then setStyleBits will set the mask to 0 :

    // this is the counter-point to -[CWindow _nativeSetStyleBit:]
    private void setStyleBits(final int mask, final boolean value) {
        execute(ptr -> nativeSetNSWindowStyleBits(ptr, mask, value ? mask : 0));
    }

Before if its unfocusable, then setStyleBits will not set the bits SHOULD_BECOME_KEY and SHOULD_BECOME_MAIN if the window is not focusable. 
While debugging it was observed that the bits set on the window in this case was 0000111110000010 (the 9th bit is set, whereas the 12th and 13th bits are not set)

After: .. well exactly the same thing .. you just added a RESIZABLE bit that will be ignored.

Now after my fix RESIZABLE bit along with the other bits won’t be set if the window is not focusable. Bits set on the window in this case was 0000110110000010  (the 9th bit is not set)

The bit setting is done in the native method Java_sun_lwawt_macosx_CPlatformWindow_nativeSetNSWindowStyleBits.

Thanks,
Manajit


> On 26-Feb-2018, at 1:28 PM, Manajit Halder <manajit.halder at oracle.com> wrote:
> 
> Hi Phil and Kevin,
> 
> Sorry for specifying wrong JDK version. This review is for JDK 11.
> 
> Regarding spec:
>    	It is not mentioned in the focus spec whether the unfocusable maximized frames should be resizable or not.
>    	The fix is given based on the implementation on other platforms (windows and linux/unix) . On both the platforms (windows and unix/linux) the test passes whereas the test fails on Mac.
> 	An old Issue https://bugs.openjdk.java.net/browse/JDK-4980161 <https://bugs.openjdk.java.net/browse/JDK-4980161> is related to this fix.
>   
> Regarding fix:
> 	The test case java/awt/Frame/UnfocusableMaximizedFrameResizablity/UnfocusableMaximizedFrameResizablity.java fails on Mac and passes on windows and linux/unix.
> 	As per the test case when setFocusableWindowState is set to false frame should not be resizable. 
> 	updateFocusableWindowState() updates the window focusable state and the fix under review sets RESIZABLE property to true or false based on focusability of the frame.
> 	
> Regards,
> Manajit
> 
> 
>> webrev appears to be against the correct repo :
>> sh://hg.openjdk.java.net/jdk/client <sh://hg.openjdk.java.net/jdk/client> 
>> 
>> 
>> First can you point to what in the spec. or whatever points to
>> the correct desired behaviour being what you describe :
>> >If the frame is focusable then it is resizable otherwise not.
>> 
>> And regarding the fix, it isn't obvious how adding RESIZABLE changes anything here
>> for the unfocusable case which seems to be the situation that is under discussion.
>> 
>> Before: if its unfocusable, then setStyleBits will set the mask to 0 :
>> 
>>     // this is the counter-point to -[CWindow _nativeSetStyleBit:]
>>     private void setStyleBits(final int mask, final boolean value) {
>>         execute(ptr -> nativeSetNSWindowStyleBits(ptr, mask, value ? mask : 0));
>>     }
>> 
>> After: .. well exactly the same thing .. you just added a RESIZABLE bit that will be ignored.
>> 
>> It may make a difference in the isFocusable case .. but that does not match what you
>> say you are doing.
>> 
>> -phil.
>> 
>> On 02/16/2018 08:17 AM, Kevin Rushforth wrote:
>>> 
>>> > Kindly review the fix for JDK10.
>>> 
>>> You mean JDK 11, right?
>>> 
>>> -- Kevin
>>> 
>>> 
>>> Manajit Halder wrote:
>>>> 
>>>> Hi All,
>>>> 
>>>> Kindly review the fix for JDK10.
>>>> 
>>>> Bug: 
>>>> https://bugs.openjdk.java.net/browse/JDK-7158623 <https://bugs.openjdk.java.net/browse/JDK-7158623>
>>>> 
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~mhalder/7158623/webrev.00/ <http://cr.openjdk.java.net/%7Emhalder/7158623/webrev.00/>
>>>> 
>>>> Issue:
>>>> Unfocusable frame was resizable.
>>>> 
>>>> Fix:
>>>> After the fix resizability of the frame depends on focusability of the frame. If the frame is focusable then it is resizable otherwise not.
>>>> The test code was cleaned to remove old test machinery code.
>>>> 
>>>> Regards,
>>>> Manajit
>>>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20180308/9ab0e137/attachment-0001.html>