1704102 - On 10.14 and 10.15, native context menus don't adopt Dark Mode unless widget.macos.respect-system-appearance is set

Status: VERIFIED FIXED

(Core :: Widget: Cocoa, task, P2)

Description

[Markus Stange [:mstange]]

This was fixed for macOS 11 in bug 1702100, but it uses an API that's not available before macOS 11.

Steps to reproduce:

1. Be on macOS 10.14 or 10.15.
2. Enable System Dark Mode.
3. Open a context menu.

Expected results:
The menu should have a dark appearance.

Actual results:
The menu uses the light appearance.

Enabling widget.macos.respect-system-appearance (bug 1623686) fixes this, but causes other bugs (that's why it's not enabled by default).

It would be great if we could find a way to make this work without widget.macos.respect-system-appearance.

Comment 1

[Markus Stange [:mstange]]

The problem here is that NSMenu gets its appearance from the NSWindow that it is opened on. With widget.macos.respect-system-appearance set to false (the current default), we always override the NSWindow appearance to be the light appearance. So the menu inherits the light appearance.
On macOS 11, there is an API to override the NSMenu appearance, and we use that API (since bug 1702100). But this API does not exist on 10.14 / 10.15.

One approach that could work is that we could briefly set the NSWindow appearance to the system appearance, open the menu, and immediately set it to the light appearance again. This seems to work without creating flashing effects, if the reset happens in menuWillOpen.

Comment 2

[Markus Stange [:mstange]]

I've tried to explore other options; for example, _NSSLMPopUpCarbonMenu3 (the internal function that opens the menu) has a flags parameter that causes it to obtain the appearance from the NSView instead of the NSWindow, but we have no control over the flags parameter - its caller, -[NSCarbonMenuImpl _popUpContextMenu:withEvent:forView:withFont:], always passes the "inherit from NSWindow" flag (4).

Comment 4

[Markus Stange [:mstange]]

Attachment: Bug 1704102 - On 10.14 and 10.15, make NSMenu adopt the system appearance by briefly changing the NSWindow appearance during menu opening. r=harry

Comment 5

[Markus Stange [:mstange]]

Martin, could you test the try build once it's ready? https://treeherder.mozilla.org/jobs?repo=try&revision=9e1a11d3316ec3b3af69de9c3124fa464d00e1e5

Comment 6

[Martin Balfanz [:mbalfanz]]

Thanks Markus! I tested the menus on 10.15.7 and it works like a charm 🥳

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/30df58ef67fa
On 10.14 and 10.15, make NSMenu adopt the system appearance by briefly changing the NSWindow appearance during menu opening. r=harry

Comment 8

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/30df58ef67fa

Comment 9

[Markus Stange [:mstange]]

Comment on attachment 9219644 [details]
Bug 1704102 - On 10.14 and 10.15, make NSMenu adopt the system appearance by briefly changing the NSWindow appearance during menu opening. r=harry

Beta/Release Uplift Approval Request

• User impact if declined: Light context menus in system dark mode for users on 10.14 + 10.15 - this is not a regression, but it's a common complaint with native context menus.
  Taking this patch makes the uplift for bug 1705120 easier; otherwise bug 1705120 will need to be rebased.
• Is this code covered by automated tests?: No
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: see comment 0
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): The patch has baked for a week and no problems have been found, and opening context menus is a very common operation. If this patch does turn out to be problematic, it can be turned off with the pref widget.macos.enable-pre-bigsur-workaround-for-dark-mode-context-menus. I don't anticipate any problems, but it is a bit of a hack, so it's not 100% impossible that problems could be found.
• String changes made/needed:

Comment 10

[Raluca Popovici, Desktop QA]

Verified - Fixed in latest Nightly 90.0a1 using macOS 10.14, 10.15 and 11. The context menu uses the system dark mode.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9219644 [details]
Bug 1704102 - On 10.14 and 10.15, make NSMenu adopt the system appearance by briefly changing the NSWindow appearance during menu opening. r=harry

Makes it easier to uplift bug 1705120. Approved for 89.0b12.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/46b61a1f3a5a

Comment 13

[Raluca Popovici, Desktop QA]

Verified - Fixed in latest Beta 89.0b12 using macOS 10.14, 10.15 and 11. The context menu uses the system dark mode.