<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
Wed Jul 11 10:46:40 UTC 2018
+1
Regards
Prasanta
On 7/10/2018 5:03 PM, Manajit Halder wrote:
> 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/%7Emhalder/8204860/webrev.06/>
>
> Regards,
> Manajit
>
>> On 09-Jul-2018, at 6:33 PM, Prasanta Sadhukhan
>> <prasanta.sadhukhan at oracle.com
>> <mailto: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/>
>>>> <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>
>>>>> <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/>
>>>>>> <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><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/>
>>>>>>>> <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><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/>
>>>>>>>>>> <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>
>>>>>>>>>>> <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>
>>>>>>>>>>>> <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/
>>>>>>>>>>> <http://cr.openjdk.java.net/%7Emhalder/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/20180711/1f00435c/attachment-0001.html>
More information about the awt-dev
mailing list