On Mon, 16 Mar 2026 11:23:00 GMT, Jose Pereda <[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`. > > Jose Pereda has updated the pull request incrementally with two additional > commits since the last revision: > > - undo change > - Address feedback It looks good to me. But shall request more eyes for the point raised by @beldenfox. @andy-goryachev-oracle Kindly request you to take a look at the PR and [the point](https://github.com/openjdk/jfx/pull/2107#issuecomment-4058834180) raised by @beldenfox /Reviewers 2 modules/javafx.controls/src/main/java/javafx/scene/control/skin/MenuBarSkin.java line 515: > 513: // If an owned dialog is shown, the owner stage should keep > it as it was. > 514: return; > 515: } minor recommendation : The if else block could be changed to : if (stage != null) { if (staged.isFocused()) { ... } else { return; } } ------------- PR Review: https://git.openjdk.org/jfx/pull/2107#pullrequestreview-3961365450 PR Review Comment: https://git.openjdk.org/jfx/pull/2107#discussion_r2947327232
