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 <[email protected]> > 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 <[email protected] >>> <mailto:[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 >>>> >>>> > 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.
