RFR: 8342096: Popup menus that request focus are not shown on Linux with Wayland

Alexander Zvegintsev azvegint at openjdk.org
Fri Dec 13 22:19:36 UTC 2024


On Fri, 13 Dec 2024 12:39:02 GMT, Alexey Ivanov <aivanov at openjdk.org> wrote:

>> The previous [JDK-8319103](https://bugs.openjdk.org/browse/JDK-8319103) fix was not complete.
>> 
>> The case where a menu item with a focusable component was not a direct child of a window was missing(failing the `if (window == oppositeWindow.getParent() ) {` check), so the ungrab event was posted prematurely.
>> 
>> This can be fixed by adding `waylandWindowFocusListener` to all submenus in hierarchy.
>> 
>> The manual test updated to use this case, and also added an automated test that checks that it didn't close prematurely.
>
> src/java.desktop/unix/classes/sun/awt/UNIXToolkit.java line 532:
> 
>> 530:             }
>> 531:         }
>> 532:         return false;
> 
> If you like, you may use Stream API:
> Suggestion:
> 
>         return Arrays.stream(window.getWindowFocusListeners())
>                      .anyMatch(fl -> fl == waylandWindowFocusListener);
> 
> I haven't tested the code though.

I didn't use the stream API on purpose to be a bit faster, since we're usually working with very small arrays (usually about 0-1 elements).

> src/java.desktop/unix/classes/sun/awt/UNIXToolkit.java line 550:
> 
>> 548:                             addWaylandWindowFocusListenerToWindow(oppositeWindow);
>> 549:                             return;
>> 550:                         }
> 
> So it's possible now that several windows in a hierarchy will have the listener added… Won't the listener be leaked? In other words, do we always remove the listener?

As far as my tests with an instrumented build show - yes, but I discovered another case not covered:


for (int i = 0; i <= 1; i++) {
    JMenu menu = new JMenu("Menu " + i);
    menu.add(new JTextField("JTextField " + i));
    popupMenu.add(menu);
}


Clicking on gnome text editor does not hide the menu if the second menu is open. I do have a fix for it, but will not rush to post it, as I do not have enough time to test it thoroughly.
I will move this PR to draft status as I will be on vacation starting next week.

> test/jdk/javax/swing/JPopupMenu/NestedFocusablePopupTest.java line 53:
> 
>> 51:             //test is valid only when running on Wayland.
>> 52:             return;
>> 53:         }
> 
> I think it should rather throw `jtreg.SkippedException`.
> 
> With [jtreg enhancements](https://github.com/openjdk/jtreg/pull/217) and further reporting enhancements, we'll be able to see if tests aren't actually executed.

Ok, but this test will lose the ability to run standalone with just `java NestedFocusablePopupTest.java`.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22729#discussion_r1884600821
PR Review Comment: https://git.openjdk.org/jdk/pull/22729#discussion_r1884598134
PR Review Comment: https://git.openjdk.org/jdk/pull/22729#discussion_r1884599302


More information about the client-libs-dev mailing list