<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

[Sergey Bylokhov]

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>> 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.