<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
Mon Jul 2 17:12:23 UTC 2018
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/~mhalder/8204860/webrev.04/>
Regards,
Manajit
> On 01-Jul-2018, at 4:45 AM, Sergey Bylokhov <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/~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 <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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20180702/4aefbc0f/attachment.html>
More information about the awt-dev
mailing list