<AWT Dev> <AWT dev>[11] Review request for JDK-8204860: The frame could be resized by dragging a corner of the frame with the mouse

Wed Jul 4 12:22:21 UTC 2018

Hi, Manajit.
Can you please recheck the usage of setCanFullscreen() method at line 
691. Is it possible to make the window "FULLSCREENABLE" when it is not 
focusable? I guess it can be checked by something like this:

frame.setFocusableWindowState(false);
frame.setResizable(true);
v1 = frame.getRootPane().getClientProperty("apple.awt.fullscreenable");
frame.setVisible(false);
frame.setVisible(true);
v2 = frame.getRootPane().getClientProperty("apple.awt.fullscreenable");
if(!v1.equals(v2)) Error();

On 02/07/2018 20:12, Manajit Halder wrote:
> Hi Sergey,
> 
> Thanks for your review comment. Code changed as per your suggestion.
> Please review the changes:
> http://cr.openjdk.java.net/~mhalder/8204860/webrev.04/
> 
> Regards,
> Manajit
> 
>> On 01-Jul-2018, at 4:45 AM, Sergey Bylokhov 
>> <Sergey.Bylokhov at oracle.com <mailto:Sergey.Bylokhov at oracle.com>> wrote:
>>
>> Hi, Manajit
>> Can you please simplify such new code in the fix:
>> (!isFocusable || !isResizable) ? false : true
>>
>> "false: true" may be omitted.
>>
>> Also please split this new long line:
>> 486 return (target instanceof Frame) ? ((Frame)target).isResizable() : 
>> ((target instanceof Dialog) ? ((Dialog)target).isResizable() : false
>>
>> On 28/06/2018 05:54, Manajit Halder wrote:
>>> Hi Sergey,
>>> Thanks for your review comment. I have modified the fix to 
>>> incorporate your suggestions.
>>> Please review the modified webrev:
>>> http://cr.openjdk.java.net/~mhalder/8204860/webrev.03/
>>> Regards,
>>> Manajit
>>>> On 26-Jun-2018, at 4:59 AM, Sergey Bylokhov 
>>>> <sergey.bylokhov at oracle.com 
>>>> <mailto:sergey.bylokhov at oracle.com><mailto:sergey.bylokhov at oracle.com>> 
>>>> wrote:
>>>>
>>>> Hi, Manajit.
>>>> There is one more inconsistency, I have tested it using the test for 
>>>> teh previous fix: UnfocusableMaximizedFrameResizablity.java
>>>>
>>>> In this case the window is zoomable(the green button is active, but 
>>>> does not work as expected):
>>>>        frame.setFocusableWindowState(false);
>>>>        frame.setResizable(true);
>>>>
>>>> In this case the window is not zoomable(the green button is inactive):
>>>>        frame.setResizable(true);
>>>>        frame.setFocusableWindowState(false);
>>>>
>>>> I suggest to update the testcase to cover all this cases which we 
>>>> found during review.
>>>>
>>>> On 21/06/2018 12:26, Manajit Halder wrote:
>>>>> Hi Phil and Sergey,
>>>>> I have changed the code as per your suggestions. Now window is 
>>>>> resized based on the following condition:
>>>>> If window is non-focusable OR window is focusable but non-resizable 
>>>>> THEN window is made non-resizable.
>>>>> Otherwise window is made resizable (when window is resizable and 
>>>>> focusable).
>>>>> Please review the changes:
>>>>> http://cr.openjdk.java.net/~mhalder/8204860/webrev.02/
>>>>> Note: Fix was verified by running all the java/awt/ jtreg test 
>>>>> cases and by executing the reported JCK interactive test case.
>>>>> Regards,
>>>>> Manajit
>>>>>> On 20-Jun-2018, at 6:27 PM, Manajit Halder 
>>>>>> <manajit.halder at oracle.com <mailto:manajit.halder at oracle.com>> wrote:
>>>>>>
>>>>>> Hi Phil,
>>>>>>
>>>>>> Please find my answer inline to your comment.
>>>>>>
>>>>>>> On 15-Jun-2018, at 11:24 PM, Phil Race <philip.race at oracle.com 
>>>>>>> <mailto:philip.race at oracle.com>> wrote:
>>>>>>>
>>>>>>> I would like to refer back to a comment you made in the previous fix
>>>>>>> http://mail.openjdk.java.net/pipermail/awt-dev/2018-February/013626.html
>>>>>>>
>>>>>>> > It is not mentioned in the focus spec whether the unfocusable 
>>>>>>> maximized frames should be resizable or not.
>>>>>>>
>>>>>>> Yet there seems to be a JCK test that says it should not be 
>>>>>>> resizable ?
>>>>>>>
>>>>>>> Can you review the spec. again ?
>>>>>>> JCK must have based the test on something .. else the test is not 
>>>>>>> valid.
>>>>>>
>>>>>> Yes, I checked FocusSpec.html and it doesn’t specify anything 
>>>>>> about resizable behaviour of non-focusable Frame.
>>>>>>
>>>>>> The UnfocusableMaximizedFrameResizablity.java  test passes on 
>>>>>> Window and Linux and fails on Mac OS.
>>>>>> Fix for issue 7158623 was done accordingly to make sure the 
>>>>>> behaviour is same on all platforms.
>>>>>>
>>>>>> If this behaviour is not correct then Window and Linux code should 
>>>>>> be changed accordingly so that all three platforms behave same.
>>>>>>
>>>>>>>
>>>>>>> If we want that behaviour specified .. we should be specifying it ..
>>>>>>> But I am not sure if it is actually enforceable on all window 
>>>>>>> managers / desktops.
>>>>>>>
>>>>>>> But I have the same issue with this fix as the previous one. 
>>>>>>> Perhaps not the fix,
>>>>>>> but the explanation. The code being changed can't be understood 
>>>>>>> without seeing
>>>>>>> the method it calls, and the native method it in turn calls.
>>>>>>>
>>>>>>> Can you provide a detailed explanation as to how this change 
>>>>>>> propagates down
>>>>>>> and does the right thing ?
>>>>>>>
>>>>>> The call flow:
>>>>>>
>>>>>> updateFocusableWindowState() calls setStyleBits with style bits to 
>>>>>> be set on the window.
>>>>>> setStyleBits() calls native method nativeSetNSWindowStyleBits. 
>>>>>> nativeSetNSWindowStyleBits passes mask and 0 as the second 
>>>>>> parameter in our case (for non-focusable windows).
>>>>>> Java_sun_lwawt_macosx_CPlatformWindow_nativeSetNSWindowStyleBits 
>>>>>> in AWTWindow.m generates newBits and applies it on the NSWindow.
>>>>>>
>>>>>> My previous fix for issue 7158623 explains bits set on the window.
>>>>>> http://openjdk.5641.n7.nabble.com/lt-AWT-Dev-gt-Subject-lt-AWT-dev-gt-11-Review-request-for-JDK-7158623-macosx-Should-an-unfocusable-m-td326691.html#a329071
>>>>>>
>>>>>>
>>>>>>> BTW stylistically - if this is the right fix - you could do :
>>>>>>>
>>>>>>> setStyleBits(SHOULD_BECOME_KEY | SHOULD_BECOME_MAIN | 
>>>>>>> ((isFocusable) ? RESIZABLE : 0), isFocusable);
>>>>>> Changed the code as per the suggestion. Please review the modified 
>>>>>> code.
>>>>>> http://cr.openjdk.java.net/~mhalder/8204860/webrev.01/
>>>>>>
>>>>>> Regards,
>>>>>> Manajit
>>>>>>
>>>>>>> -phil.
>>>>>>>
>>>>>>> On 06/14/2018 11:37 PM, Manajit Halder wrote:
>>>>>>>> Hi All,
>>>>>>>>
>>>>>>>> Kindly review the fix for JDK11.
>>>>>>>>
>>>>>>>> Bug:
>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8204860
>>>>>>>>
>>>>>>>> Webrev:
>>>>>>>> http://cr.openjdk.java.net/~mhalder/8204860/webrev.00/ 
>>>>>>>> <http://cr.openjdk.java.net/%7Emhalder/8204860/webrev.00/>
>>>>>>>>
>>>>>>>> Fix:
>>>>>>>> Frame is focusable:
>>>>>>>> Retaining the existing frame resizable behaviour (Fixes the 
>>>>>>>> current issue).
>>>>>>>> Frame is non-focusable:
>>>>>>>> Making the Frame non-resizable (Fix for issue 7158623).
>>>>>>>>
>>>>>>>> Regards,
>>>>>>>> Manajit
>>>>>>>
>>>>>>
>>>>
>>>>
>>>> --
>>>> Best regards, Sergey.
>>
>>
>> --
>> Best regards, Sergey.
> 

-- 
Best regards, Sergey.