8372398: [macOS] Shown ContextMenu does not close when macOS system menu is clicked by jperedadnr · Pull Request #2102 · openjdk/jfx

jperedadnr · 2026-03-09T19:38:15Z

This PR adds a fix to close the popup windows on macOS when clicking over the system menu bar.

According to the macOS standard behaviour for native applications, when a popup window is showing, and the user clicks on the system menu bar, the click event is consumed and the popup is closed. A second click is then required to open the system menu bar.

This is done by the popup windows directly, as they are NSMenu objects that enable a modal event tracking loop, capturing all events including those from the system menu bar, in order to dismiss the popup when the click is outside the window.

However, JavaFX just implements regular NSWindows, and there is no such event loop. Therefore, this PR adds a notification to the system menu instead. When the menu is about to open, the popup window gets a notification, which is processed to cancel the menu animation, preventing it from showing up, and also closing the popup. And then, with the popup closed, a new click from the user will open the system menus as usual.

This applies to all JavaFX menus from the system menu bar, that is: the application menu (the "java" menu when the application is not packaged yet) and other menus created by the application, if any.

It doesn't apply to the Apple system menu, which is not handle by JavaFX, though. This causes a difference with native applications, that treat such menu in the same way.

I confirm that I make this contribution in accordance with the OpenJDK Interim AI Policy.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

JDK-8372398: [macOS] Shown ContextMenu does not close when macOS system menu is clicked (Bug - P3)

Reviewers

Martin Fox (@beldenfox - Committer)
Ambarish Rapte (@arapte - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx.git pull/2102/head:pull/2102
$ git checkout pull/2102

Update a local copy of the PR:
$ git checkout pull/2102
$ git pull https://git.openjdk.org/jfx.git pull/2102/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2102

View PR using the GUI difftool:
$ git pr show -t 2102

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/2102.diff

Using Webrev

Link to Webrev Comment

bridgekeeper · 2026-03-09T19:39:06Z

👋 Welcome back jpereda! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2026-03-09T19:40:11Z

@jperedadnr This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8372398: [macOS] Shown ContextMenu does not close when macOS system menu is clicked

Reviewed-by: mfox, arapte

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 79 new commits pushed to the master branch:

3306add: 8387020: StyleManagerTest#testSetApplicationUserAgentStylesheetFromDataURI does not correctly reset the Application user agent stylesheet
9258f07: 8356042: RichTextArea: tab stops attributes
704f4f1: 8384814: [macOS] Trackpad scroll gestures suddenly stop
... and 76 more: https://git.openjdk.org/jfx/compare/7b845519f4776266abca09dbdd5a2b22425e4003...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

mlbridge · 2026-03-09T19:43:06Z

Webrevs

Copilot

Pull request overview

This PR aims to make JavaFX popup windows on macOS dismiss when the user clicks the system menu bar (so the first click closes the popup and a second click is needed to open the menu), matching typical native macOS behavior.

Changes:

Post a new notification from GlassMenu right before a menu starts opening.
Register popup windows to observe that notification.
When observed, attempt to cancel menu tracking and dismiss the popup.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File	Description
modules/javafx.graphics/src/main/native-glass/mac/GlassWindow.m	Registers popup windows as observers of a new “menubar will open” notification.
modules/javafx.graphics/src/main/native-glass/mac/GlassWindow+Overrides.m	Adds the notification handler intended to cancel menu opening and dismiss the popup; removes the observer on close.
modules/javafx.graphics/src/main/native-glass/mac/GlassMenu.m	Posts the notification from `menuWillOpen:` before running existing Java callbacks.
modules/javafx.graphics/src/main/native-glass/mac/GlassMenu.h	Introduces the notification name constant.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-09T19:47:29Z

+    // Close the popup window
+    [self->nsWindow close];
+}


Calling [self->nsWindow close] here will destroy the Glass window (notifyDestroy sets ptr=0) rather than just dismissing/hiding the popup, which can break popup reuse (PopupStage typically toggles visibility instead of closing). It also bypasses the existing deferred-close logic in MacWindow._close (performSelectorOnMainThread) that avoids AppKit reentrancy crashes. Prefer dismissing the popup via the existing ungrab/hide path (e.g., _ungrabFocus + orderOut / triggering the Java-side hide) rather than closing the NSWindow directly.

beldenfox · 2026-03-12T15:56:51Z

I ran into some issues with this PR. You can reproduce using the manual PopupControlTest or any app that has a standard (non-system) menu bar. Click on "Menu" to show the menu's popup, click on the system menu bar to dismiss it, and then click on "Menu" again. The popup won't appear.

I debugged this a bit. To track this sort of dismissal the Menu control relies on its popup window going through the auto-hide logic (I think auto-hide sets a transient property that Menu monitors to detect when the popup goes away). The auto-hide logic is normally triggered by a focus ungrab event. It looks like there's a similar case in the glass code already; when the user clicks on a window's title bar glass calls _ungrabFocus to ensure auto-hiding popups are closed (see sendEvent: in GlassWindow.m).

And do we have a policy on Copilot? In this case I think it got something half-right but also got several things wrong. I don't like leaving misinformation lying around in a PR but also don't want to spend time correcting an AI.

jperedadnr · 2026-03-13T17:32:02Z

Thanks, @beldenfox, I can reproduce the issue with PopupControlTest. I'll have a look.

jperedadnr · 2026-03-13T19:16:29Z

I've just pushed a change, that should fix the issue in PopupControlTest.

beldenfox · 2026-03-24T16:18:33Z

+
+    // For non-auto-hide popups, jWindowNotifyClose fires WINDOW_CLOSE_REQUEST,
+    // and that calls hide() on the popup.
+    if ([self->nsWindow isVisible]) {


Should non-auto-hide popups be hidden? The spec doesn't talk about them much and I'm not sure what they're used for.

That's a very good question, indeed.

I'm testing a native Xcode project, and a NSPanel by default is non-auto-hide. For instance, Finder -> Get Info opens an auxiliary window, and it won't close when opening the system menu bar or clicking elsewhere.

However, Finder -> right click on a file-> Context Menu is auto-hide (hides when clicking the system menu bar or elsewhere).

The main difference is that the native ContextMenu window is actually a NSMenu, while in JavaFX we use NSPanel for all popups, and we install event handlers to treat it as auto-hide or non-auto-hide.

With this PR as is, the non-auto-hide popups should be closed when clicking over the system menu bar, but now I see that PopupWindowPeerListener::closing overrides WindowPeerListener::closing with an empty implementation, and WINDOW_CLOSE_REQUEST is not fired at all...

So my call to jWindowNotifyClose in this case is wrong (it doesn't do anything).

And while the non-auto-hide popup won't close (okay), the click on the system menu bar event is consumed and there is no way to show it while the popup is showing (wrong).

I'll rework this.

@beldenfox It should be fixed now.

kevinrushforth · 2026-04-03T20:41:57Z

/reviewers 2

openjdk · 2026-04-03T20:42:18Z

@kevinrushforth
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

beldenfox · 2026-04-07T16:23:35Z

+    // If this popup is not visible or is a non-auto-hide popup, return early, without cancelling the menu event
+    // or hiding the popup.
+    // Note that only auto-hide popups have an active focus grab.
+    if (![self->nsWindow isVisible] || ![GlassWindow _hasGrab]) {


I'm not sure you need the isVisible check though there's no harm in leaving it in. As far as I know there should never be an invisible popup; when JavaFX hides the popup the NSWindow is destroyed, not hidden.

If we remove the notification (as discussed in the other comment), this code would be removed.

If we keep it, the visible check is indeed not needed, since there can be only one auto-hide popup with an active grab (and it will be visible), which is the one that should cancel tracking and reset the grab.

Note that I had only the visible check before I added the grab one.

jperedadnr · 2026-04-10T16:27:03Z

/template append

openjdk · 2026-04-10T16:27:56Z

@jperedadnr The pull request template has been appended to the pull request body

beldenfox

LGTM and my manual testing all passed.

bridgekeeper · 2026-05-14T15:33:13Z

@jperedadnr This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

jperedadnr · 2026-05-14T15:34:31Z

/keepalive

openjdk · 2026-05-14T15:35:36Z

@jperedadnr The pull request is being re-evaluated and the inactivity timeout has been reset.

bridgekeeper · 2026-06-11T19:07:06Z

@jperedadnr This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

jperedadnr · 2026-06-12T09:42:26Z

/keepalive

openjdk · 2026-06-12T09:43:09Z

@jperedadnr The pull request is being re-evaluated and the inactivity timeout has been reset.

arapte

Sanity testing looks good to me. Tested several scenarios with ChoiceBox, ComboBox, DatePicker, ColorPicker
Could you please add a automated headful system test.

jperedadnr · 2026-06-22T16:27:41Z

@arapte I've added a system test, with a couple of tests: one with auto-hide popups, that fails before this PR and passes after it, and one with non-auto-hide popups that passes before and after this PR (so behaviour doesn't change).

beldenfox

New test fails in master branch and succeeds in this PR. LGTM.

arapte

lgtm

jperedadnr · 2026-06-25T18:49:42Z

/integrate

openjdk · 2026-06-25T18:50:44Z

Going to push as commit e84b145.
Since your change was applied there have been 79 commits pushed to the master branch:

3306add: 8387020: StyleManagerTest#testSetApplicationUserAgentStylesheetFromDataURI does not correctly reset the Application user agent stylesheet
9258f07: 8356042: RichTextArea: tab stops attributes
704f4f1: 8384814: [macOS] Trackpad scroll gestures suddenly stop
... and 76 more: https://git.openjdk.org/jfx/compare/7b845519f4776266abca09dbdd5a2b22425e4003...master

Your commit was automatically rebased without conflicts.

openjdk · 2026-06-25T18:50:52Z

@jperedadnr Pushed as commit e84b145.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Close popup windows on macOS when clicking over the system menu bar

46ff026

Copilot AI review requested due to automatic review settings March 9, 2026 19:38

Copilot started reviewing on behalf of jperedadnr March 9, 2026 19:39 View session

openjdk Bot added the rfr Ready for review label Mar 9, 2026

Copilot AI reviewed Mar 9, 2026

View reviewed changes

Address feedback

c870109

beldenfox reviewed Mar 24, 2026

View reviewed changes

Rework menuWillOpenHandler to apply it only to auto-hide popups

6b86e15

kevinrushforth self-requested a review April 3, 2026 21:37

beldenfox reviewed Apr 7, 2026

View reviewed changes

jperedadnr added 3 commits April 8, 2026 19:14

Remove notification and check if there is a grab instead

a31a617

undo unneeded changes

2f114df

undo unneeded changes

37d54b0

openjdk Bot added rfr Ready for review and removed rfr Ready for review labels Apr 10, 2026

beldenfox approved these changes Apr 16, 2026

View reviewed changes

arapte suggested changes Jun 22, 2026

View reviewed changes

Add system test

88f0036

beldenfox approved these changes Jun 23, 2026

View reviewed changes

arapte approved these changes Jun 25, 2026

View reviewed changes

openjdk Bot added the ready Ready to be integrated label Jun 25, 2026

openjdk Bot added the integrated Pull request has been integrated label Jun 25, 2026

openjdk Bot closed this Jun 25, 2026

openjdk Bot removed ready Ready to be integrated rfr Ready for review labels Jun 25, 2026

Uh oh!

Conversation

jperedadnr commented Mar 9, 2026 • edited by openjdk Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewers

Reviewing

Uh oh!

bridgekeeper Bot commented Mar 9, 2026

Uh oh!

openjdk Bot commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mlbridge Bot commented Mar 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Webrevs

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

Uh oh!

beldenfox commented Mar 12, 2026

Uh oh!

jperedadnr commented Mar 13, 2026

Uh oh!

jperedadnr commented Mar 13, 2026

Uh oh!

beldenfox Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

jperedadnr Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

jperedadnr Mar 25, 2026

Choose a reason for hiding this comment

Uh oh!

kevinrushforth commented Apr 3, 2026

Uh oh!

openjdk Bot commented Apr 3, 2026

Uh oh!

Uh oh!

beldenfox Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

jperedadnr Apr 8, 2026

Choose a reason for hiding this comment

Uh oh!

jperedadnr commented Apr 10, 2026

Uh oh!

openjdk Bot commented Apr 10, 2026

Uh oh!

beldenfox left a comment

Choose a reason for hiding this comment

Uh oh!

bridgekeeper Bot commented May 14, 2026

Uh oh!

jperedadnr commented May 14, 2026

Uh oh!

openjdk Bot commented May 14, 2026

Uh oh!

bridgekeeper Bot commented Jun 11, 2026

Uh oh!

jperedadnr commented Jun 12, 2026

Uh oh!

openjdk Bot commented Jun 12, 2026

Uh oh!

arapte left a comment

Choose a reason for hiding this comment

Uh oh!

jperedadnr commented Jun 22, 2026

Uh oh!

beldenfox left a comment

Choose a reason for hiding this comment

Uh oh!

jperedadnr commented Mar 9, 2026 •

edited by openjdk Bot

Loading

openjdk Bot commented Mar 9, 2026 •

edited

Loading

mlbridge Bot commented Mar 9, 2026 •

edited

Loading