On Wed, 11 Mar 2026 20:14:56 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.
>
> Jose Pereda has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Address feedback

I just wanted to test a double-nested submenus - see

https://github.com/andy-goryachev-oracle/Test/blob/main/src/goryachev/bugs/SystemMenu_Disabled_8263959.java

Looks good!
One minor comment, but ok as is.

modules/javafx.graphics/src/main/java/com/sun/javafx/tk/quantum/GlassSystemMenu.java
 line 249:

> 247:             }
> 248:         } else {
> 249:             final MenuItem glassSubMenuItem = 
> app.createMenuItem(parseText(menuitem), null);

minor: `final` is not necessary for _effectively final_ declarations

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

> 88:     @Test
> 89:     void testMenuItemEnabledAfterParentReEnabled() {
> 90:         Assumptions.assumeTrue(PlatformUtil.isMac(), "System menu bar 
> tests only apply to macOS");

Unrelated question: this is the only `@Test` here - will the frame be displayed 
on Windows?  Or junit will not even run the test?

Can the whole test be skipped on non-macOS systems?

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

Marked as reviewed by angorya (Reviewer).

PR Review: https://git.openjdk.org/jfx/pull/2103#pullrequestreview-3938810769
PR Review Comment: https://git.openjdk.org/jfx/pull/2103#discussion_r2926551444
PR Review Comment: https://git.openjdk.org/jfx/pull/2103#discussion_r2926588445

Reply via email to