<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

Prasanta Sadhukhan prasanta.sadhukhan at oracle.com
Mon Jul 9 13:03:08 UTC 2018 

Also, I guess JFrame handling should be in EDT.

Regards
Prasanta
On 7/9/2018 6:14 PM, Sergey Bylokhov wrote:
> Hi, Manajit.
> Did you run the test on other platforms?
> I am not sure, but look like the line below can throw npe:
> if(!prop1.equals(prop2)) {
>
> On 06/07/2018 16:37, Manajit Halder wrote:
>> Hi Sergey,
>>
>> Checked the behaviour of setCanFullscreen() and found the test was 
>> failing. The window was "FULLSCREENABLE” when it was not focusable.
>> Changed the code to fix this issue. After fix the window (JFrame) is 
>> not “FULLSCREENABLE” when it is not resizable and non-focusable.
>>
>> Please review the changes:
>> http://cr.openjdk.java.net/~mhalder/8204860/webrev.05/ 
>> <http://cr.openjdk.java.net/%7Emhalder/8204860/webrev.05/>
>>
>> Regards,
>> Manajit
>>
>>> On 04-Jul-2018, at 5:52 PM, Sergey Bylokhov 
>>> <Sergey.Bylokhov at oracle.com <mailto:Sergey.Bylokhov at oracle.com>> wrote:
>>>
>>> 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/ 
>>>> <http://cr.openjdk.java.net/%7Emhalder/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><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/ 
>>>>>> <http://cr.openjdk.java.net/%7Emhalder/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><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/ 
>>>>>>>> <http://cr.openjdk.java.net/%7Emhalder/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.
>>
>
>

More information about the awt-dev mailing list