Closed Bug 1704102 Opened 3 years ago Closed 3 years ago

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

Categories

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

All
macOS
task

Tracking

()

VERIFIED FIXED
90 Branch
Tracking Status
firefox89 --- verified
firefox90 --- verified

People

(Reporter: mstange, Assigned: mstange)

References

Details

(Whiteboard: [proton-context-menus][mac:mr1] [priority:2a] [proton-uplift])

Attachments

(1 file)

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.

Summary: Native context menu appearance is getting overridden by NSWindow appearance on 10.14 and 10.15, should get appearance from the system appearance instead → On 10.14 and 10.15, native context menus don't adopt Dark Mode unless widget.macos.respect-system-appearance is set

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.

Blocks: 34572
Whiteboard: [proton-context-menus][mac:mr1]

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).

Flags: needinfo?(mbalfanz)

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

Flags: needinfo?(mbalfanz)
Priority: -- → P2
Whiteboard: [proton-context-menus][mac:mr1] → [proton-context-menus][mac:mr1] [priority:2a]
Pushed by mstange@themasta.com:
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
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

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:
Attachment #9219644 - Flags: approval-mozilla-beta?
Flags: qe-verify+
QA Whiteboard: [qa-triaged]

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

Status: RESOLVED → VERIFIED
QA Whiteboard: [qa-triaged]
Flags: qe-verify+

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.

Attachment #9219644 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

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

Whiteboard: [proton-context-menus][mac:mr1] [priority:2a] → [proton-context-menus][mac:mr1] [priority:2a] [proton-uplift]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: