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

Alexey Ivanov aivanov at openjdk.org
Fri Dec 13 12:50:39 UTC 2024


On Fri, 13 Dec 2024 04:16:06 GMT, Alexander Zvegintsev <azvegint 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.

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?

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.

test/jdk/javax/swing/JPopupMenu/NestedFocusablePopupTest.java line 80:

> 78:             if (!popupMenu.isVisible()) {
> 79:                 throw new RuntimeException("Popup is not visible");
> 80:             }

Perhaps, you should call `popupMenu.isVisible()` on EDT?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22729#discussion_r1883879389
PR Review Comment: https://git.openjdk.org/jdk/pull/22729#discussion_r1883871284
PR Review Comment: https://git.openjdk.org/jdk/pull/22729#discussion_r1883884693
PR Review Comment: https://git.openjdk.org/jdk/pull/22729#discussion_r1883889144


More information about the client-libs-dev mailing list