RFR: 8359108: Mac - When Swing starts First, native application menu doesn't work for JavaFX [v3]

Fri Oct 10 17:32:19 UTC 2025

On Tue, 30 Sep 2025 23:31:20 GMT, Pabulaner IV <duke at openjdk.org> wrote:

>> This pull request fixes the system menu bar on MacOS when combining windows of Swing and JavaFX.
>> 
>> # Behavior before
>> 
>> If for some reason You needed to initialize AWT before JavaFX and You wanted to install the system menu bar from the JavaFX side, this wasn't possible. This issue is persistent even when You didn't open a Swing window. 
>> 
>> One scenario where this kind of issue happens is when You use install4j (see https://www.ej-technologies.com/install4j). In this case AWT is initialized by install4j and therefore You can't use the JavaFX system menu bar anymore.
>> 
>> 
>> # Behavior after
>> 
>> The fix allows JavaFX to install a system menu bar even if it is initialized after AWT. This is achieved by only changing code inside JavaFX. Each JavaFX window stores the previously installed menu bar when gaining focus and will restore this menu bar if the focus was lost. This only happens if the system menu bar installed by the JavaFX window is still unchanged.
>> 
>> 
>> # Tests
>> 
>> This PR introduces tests for the system menu bar in addition to verifying its own behavior / changes. The tests include single- and multi-window tests while interacting with Swing. The tests ensure that the menu bar stays the same for each window, no matter how You switch focus between them.
>> 
>> 
>> # Additional benifits 
>> 
>> This fix is not specifically for AWT, but allows JavaFX to interact much more compatibly with other frameworks that make use of the system menu bar.
>> 
>> 
>> # Review from AWT
>> 
>> In the previous PR related to this one, the comment was made that the folks from AWT should take a look at this fix. It would be great and much appreciated if someone could initiate it.
>> 
>> 
>> # Add disable flag?
>> 
>> We could also add a flag to prevent JavaFX from installing a system menu bar for users who have found other fixes for their projects / setups. This could be used to restore the previous behavior when AWT is initialized first.
>> 
>> 
>> Co-Author: @FlorianKirmaier
>
> Pabulaner IV has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - 8359108: Mac - When Swing starts First, native application menu doesn't work for JavaFX
>  - 8359108: Mac - When Swing starts First, native application menu doesn't work for JavaFX

This is a very preliminary review. I haven't run it yet, but I will soon. I left a few inline comments of things I happened to spot.

The fix itself looks good to me. 

The test framework and set of tests looks fine on first glance, but I haven't looked at them in detail. I will do so, along with running them

modules/javafx.graphics/src/main/native-glass/mac/GlassWindow.h line 70:

> 68:     // This is used to allow JFX to install its own system menu
> 69:     // without interfering with the hosting application.
> 70:     NSMenu* hostMenu;

Minor: align the field name to match the rest of the fields in this file

tests/system/src/test/java/test/javafx/stage/MacOSSystemMenuJFXPanelSwingFirstTest.java line 2:

> 1: /*
> 2:  * Copyright (c) 2017, 2024, Oracle and/or its affiliates. All rights reserved.

Replace `2024` with `2025`. Also, unless this file was substantially copied from an existing file (which it doesn't look like it was), remove the `2017,`.

This applies to all of the new files.

tests/system/src/test/java/test/javafx/stage/MacOSSystemMenuJFXPanelSwingFirstTest.java line 25:

> 23:  * questions.
> 24:  */
> 25: package test.javafx.stage;

I recommend moving these tests to a new package, perhaps `test.com.sun.glass.ui.mac`, since this is testing macOS glass functionality.

Or maybe, `test.robot...`, since even though they aren't robot tests, they rely on Window focus working.

tests/system/src/test/java/test/javafx/stage/MacOSSystemMenuTestBase.java line 1:

> 1: package test.javafx.stage;

This needs a copyright header block.

tests/system/src/test/java/test/javafx/stage/MacOSSystemMenuTestBase.java line 32:

> 30: import javax.swing.SwingUtilities;
> 31: 
> 32: import org.junit.jupiter.api.Test;

This is an unused import and should be removed. Especially since if there ever were a method in this class annotated with `@Test` it wouldn't work correctly since the tests in this test group are written so that there can be no more than one `@Test` in a test class, including its superclasses.

tests/system/src/test/java/test/javafx/stage/MacOSSystemMenuTestBase.java line 146:

> 144:     /// Helpers for creation and focusing of windows
> 145:     ///
> 146:     /////////////////////////////////////////////////////////

Using `///` is reserved for markdown style javadoc comments. Replace this with something else.

tests/system/src/test/java/test/javafx/stage/MacOSSystemMenuTestBase.java line 406:

> 404:     /// Helpers for synchronization
> 405:     ///
> 406:     /////////////////////////////////////////////////////////

Replace `///` with something else.

tests/system/src/test/java/test/javafx/stage/MacOSSystemMenuTestBase.java line 422:

> 420:     }
> 421: 
> 422:     protected void sleep(long millis) {

This is unused, and duplicates the functionality of `Util.sleep` anyway. I recommend to remove it.

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

PR Review: https://git.openjdk.org/jfx/pull/1904#pullrequestreview-3324402471
PR Review Comment: https://git.openjdk.org/jfx/pull/1904#discussion_r2420935478
PR Review Comment: https://git.openjdk.org/jfx/pull/1904#discussion_r2420944384
PR Review Comment: https://git.openjdk.org/jfx/pull/1904#discussion_r2421005431
PR Review Comment: https://git.openjdk.org/jfx/pull/1904#discussion_r2421290204
PR Review Comment: https://git.openjdk.org/jfx/pull/1904#discussion_r2421333115
PR Review Comment: https://git.openjdk.org/jfx/pull/1904#discussion_r2421304289
PR Review Comment: https://git.openjdk.org/jfx/pull/1904#discussion_r2421321910
PR Review Comment: https://git.openjdk.org/jfx/pull/1904#discussion_r2421314423