The changes looks fine to me.
Thanks and regards,
Shashi
On 21/03/18 2:56 AM, Phil Race wrote:
Makes sense once I'd looked at the native side.
Approved.
-phil.
On 03/08/2018 01:57 AM, Manajit Halder wrote:
Hi Phil,
Please find my answers inline:
Before: if its unfocusable, then setStyleBits will set the mask to 0 :
// this is the counter-point to -[CWindow _nativeSetStyleBit:]
private void setStyleBits(final int mask, final boolean value) {
execute(ptr -> nativeSetNSWindowStyleBits(ptr, mask, value ?
mask : 0));
}
Before if its unfocusable, then setStyleBits will not set the bits
SHOULD_BECOME_KEY and SHOULD_BECOME_MAIN if the window is not focusable.
While debugging it was observed that the bits set on the window in
this case was0000111110000010 (the 9th bit is set, whereas the 12th
and 13th bits are not set)
After: .. well exactly the same thing .. you just added a RESIZABLE
bit that will be ignored.
Now after my fix RESIZABLE bit along with the other bits won’t be set
if the window is not focusable. Bits set on the window in this case
was 0000110110000010 (the 9th bit is not set)
The bit setting is done in the native method
Java_sun_lwawt_macosx_CPlatformWindow_nativeSetNSWindowStyleBits.
Thanks,
Manajit
On 26-Feb-2018, at 1:28 PM, Manajit Halder
<[email protected] <mailto:[email protected]>> wrote:
Hi Phil and Kevin,
Sorry for specifying wrong JDK version. This review is for JDK 11.
Regarding spec:
It is not mentioned in the focus spec whether the unfocusable
maximized frames should be resizable or not.
The fix is given based on the implementation on other platforms
(windows and linux/unix) . On both the platforms (windows and
unix/linux) the test passes whereas the test fails on Mac.
An old Issue https://bugs.openjdk.java.net/browse/JDK-4980161 is
related to this fix.
Regarding fix:
The test case
java/awt/Frame/UnfocusableMaximizedFrameResizablity/UnfocusableMaximizedFrameResizablity.java
fails on Mac and passes on windows and linux/unix.
As per the test case when setFocusableWindowState is set to false
frame should not be resizable.
updateFocusableWindowState() updates the window focusable state and
the fix under review sets RESIZABLE property to true or false based
on focusability of the frame.
Regards,
Manajit
webrev appears to be against the correct repo :
sh://hg.openjdk.java.net/jdk/client
First can you point to what in the spec. or whatever points to
the correct desired behaviour being what you describe :
>If the frame is focusable then it is resizable otherwise not.
And regarding the fix, it isn't obvious how adding RESIZABLE
changes anything here
for the unfocusable case which seems to be the situation that is
under discussion.
Before: if its unfocusable, then setStyleBits will set the mask to 0 :
// this is the counter-point to -[CWindow _nativeSetStyleBit:]
private void setStyleBits(final int mask, final boolean value) {
execute(ptr -> nativeSetNSWindowStyleBits(ptr, mask, value
? mask : 0));
}
After: .. well exactly the same thing .. you just added a RESIZABLE
bit that will be ignored.
It may make a difference in the isFocusable case .. but that does
not match what you
say you are doing.
-phil.
On 02/16/2018 08:17 AM, Kevin Rushforth wrote:
> Kindly review the fix for JDK10.
You mean JDK 11, right?
-- Kevin
Manajit Halder wrote:
Hi All,
Kindly review the fix for JDK10.
Bug:
https://bugs.openjdk.java.net/browse/JDK-7158623
Webrev:
http://cr.openjdk.java.net/~mhalder/7158623/webrev.00/
<http://cr.openjdk.java.net/%7Emhalder/7158623/webrev.00/>
Issue:
Unfocusable frame was resizable.
Fix:
After the fix resizability of the frame depends on focusability
of the frame. If the frame is focusable then it is resizable
otherwise not.
The test code was cleaned to remove old test machinery code.
Regards,
Manajit
