On Thu, 9 May 2024 19:48:19 GMT, Johan Vos <j...@openjdk.org> wrote: >> A listener was added but never removed. >> This patch removes the listener when the menu it links to is cleared. Fix >> for https://bugs.openjdk.org/browse/JDK-8319779 > > Johan Vos has updated the pull request incrementally with one additional > commit since the last revision: > > Add more type info
The new test passed with both the old and the new version (I reverted the code, aside from leaving `setMenuBindings` `protected`). Is the test testing the right thing? (If relevant, this was done on a Windows platform) tests/system/src/test/java/test/com/sun/javafx/tk/quantum/SystemMenuBarTest.java line 287: > 285: > 286: @Test > 287: public void testJDK8309935() { minor: Not sure what our policies are, but I find test names like this pretty useless, perhaps a short indication what this is doing? tests/system/src/test/java/test/com/sun/javafx/tk/quantum/SystemMenuBarTest.java line 346: > 344: }); > 345: Util.runAndWait(() -> { > 346: }); I checked the code that `runAndWait` would do when you pass in 0 runnables, and it basically does no waiting at all (less than a microsecond for sure). If this is really essential (I removed it and the test still passed) then I think a `sleep` might be better. ------------- Changes requested by jhendrikx (Reviewer). PR Review: https://git.openjdk.org/jfx/pull/1283#pullrequestreview-2114917800 PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1637684395 PR Review Comment: https://git.openjdk.org/jfx/pull/1283#discussion_r1637687030