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
<[email protected] <mailto:[email protected]>> 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
<[email protected]
<mailto:[email protected]><mailto:[email protected]>>
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
<[email protected]
<mailto:[email protected]><mailto:[email protected]><mailto:[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/
<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
<[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.
--
Best regards, Sergey.
--
Best regards, Sergey.
