1843044 - Proposal: Consistently use system colors on the default theme.

Status: RESOLVED FIXED

(Firefox :: Theme, task, P3)

Description

[Emilio Cobos Álvarez (:emilio)]

I propose we move the default theme color tweaks (browser-custom-colors etc) into nsLookAndFeel. The point being that we can change what those system colors return in the default themes quite easily, as needed.

This should simplify the front-end significantly:

• The theme would work more similarly across platforms.
• The theme will just work with high-contrast and dark mode, without a custom light-weight theme.
• It would be easier (if we wanted to) to follow the system patterns like bug 1701266 etc, which other browsers use.

And it shouldn't be particularly harder for Gecko.

I have a PoC for the dark theme on Windows for example. Light theme would need some tweaks to the toolbar and titlebar colors, to match the current default theme colors, but should be relatively straight-forward.

I removed some win7/win8 code while at it but we could leave it there if needed (hopefully not!).

WDYT? This would be on top of bug 1796730.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

Attachment: WIP: Bug 1843044 - PoC: Default theme using dark system colors.

As per the comment in the bug the light theme needs tweaks but the point
stands.

Comment 2

[:Gijs (out for now; he/him)]

The high level seems reasonable to me.

Comment 3

[Dão Gottwald [:dao]]

I do think the way this currently works on Linux makes more sense than on Windows. So +1 for remodeling this, but the devil is going to be in the detail. E.g. the explicit Dark theme has a slight blue tint; would we really want to implement the same in Gecko for system / automatic dark mode? I guess we'd need to differentiate between chrome and content so that web content won't end up with the same blue tint? On Linux we defer to Gtk for system dark mode, so it's usually more neutral gray tones that integrate better with the system. I think this difference between system dark and custom / explicit Dark works fine on Linux and I'd personally be okay with if not in favor of doing the same on Windows, but that would require buy-in from UX.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Sorry, wdym with "blue tint" here? You mean the accent / selection colors? The system colors exposed to content come from our dark theme fwiw. And the accent color is already different between chrome and content on windows (https://searchfox.org/mozilla-central/rev/2d06b7d5fbfa7a31776389da87c5f895b80df8d9/widget/nsXPLookAndFeel.cpp#1296-1300)

But ok assuming this seems roughly desirable I can dig a bit more into how it should look like.

Comment 5

[Dão Gottwald [:dao]]

I meant those colors: https://searchfox.org/mozilla-central/rev/2d06b7d5fbfa7a31776389da87c5f895b80df8d9/toolkit/mozapps/extensions/default-theme/manifest.json#20
Most if not all of them are skewed towards blue.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Ah, we're using those here, and that seems fine. If it becomes a problem we can tweak only for content as needed tho.

Comment 7

[Dão Gottwald [:dao]]

I find this questionable for content where those colors might mix with custom color palettes from random web pages, potentially even on top of a non-default Firefox theme with yet another color palette; I'd prefer a more neutral approach, but maybe that's just me. You don't need to make this your problem given that this is already baked into nsXPLookAndFeel.

Comment 8

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1843044 - Make windows titlebar system colors return our theme's colors for the default theme. r=mhowell,handyman,cmartin

This doesn't change behavior yet because the front-end doesn't use them for the
cases where we're changing behavior, but this is in preparation to use them
unconditionally and simplify a bit the code there.

Comment 9

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1843044 - Make the default theme work with dark color schemes. r=dao,pbz

This builds on the light-dark() function added in bug 1845679 to provide
custom colors for both the default light and dark themes, and remove the
"default theme in dark mode" bits.

This is all to be landed after the soft-freeze in any case. Untested on
macOS for now, but no reason it shouldn't work. Will test later today.

Depends on D184707

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/08098e30932d
Make windows titlebar system colors return our theme's colors for the default theme. r=mhowell

Comment 11

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/08098e30932d

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/37a26c9f2c5c
Make the default theme work with dark color schemes. r=dao,pbz

Comment 13

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/37a26c9f2c5c

Comment 14

[Cristian Tuns]

Backed out for breaking macOS dark theme

• Backout link

and backout merged to central.

Comment 15

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1843044 - Make titlebar system colors on windows and macOS reflect reality. r=#win-reviewers,#mac-reviewers

I'm about to use them more in the front-end, and it'd be nice if these
did the right thing.

For that, make our stand-ins system return the Firefox titlebar colors.

Put the Windows "show accent color on title bars" code behind a pref
because we don't support it. I filed bug 1851155 to consider enabling it
by default.

After this changes there's only one place to tweak the (cross-platform)
titlebar colors (in nsXPLookAndFeel), with high-contrast mode and so on
behaving correctly by default.

The macOS special-case for the titlebar is to preserve the current
behavior from bug 1711261. If we want to change that to also apply to
dark mode or what not, we can do it in a follow-up.

I didn't bother putting the new colors in test_bug232227. That only
(tries to) test that we return fixed colors when the rfp/stand-ins prefs
are on. But we don't need to test the exact value of all the colors.

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d9dadd19a62a
Make titlebar system colors on windows and macOS reflect reality. r=mac-reviewers,bradwerth

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/328530640eae
Add a missing include and while at it dynamic handling for the titlebar color pref.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/18c107db0071
Annotate a test that used to pass on mac by chance.

Comment 19

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/d9dadd19a62a
https://hg.mozilla.org/mozilla-central/rev/328530640eae
https://hg.mozilla.org/mozilla-central/rev/18c107db0071

Comment 20

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/85ed967e285f
Make the default theme work with dark color schemes. r=dao,pbz

Comment 21

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/85ed967e285f