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

[shashidhara.veerabhadraiah at oracle.com]

The changes looks fine to me.

Thanks and regards,

Shashi


On 21/03/18 2:56 AM, Phil Race wrote:
> Makes sense once I'd looked at the native side.
>
> Approved.
>
> -phil.
>
> On 03/08/2018 01:57 AM, Manajit Halder wrote:
>> 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 was0000111110000010 (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 <mailto: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 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
>>>>
>>>>
>>>> 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
>>>>>>
>>>>>> 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/20180321/4c00854b/attachment-0001.html>