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/~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 <[email protected]> wrote:
> 
> Hi Phil,
> 
> Please find my answer inline to your comment.
> 
>> On 15-Jun-2018, at 11:24 PM, Phil Race <[email protected] 
>> <mailto:[email protected]>> 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 
>> <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
>  
> <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/~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 
>>> <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
>> 
> 

Reply via email to