On Wed, 11 Mar 2026 00:46:22 GMT, Jose Pereda <[email protected]> wrote:

> This PR fixes https://bugs.openjdk.org/browse/JDK-8263959, an issue on macOS 
> that happens when a menu is disabled, and then enabled back again, where leaf 
> menuItems remain disabled unexpectedly, by re-syncing the native NSMenuItem 
> enabled state from Java menuItem enabled state. 
> 
> Explanation: We create the native NSMenuItems with `autoenablesItems:YES`, 
> which means the OS calls `GlassMenu::validateMenuItem:` on each item's target 
> to determine if it should be enabled. This calls the Java 
> `GlassSystemMenu::validate` callback, which updates the accelerator bindings, 
> but it doesn't update the enabled state:  It remains as it was (in this case, 
> disabled when the parent menu was disabled) as `[glassTargetItem->item 
> isEnabled]` returns the old state (NO/disabled) rather than the updated Java 
> state (`!menuitem.isDisable()`/enabled).
> 
> One possible and valid fix would be changing `autoenablesItems:YES` to 
> `autoenablesItems:NO`, as the Java layer already manages the enable state via 
> `GlassMenu::_setEnabled` (making `validateMenuItem:` redundant for this case).
> 
> However, the proposed fix doesn't change that, and simply syncs the native 
> side with the Java side while validation is being performed.
> 
> A system test has been included, if fails before this patch, passes after it.

Fix lgtm, provided a few comments for the test.

tests/system/src/test/java/test/robot/javafx/scene/SystemMenuBarEnableTest.java 
line 72:

> 70:     @BeforeAll
> 71:     static void initFX() throws Exception {
> 72:         Assumptions.assumeTrue(PlatformUtil.isMac(), "System menu bar 
> tests only apply to macOS");

This could cause test failure on other platforms. Should move to the `@test` 
method.

tests/system/src/test/java/test/robot/javafx/scene/SystemMenuBarEnableTest.java 
line 102:

> 100:             // Select the second item via keyboard
> 101:             Util.runAndWait(() -> {
> 102:                 Robot robot = new Robot();

`robot` can be a member variable of class `SystemMenuBarEnableTest`, 
initialized in `TestApp.start()` method and reused instead of re-creating.

tests/system/src/test/java/test/robot/javafx/scene/SystemMenuBarEnableTest.java 
line 105:

> 103:                 robot.keyType(KeyCode.DOWN);
> 104:                 robot.keyType(KeyCode.DOWN);
> 105:                 System.out.println("Pressed DOWN+DOWN+ENTER, menu should 
> have selected second item");

May be remove these print statements. the comment before the block are 
sufficient, and the prints are only noise in test report.

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

Changes requested by arapte (Reviewer).

PR Review: https://git.openjdk.org/jfx/pull/2103#pullrequestreview-3927627632
PR Review Comment: https://git.openjdk.org/jfx/pull/2103#discussion_r2916865681
PR Review Comment: https://git.openjdk.org/jfx/pull/2103#discussion_r2916583660
PR Review Comment: https://git.openjdk.org/jfx/pull/2103#discussion_r2916883014

Reply via email to