Closed Bug 1593390 Opened 5 years ago Closed 4 years ago

Use NSApplication::effectiveAppearance and/or NSRequiresAquaSystemAppearance to detect dark mode instead of standardUserDefaults

Categories

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

defect

Tracking

()

RESOLVED FIXED
89 Branch
Tracking Status
firefox89 --- fixed

People

(Reporter: Gijs, Assigned: mstange)

References

Details

(Whiteboard: [mac:darkmode])

Attachments

(1 file, 1 obsolete file)

Filed on behalf of https://twitter.com/lapcatsoftware/status/1190006955068276748 . Quoting for posterity:

Firefox doesn't correctly check dark mode on Mac. The API is -[NSApplication effectiveAppearance]. Firefox uses SPI: https://searchfox.org/mozilla-central/rev/59de675101da711520c0bb6e34a1ea2372e7ddbb/widget/cocoa/nsLookAndFeel.mm#629,635
At the very least it should check NSRequiresAquaSystemAppearance along with AppleInterfaceStyle.
Some people use NSRequiresAquaSystemAppearance to have a dark menu bar and Dock: https://www.techjunkie.com/only-dark-menu-bar-dock-mojave/
This works correctly with -[NSApplication effectiveAppearance], but it doesn't work with just AppleInterfaceStyle.

I don't profess to understand this, but hopefully it makes sense to people working on cocoa things.

I have submitted a patch to phabricator that implements this. However, this is blocked by the fact that we're building against the 10.11 SDK. NSApplication's effectiveAppearance will always return a value of "light mode" when built against the 10.11 SDK. We will need to build against the 10.14 SDK (tracked in bug 1475652) before we can land this (or a similar) patch.

Depends on: mojave-sdk

Apparently it is possible to opt into dark mode even when linking against earlier SDKs:

If you build your app against an earlier SDK but still want to support Dark Mode, include the NSRequiresAquaSystemAppearance key (with a value of NO) in your app's Info.plist file. Do so only if your app's appearance looks correct when running in macOS 10.14 and later with Dark Mode enabled.
https://developer.apple.com/documentation/appkit/nsappearancecustomization/choosing_a_specific_appearance_for_your_macos_app?language=objc#2993818

(In reply to Markus Stange [:mstange] from comment #3)

Apparently it is possible to opt into dark mode even when linking against earlier SDKs:

Yes, I remember seeing this too and I just tested it out. Unfortunately, this causes other issues that we'll have to sort out before we could land this. For example, modal dialogs appear with a dark background with this key in the Info.plist. That makes our black text hard to read. A good example is the dialog that warns a user when they're closing Firefox with multiple tabs open.

I went ahead and updated the patch to include this key to make it easier for people to test it out.

(In reply to Stephen A Pohl [:spohl] from comment #4)

Yes, I remember seeing this too and I just tested it out. Unfortunately, this causes other issues that we'll have to sort out before we could land this. For example, modal dialogs appear with a dark background with this key in the Info.plist. That makes our black text hard to read. A good example is the dialog that warns a user when they're closing Firefox with multiple tabs open.

This appears to be much more nuanced than first thought: The modal dialog appears correctly until I switch from dark to light mode for the first time. After that, the colors are inverted intermittently: light text on light background in light mode and dark text on dark background in dark mode. The correct colors seem to restore sometimes when switching away from Firefox and back. This will need more investigation. The same issue can be seen with right-click context menus.

The priority flag is not set for this bug.
:spohl, could you have a look please?

For more information, please visit auto_nag documentation.

Flags: needinfo?(spohl.mozilla.bugs)
Flags: needinfo?(spohl.mozilla.bugs)
Priority: -- → P2
Severity: normal → S3

(In reply to Markus Stange [:mstange] (on PTO until Sep 7) from comment #3)

Apparently it is possible to opt into dark mode even when linking against earlier SDKs:

If you build your app against an earlier SDK but still want to support Dark Mode, include the NSRequiresAquaSystemAppearance key (with a value of NO) in your app's Info.plist file. Do so only if your app's appearance looks correct when running in macOS 10.14 and later with Dark Mode enabled.
https://developer.apple.com/documentation/appkit/nsappearancecustomization/choosing_a_specific_appearance_for_your_macos_app?language=objc#2993818

What is the status of this bug now that NSAquaSystemAppearance is set to true? I'm curious because in your reply it says it needs to be set to false.

Blocks: 1558086

(In reply to Alexei Solonari from comment #7)

(In reply to Markus Stange [:mstange] (on PTO until Sep 7) from comment #3)

Apparently it is possible to opt into dark mode even when linking against earlier SDKs:

If you build your app against an earlier SDK but still want to support Dark Mode, include the NSRequiresAquaSystemAppearance key (with a value of NO) in your app's Info.plist file. Do so only if your app's appearance looks correct when running in macOS 10.14 and later with Dark Mode enabled.
https://developer.apple.com/documentation/appkit/nsappearancecustomization/choosing_a_specific_appearance_for_your_macos_app?language=objc#2993818

What is the status of this bug now that NSAquaSystemAppearance is set to true? I'm curious because in your reply it says it needs to be set to false.

We can't set this key to NO until Firefox appears correctly in dark mode without aqua appearance. This is expected to be a large undertaking. You can test the current state by changing this key manually and switching your system to dark mode.

No longer depends on: mojave-sdk
Depends on: 1623686

(In reply to Stephen A Pohl [:spohl] from comment #8)

We can't set this key to NO until Firefox appears correctly in dark mode without aqua appearance. This is expected to be a large undertaking. You can test the current state by changing this key manually and switching your system to dark mode.

I just did this, and thus far it doesn't seem to have led to any problems. It "just works" in terms of making the few macOS-native widgets (i.e. the ones that don't use Firefox theming) correctly adopt Dark Mode.

What do you mean by "appears correctly in dark mode without aqua appearance"? Is there anything in particular that's known to be broken with .darkAqua? Again, in the (very brief) time since I've set NSRequiresAquaSystemAppearance to NO everything seems perfectly fine...

(In reply to Elsie Hupp from comment #9)

(In reply to Stephen A Pohl [:spohl] from comment #8)

We can't set this key to NO until Firefox appears correctly in dark mode without aqua appearance. This is expected to be a large undertaking. You can test the current state by changing this key manually and switching your system to dark mode.

I just did this, and thus far it doesn't seem to have led to any problems. It "just works" in terms of making the few macOS-native widgets (i.e. the ones that don't use Firefox theming) correctly adopt Dark Mode.

What do you mean by "appears correctly in dark mode without aqua appearance"? Is there anything in particular that's known to be broken with .darkAqua? Again, in the (very brief) time since I've set NSRequiresAquaSystemAppearance to NO everything seems perfectly fine...

Try opening the library window, context menus, or the page info window. It won't look right. If that's what you're referring to by "the ones that use Firefox theming," well, that's the entire problem. It's not that macOS parts are themed incorrectly, it's that the pseudo-native stuff still hasn't been fixed yet. They're working on it. :)

Oh, I see I missed your earlier reference to the "quit" warning (which I have disabled because I auto-reopen all tabs). I can keep an eye out for anything egregiously broken, but the main macOS-native dialogues (i.e. drop-down menus, "Open...", "Save...", "Print...", etc.) all seem to work fine.

I imagine there are a few monstrous-hybrid widgets that will still look kind of weird. For instance, the right-click contextual menu is light with light text, as are tooltips. The Library sidebar looks fine, but, yes, the Page Info window is completely borked. I can report back (in existing bug reports, ideally) if I find anything else.

It looks like it's mostly there, though, which is quite exciting!

(In reply to Elsie Hupp from comment #11)

I can report back (in existing bug reports, ideally) if I find anything else.

Thank you, but there isn't much need for this at the moment. Most of the heavy lifting is currently occurring in bug 34572 in case you want to follow along.

The password autofill popover is white-on-white. I don't think that fits under the heading of context menus. But you probably knew that already.

Whiteboard: [mac:darkmode]
Depends on: 1702351
Assignee: nobody → mstange.moz
Status: NEW → ASSIGNED
Pushed by mstange@themasta.com:
https://hg.mozilla.org/integration/autoland/rev/8da69dee2955
Use NSApp.effectiveAppearance to detect system dark mode. r=mac-reviewers,harry
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → 89 Branch
Attachment #9106753 - Attachment is obsolete: true
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: