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