1623686 - Stop forcing aqua appearance on (enable widget.macos.support-dark-appearance by default)

Status: RESOLVED FIXED

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

Description

[Stephen A Pohl [:spohl]]

We should remove the use of HITheme and switch to using Cocoa APIs for things such as context menus etc. This would give us built-in support for macOS dark mode.

Note that bug 1623684 should be reverted once the changes here land.

Comment 1

[Stephen A Pohl [:spohl]]

The use of HITheme APIs is currently restricted to nsNativeThemeCocoa.mm. Here is a list of methods and functions where HITheme APIs are being called:

NSProgressBarCell - (void)drawWithFrame:(NSRect)cellFrame inView:(NSView*)controlView
nsNativeThemeCocoa::DrawMenuBackground
nsNativeThemeCocoa::DrawMenuItem
nsNativeThemeCocoa::DrawMenuSeparator
nsNativeThemeCocoa::DrawFocusOutline
static void RenderButton
nsNativeThemeCocoa::DrawHIThemeButton
nsNativeThemeCocoa::DrawTreeHeaderCell
nsNativeThemeCocoa::DrawSpinButtons
nsNativeThemeCocoa::DrawSpinButton
nsNativeThemeCocoa::DrawTextBox
nsNativeThemeCocoa::DrawTabPanel
nsNativeThemeCocoa::DrawScale
static void RenderResizer
nsNativeThemeCocoa::DrawResizer
nsNativeThemeCocoa::RenderWidget:
	Widget::eSheetBackground
	Widget::eDialogBackground
	Widget::eSeparator
	Widget::eGroupBox
nsNativeThemeCocoa::GetMinimumWidgetSize:
	StyleAppearance::Resizer

It appears that we could figure out a way to remove these calls by replacing them with the use of NSCell and/or NSControl and its subclasses. This does not, however, address the fact that we are currently using the VibrancyManager to draw things such as context menu backgrounds, tooltips etc. Markus, having worked on this quite a bit do you think we should take a two-step approach and remove the use of HITheme first and then work on removing the VibrancyManager? I am asking because we are currently facing two possible problems going forward, and one existing one:

Existing Problem:

• We do not currently support dark mode properly since popups, context menus etc. don't have the correct dark mode appearance. This would be addressed by replacing the VibrancyManager with native context menus etc.

Possible future problems:

1. HITheme may not only be deprecated in a future version of macOS, but it may actually stop working entirely. This could presumably be addressed by finding a workaround using NSCell, NSControl and its subclasses.
2. The NSRequiresAquaSystemAppearance may stop working in a future version of macOS. This means that we wouldn't be able to upgrade our SDK and we would have to find a replacement for the VibrancyManager.

Replacing the VibrancyManager would presumably also address the use of HITheme since it would go away in favor of native context menus etc. Do you happen to have an idea of how involved this would turn out to be?

Comment 2

[Markus Stange [:mstange]]

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

It appears that we could figure out a way to remove these calls by replacing them with the use of NSCell and/or NSControl and its subclasses.

I suppose, yes. I suspect though that for most of these you won't find a usable NSCell equivalent. The other alternative would be to imitate the native rendering with our own rendering.

And even with NSCell, there are challenges to overcome when turning on non-aqua rendering: For example, regular buttons are already drawn via NSCell, but with the non-aqua rendering they become transparent with only a border and no background. We'll have to find a way to get the background drawn, in the requested style (light or dark).

This does not, however, address the fact that we are currently using the VibrancyManager to draw things such as context menu backgrounds, tooltips etc. Markus, having worked on this quite a bit do you think we should take a two-step approach and remove the use of HITheme first and then work on removing the VibrancyManager?

Those projects can definitely be done separately, yes. But rather than calling this project "removing VibrancyManager", let's call out the specific widgets we want to render natively. Concretely, we want to:

• Use native context menus rather than rendering our own context menus.
• The same for tooltips, maybe?

For the vibrancy in the main browser window tab bar, and the main browser window sidebar, we will still need the VibrancyManager.

I am asking because we are currently facing two possible problems going forward, and one existing one:

Existing Problem:

• We do not currently support dark mode properly since popups, context menus etc. don't have the correct dark mode appearance. This would be addressed by replacing the VibrancyManager with native context menus etc.

That's one way to fix it. The other, much simpler, way is to make VibrancyManager render dark vibrancy in the right case.

Possible future problems:

1. HITheme may not only be deprecated in a future version of macOS, but it may actually stop working entirely.

Sure... but it's not deprecated yet, and Apple usually gives us a lot of heads up with deprecation: APIs are deprecated in one release and then removed in a later release.

2. The NSRequiresAquaSystemAppearance may stop working in a future version of macOS. This means that we wouldn't be able to upgrade our SDK and we would have to find a replacement for the VibrancyManager.

True, NSRequiresAquaSystemAppearance may stop working in the future. But then it will stop working regardless of SDK. Also, the VibrancyManager doesn't really factor in here; it can definitely work with or without aqua system appearance.

Comment 3

[Markus Stange [:mstange]]

Morphing this bug slightly: We should keep reducing our use of HITheme, but doing so is not necessary in order to flip the "force aqua" switch. Fixing bug 1644261 is the biggest roadblock, as far as I can tell.

Comment 5

[Markus Stange [:mstange]]

Update: In bug 1697331 I've introducing a pref for this. The Info.plist change has been made, but to unlock the benefits, we will need to set widget.macos.respect-system-appearance to true by default.
At the moment, doing so in macOS Dark Mode will cause many things to render with white-on-white text. I'm going through all of those cases and fixing them one by one, in the bugs that are blocking this bug. Once all problematic cases are fixed, we can flip the pref.

Comment 8

[Mehmet]

Hi, can you please link bug 1705821 to this one? Thanks :)

Comment 11

[Markus Stange [:mstange]]

Bug 1715145 has changed the pref name to widget.macos.support-dark-appearance.

Comment 12

[Harry Twyford]

This was enabled in Release in bug 1719734.