On Mon, 16 Mar 2026 10:45:52 GMT, Ambarish Rapte <[email protected]> wrote:
>> This PR fixes an issue on macOS with a system menu bar, that happens when a >> dialog is shown while a menu from the system menu bar is also showing. >> >> Currently the menubar is removed whenever the stage loses focus, but that >> causes a broken state in the menu (as reported in >> https://bugs.openjdk.org/browse/JDK-8335541) as the dialog showing prevents >> for a clean tear down of the menubar. And even if there were no issues, >> there is actually no need of destroying and recreating the same menubar all >> over again, since nothing really changed (the dialog doesn't have a menubar >> on its own, and shares the very same one from its owner stage). >> >> A test has been added, that fails before this patch and passes with it. >> Another minor test has been included, just to give some details about what >> happens when a menu titled exactly `"Help"` is added to the system menu bar. >> It works before and after the patch. >> >> While adding the test, I noticed that there were failures when the menu was >> hidden: >> >> SystemMenuBarHelpMenuTest STANDARD_ERROR >> Exception in thread "JavaFX Application Thread" >> java.lang.IllegalStateException: Not on FX application thread; currentThread >> = JavaFX Application Thread >> at >> javafx.graphics@27-internal/com.sun.javafx.tk.Toolkit.checkFxUserThread(Toolkit.java:282) >> at >> javafx.graphics@27-internal/com.sun.javafx.tk.quantum.QuantumToolkit.checkFxUserThread(QuantumToolkit.java:480) >> at >> javafx.controls@27-internal/javafx.scene.control.Menu.hide(Menu.java:428) >> at >> javafx.graphics@27-internal/com.sun.javafx.tk.quantum.GlassMenuEventHandler.handleMenuClosed(GlassMenuEventHandler.java:46) >> at >> javafx.graphics@27-internal/com.sun.glass.ui.Menu.notifyMenuClosed(Menu.java:196) >> >> so I wrapped the calls in `Menu::notifyMenuClosed` and >> `Menu::notifyMenuClosed` with ` Utils::runOnFxThread`. > > modules/javafx.graphics/src/main/java/com/sun/glass/ui/Menu.java line 187: > >> 185: protected void notifyMenuOpening() { >> 186: if (this.eventHandler != null) { >> 187: Utils.runOnFxThread(() -> >> eventHandler.handleMenuOpening(this, System.nanoTime())); > > These methods `notifyMenuOpening` and `notifyMenuClosed` are already > executing on JavaFX Application Thread. So, the calling > `Utils.runOnFxThread()` does not defer the eventHandler call for later, but > runs immediately. > The fix behavior does not change when these two changes are reverted. Could > you please recheck. As I mentioned in the description of this PR, running the test with sh gradlew --continue --info -PFULL_TEST=true -PUSE_ROBOT=true :systemTests:cleanTest :systemTests:test --tests "test.robot.javafx.scene.SystemMenuBarHelpMenuTest" the test passes, but I got: SystemMenuBarHelpMenuTest STANDARD_ERROR Exception in thread "JavaFX Application Thread" java.lang.IllegalStateException: Not on FX application thread; currentThread = JavaFX Application Thread at javafx.graphics@27-internal/com.sun.javafx.tk.Toolkit.checkFxUserThread(Toolkit.java:282) at javafx.graphics@27-internal/com.sun.javafx.tk.quantum.QuantumToolkit.checkFxUserThread(QuantumToolkit.java:480) at javafx.controls@27-internal/javafx.scene.control.Menu.hide(Menu.java:428) at javafx.graphics@27-internal/com.sun.javafx.tk.quantum.GlassMenuEventHandler.handleMenuClosed(GlassMenuEventHandler.java:46) at javafx.graphics@27-internal/com.sun.glass.ui.Menu.notifyMenuClosed(Menu.java:196) Which is really weird, because `Not on FX application thread; currentThread = JavaFX Application Thread` is contradictory. Also weird that using `Utils::runOnFxThread` bypasses this check. In any case, with some debug, I can see that `Menu::hide` calls `Toolkit.getToolkit().checkFxUserThread();`, which checks `Thread.currentThread() == fxUserThread`, else throws `Not on FX application thread; currentThread: <Thread.currentThread().getName()>`. So my current thread is indeed the JavaFX thread, but `fxUserThread` is not, it is actually `null`. When running the system test, `fxUserThread` gets nullified in the exit call from `Toolkit.getToolkit().exit();` that is invoked from `@AfterAll -> Utils.shutdown -> Platform.exit()`, before the cleanup at the end of the tests, that doesn' have time enough to finish the menu hiding animation. I'll revert the change in Menu, and add more time in the test. ------------- PR Review Comment: https://git.openjdk.org/jfx/pull/2107#discussion_r2939681391
