On Fri, 31 May 2024 12:24:42 GMT, Prasanta Sadhukhan <psadhuk...@openjdk.org> 
wrote:

>> Issue is in macosx, when a JMenu or JPopupmenu is opened and then window is 
>> resized from the lower right corner, then the Menu / JPopupmenu stays open 
>> unlike in native osx apps like Notes, Mail etc..
>> 
>> This is because when LMouseButton is pressed on non-client area, the window 
>> should get a UngrabEvent for it to close/cancel the popupmenu (as is done in 
>> [windows](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/windows/native/libawt/windows/awt_Frame.cpp#L618-L620))
>>  but it was seen that the mac AWTWindow code only recognizes the title-bar 
>> as the non-client area so 
>> [notifyNCMouseDown](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/macosx/classes/sun/lwawt/LWWindowPeer.java#L797-L804)
>>  is not called.
>> Fix is made to recognize the edges of the window as non-client area
>
> Prasanta Sadhukhan has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Non-client aread edge area modified, test format

src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m line 1031:

> 1029:                 (p.x >= (frame.origin.x + contentRect.size.width - 3)) 
> ||
> 1030:                 (fabs(frame.origin.x - p.x) < 3) ||
> 1031:                 (fabs(frame.origin.y - p.y) < 3)) {

Does `fabs` use float? It makes code more compact but it may be imprecise and 
less performant than integer arithmetics.

test/jdk/javax/swing/JMenu/TestUngrab.java line 82:

> 80:             robot.waitForIdle();
> 81:             robot.delay(1000);
> 82:             SwingUtilities.invokeAndWait(() -> {

Suggestion:

            robot.delay(1000);

            SwingUtilities.invokeAndWait(() -> {


Adding blank lines between sections of code could improve its readability.

test/jdk/javax/swing/JMenu/TestUngrab.java line 101:

> 99:             robot.delay(1000);
> 100:             System.out.println("isPopupMenuVisible " + 
> menu.isPopupMenuVisible());
> 101:             if (menu.isPopupMenuVisible()) {

`menu.isPopupMenuVisible()` should also be accessed on EDT.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1622540182
PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1622542519
PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1622533420

Reply via email to