<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

Manajit Halder manajit.halder at oracle.com
Tue Jul 10 11:33:13 UTC 2018 

Hi Sergey and Prasanta,

Thanks for your comment. The fullscreen test (i.e. case 3 in the test) is made to run only on Mac as the full screen property (“apple.awt.fullscreenable”) is specific to Mac OS.
Please review the updated webrev:
http://cr.openjdk.java.net/~mhalder/8204860/webrev.06/ <http://cr.openjdk.java.net/~mhalder/8204860/webrev.06/>

Regards,
Manajit

> On 09-Jul-2018, at 6:33 PM, Prasanta Sadhukhan <prasanta.sadhukhan at oracle.com> wrote:
> 
> 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.
>>> 
>> 
>> 
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20180710/43b2724c/attachment.html>

More information about the awt-dev mailing list