w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.5k stars 661 forks source link

[css-color] Consider exposing native accent color as a system color keyword. #7347

Closed emilio closed 2 years ago

emilio commented 2 years ago

As per #5900 (read that for context), WebKit / Blink already expose it with -webkit-focus-ring-color.

Gecko has these internally as well (-moz-accent-color / -moz-accent-color-foreground).

Depending on how we go about issues like https://github.com/whatwg/html/issues/7993, we might need a standard version of this, but I think that it'd be useful to authors even without it.

svgeesus commented 2 years ago

Does this resolve fully to a color at computed-value time?

emilio commented 2 years ago

Yes, like all other system colors.

svgeesus commented 2 years ago

-moz-accent-color / -moz-accent-color-foreground

Is the proposal for one or two new colors. If one, will it replace both of these?

emilio commented 2 years ago

I think I'd rather have two, since that's what you need to guarantee contrast, otherwise you can't put stuff in the foreground of the accent color.

fantasai commented 2 years ago

So, AccentColor and AccentColorText or SystemAccentColor and SystemAccentColorText? https://www.w3.org/TR/css-color-4/#css-system-colors

dbaron commented 2 years ago

I'd say the first pair, since none of the other system colors start with System.

svgeesus commented 2 years ago

It seems there is agreement, so adding AccentColor and AccentColorText to CSS Color 4

josepharhar commented 2 years ago

I filed a bug to implement this in chrome: https://bugs.chromium.org/p/chromium/issues/detail?id=1338061 What exactly should AccentColorText resolve to in MacOS and windows? I know that in MacOS you can set a specific accent color, but it's just one color. I've yet to implement native accent color stuff for windows in chrome.

emilio commented 2 years ago

@josepharhar In Gecko we use a standard cocoa color: https://searchfox.org/mozilla-central/rev/b1a5802e0f73bfd6d2096e5fefc2b47831a50b2d/widget/cocoa/nsLookAndFeel.mm#135

And on windows we use white or black based on luminance: https://searchfox.org/mozilla-central/rev/b1a5802e0f73bfd6d2096e5fefc2b47831a50b2d/widget/windows/nsLookAndFeel.cpp#737

josepharhar commented 2 years ago

Thanks @emilio, I implemented a prototype and it seems like I have the same results as firefox, except for the color of the text. I copied the algorithm to choose white or black for AccentColorText from the algorithm from accent-color, and I figured that in a light color scheme we would prefer black text and in a dark color scheme we would prefer white text. What do you think?

Screen Shot 2022-07-19 at 10 49 55 PM Screen Shot 2022-07-19 at 10 48 45 PM Screen Shot 2022-07-19 at 10 47 40 PM
emilio commented 2 years ago

For macOS and Linux, the color comes from the OS. For Windows we use an algorithm that seemed to come from Microsoft here. For Android, our heuristic matches the one for checkbox check colors and so on with custom accent color, which is "white unless contrast is not good, else a darkened enough version of the accent color" (here).

josepharhar commented 1 year ago

I merged the prototype in chromium which has hard coded colors for all platforms except MacOS since chromium doesn't have all the plumbing set up to get the colors from other platforms yet.

In the screenshots in my last comment, it appeared that all platforms except MacOS are always blue in firefox. I see in your links that firefox is hooked up to get an accent color from the OS in windows, but is there a windows system setting you can find which actually affects the resulting AccentColor in firefox?

For android I heard that it works on firefox but I haven't tested it out myself yet.

emilio commented 1 year ago

@josepharhar Windows is the only platform where it's hard-coded to blue on content, though it's controlled by a runtime flag.

See the comment there and https://bugzilla.mozilla.org/show_bug.cgi?id=1776556 for details, we might want to special-case grey accent colors and just use blue then instead or something.

On Linux, the accent color is derived from your GTK theme, so it'll be orange on Ubuntu for example, but blue on Fedora. In any distro you can change the GTK theme and we will honor it. https://github.com/flatpak/xdg-desktop-portal/pull/815 is some related work that would expose the accent color more directly, but meanwhile we read it from the theme here.

On Android and macOS it should work as well.

clshortfuse commented 1 year ago

Made a codepen to debug the browsers:

https://codepen.io/shortfuse/pen/LYgoKQe


Firefox actually picks up the system color on Android. Firefox appears to use AccentColor and AccentColorText for the native control accent. The fact accent is Blue/White and not closer to Blue/BackgroundText means the native controls look odd. (There's a Radio in the demo to check this).

Dark scheme should make things darker, not just invert the colors.

Another observation is that Firefox's Highlight changes the tone on Windows in Dark Scheme. This doesn't happen on Android and it correctly picks up the System Theme's tone and applies the same Accent/Highlight base color, which just a change to opacity on Highlight (30.6%).


Chrome does not appear to have any difference for AccentColor[Text] yet, even on Canary (might be a flag). Though I did notice HighlightText changes on Dark Scheme without changing Highlight, meaning the contrast is wrong and makes it hard to read.


Webkit:

Couldn't find anything to change the system color on my iPad, but even in dark mode, all colors are constant with the exception of BackgroundText. That includes keeping Highlight bright in dark mode. No support for Accent.

josepharhar commented 1 year ago

I noticed that in safari technology preview on macos, AccentColor and AccentColorText resolve to actual values but they don't actually match the system settings. This was implemented here: https://github.com/WebKit/WebKit/commit/09df074a4a8861c5fa7ba160368f6fb1ea1890b0 @weinig is there any plan for safari/webkit to use the actual system accent color?

josepharhar commented 1 year ago

WebKit / Blink already expose it with -webkit-focus-ring-color.

Blink does expose the system accent color this way on MacOS, but no system settings are exposed this way on other platforms, which of course I'd like to do for Android and Windows (and hopefully Linux!) with AccentColor/AccentColorText. However, this could be considered a new fingerprinting vector. Has this been considered? Is there any way this could be implemented which doesn't expose the system accent color to script? I assume it is accessible to script.

Windows is the only platform where it's hard-coded to blue on content

I know there is a system setting for accent color on Windows, any reason not to use it?

clshortfuse commented 1 year ago

However, this could be considered a new fingerprinting vector. Has this been considered?

In the other issue, it has come up:

https://github.com/w3c/csswg-drafts/issues/5900#issuecomment-1150064313

It seems it will allow resolving to an rgb color.

Personally, I'd be okay with a limited color set. I really just want the hue (eg: HSL/HSV) and maybe saturation. I can then recreate what I need from there. That's because lightness would depend on dark-scheme which relates to contrast accessibility. I wouldn't just do 1:1

emilio commented 1 year ago

I know there is a system setting for accent color on Windows, any reason not to use it?

We have code for that, but received bug reports about doing that from people that had grey-ish accent colors, for which form controls looked confusing / disabled, and I didn't have the cycles to investigate further heuristics or what not.

josepharhar commented 1 year ago

In order to prevent script from getting access to the AccentColor value so we can prevent fingerprinting, would it be possible to fake the values returned by getComputedStyle somehow? :visited styles don't show up in getComputedStyle even when they apply. I'm not sure how complicated this could get for AccentColor, but would be ideal to prevent fingerprinting.

dbaron commented 1 year ago

I think if we want the underlying values to not be exposed by getComputedStyle, the simplest approach would likely be to make the keyword be a computed value and a resolved value, so that getComputedStyle just returns the keyword. That might confuse code that's expecting to get a color, though. (It's also worth considering the interaction with things like animations; if the goal is to prevent fingerprinting, probably these keywords shouldn't be interpolable.)

The computed style faking that's done for :visited is quite complex and we probably don't want to add it for a new feature; it's the way it is because it was retrofitted onto an existing feature.

It's also worth noting that there are other aspects to the :visited mitigations such as what gets drawn to a canvas. But at this point it's also worth noting that the :visited mitigations have known gaps and aren't particularly effective in their current form.

josepharhar commented 1 year ago

Thanks David!

the simplest approach would likely be to make the keyword be a computed value and a resolved value, so that getComputedStyle just returns the keyword

the :visited mitigations have known gaps and aren't particularly effective in their current form

Would making the keyboard a computed value and a resolved value be effective compared to :visited mitigations?

That might confuse code that's expecting to get a color, though

Do properties which accept colors, like background-color, accept anything thats not a color like this?

clshortfuse commented 1 year ago

Do note that Firefox on Android is giving us the colors right now. Not sure if they plan on obfuscating.

Screenshot_20230829-162347 Screenshot_20230829-162436

Edit: Sorry about the large images. I uploaded straight from my phone.

emilio commented 1 year ago

Cross-posting https://github.com/WebKit/standards-positions/issues/136#issuecomment-1688989284 which has my opinion on doing something more complex than returning a fixed color.

TLDR I don't think it's worth it. All color keywords including currentColor resolve to something at the end of the day, and I'm not convinced AccentColor is special enough here to warrant something more complex.

josepharhar commented 5 months ago

I filed another issue to discuss mitigations: https://github.com/w3c/csswg-drafts/issues/10372