Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
emilio opened this issue Jun 9, 2022 · 24 comments
Closed
Labels
css-color-4 Current Work

Comments

@emilio
Copy link
Collaborator

emilio commented Jun 9, 2022

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 whatwg/html#7993, we might need a standard version of this, but I think that it'd be useful to authors even without it.

@svgeesus
Copy link
Contributor

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

@emilio
Copy link
Collaborator Author

emilio commented Jun 10, 2022

Yes, like all other system colors.

@svgeesus
Copy link
Contributor

-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
Copy link
Collaborator Author

emilio commented Jun 10, 2022

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
Copy link
Collaborator

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

@dbaron
Copy link
Member

dbaron commented Jun 14, 2022

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

@svgeesus
Copy link
Contributor

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

@josepharhar
Copy link
Contributor

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
Copy link
Collaborator Author

emilio commented Jun 21, 2022

moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 22, 2022
…n of the color.

As per w3c/csswg-drafts#7347.

Mostly renaming, doesn't change behavior other than exposing the new
color keywords (tested in wpt).

Differential Revision: https://phabricator.services.mozilla.com/D149876

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1775247
gecko-commit: f0208afb46f98c1827160cacbab6bb2bb5ebe109
gecko-reviewers: mstange
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Jun 22, 2022
…refixed version of the color. r=mstange

As per w3c/csswg-drafts#7347.

Mostly renaming, doesn't change behavior other than exposing the new
color keywords (tested in wpt).

Differential Revision: https://phabricator.services.mozilla.com/D149876
moz-wptsync-bot pushed a commit to web-platform-tests/wpt that referenced this issue Jun 22, 2022
…n of the color.

As per w3c/csswg-drafts#7347.

Mostly renaming, doesn't change behavior other than exposing the new
color keywords (tested in wpt).

Differential Revision: https://phabricator.services.mozilla.com/D149876

bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1775247
gecko-commit: f0208afb46f98c1827160cacbab6bb2bb5ebe109
gecko-reviewers: mstange
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Jun 23, 2022
…refixed version of the color. r=mstange

As per w3c/csswg-drafts#7347.

Mostly renaming, doesn't change behavior other than exposing the new
color keywords (tested in wpt).

Differential Revision: https://phabricator.services.mozilla.com/D149876
@josepharhar
Copy link
Contributor

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
Copy link
Collaborator Author

emilio commented Jul 20, 2022

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

@astearns astearns removed the Agenda+ label Sep 8, 2022
aarongable pushed a commit to chromium/chromium that referenced this issue Jan 30, 2023
This was proposed and resolved here:
w3c/csswg-drafts#7347

This patch matches the firefox behavior. We don't currently have a way
of getting custom accent colors on any platform besides MacOS, and it
would seem that firefox doesn't do so either.

This is gated behind an experimental flag for now.

Bug: 1338061
Change-Id: I97393303f5b6fb141acaaf525d9f353bea49a604
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764721
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Reviewed-by: Mason Freed <masonf@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1098956}
@josepharhar
Copy link
Contributor

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
Copy link
Collaborator Author

emilio commented Feb 3, 2023

@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. flatpak/xdg-desktop-portal#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.

aarongable pushed a commit to chromium/chromium that referenced this issue Mar 21, 2023
This reverts commit f14e290.

Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=1425116
Despite being gated behind a flag, this patch still affects the behavior on sites where "AccentColor" or "AccentColorText" is used and can result in some sort of transparent color being rendered.

Original change's description:
> Implement AccentColor and AccentColorText system colors
>
> This was proposed and resolved here:
> w3c/csswg-drafts#7347
>
> This patch matches the firefox behavior. We don't currently have a way
> of getting custom accent colors on any platform besides MacOS, and it
> would seem that firefox doesn't do so either.
>
> This is gated behind an experimental flag for now.
>
> Bug: 1338061
> Change-Id: I97393303f5b6fb141acaaf525d9f353bea49a604
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764721
> Commit-Queue: Joey Arhar <jarhar@chromium.org>
> Reviewed-by: Mason Freed <masonf@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1098956}

Bug: 1338061
Fixed: 1425116
Change-Id: I5b773ce4fdb8da1fbebe95bfc2ec4137e5e9c2e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4357404
Reviewed-by: Mason Freed <masonf@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Auto-Submit: Joey Arhar <jarhar@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1119971}
calyxos-gerrit pushed a commit to CalyxOS/chromium that referenced this issue Mar 31, 2023
This reverts commit f14e290.

Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=1425116
Despite being gated behind a flag, this patch still affects the behavior on sites where "AccentColor" or "AccentColorText" is used and can result in some sort of transparent color being rendered.

Original change's description:
> Implement AccentColor and AccentColorText system colors
>
> This was proposed and resolved here:
> w3c/csswg-drafts#7347
>
> This patch matches the firefox behavior. We don't currently have a way
> of getting custom accent colors on any platform besides MacOS, and it
> would seem that firefox doesn't do so either.
>
> This is gated behind an experimental flag for now.
>
> Bug: 1338061
> Change-Id: I97393303f5b6fb141acaaf525d9f353bea49a604
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3764721
> Commit-Queue: Joey Arhar <jarhar@chromium.org>
> Reviewed-by: Mason Freed <masonf@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1098956}

(cherry picked from commit bb8ea91)

Bug: 1338061
Fixed: 1425116
Change-Id: I5b773ce4fdb8da1fbebe95bfc2ec4137e5e9c2e9
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4357404
Reviewed-by: Mason Freed <masonf@chromium.org>
Commit-Queue: Joey Arhar <jarhar@chromium.org>
Commit-Queue: Mason Freed <masonf@chromium.org>
Auto-Submit: Joey Arhar <jarhar@chromium.org>
Cr-Original-Commit-Position: refs/heads/main@{#1119971}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4362043
Cr-Commit-Position: refs/branch-heads/5615@{#745}
Cr-Branched-From: 9c6408e-refs/heads/main@{#1109224}
@clshortfuse
Copy link

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
Copy link
Contributor

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: WebKit/WebKit@09df074
@weinig is there any plan for safari/webkit to use the actual system accent color?

@josepharhar
Copy link
Contributor

josepharhar commented Aug 18, 2023

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
Copy link

clshortfuse commented Aug 18, 2023

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

In the other issue, it has come up:

#5900 (comment)

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
Copy link
Collaborator Author

emilio commented Aug 18, 2023

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
Copy link
Contributor

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
Copy link
Member

dbaron commented Aug 23, 2023

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
Copy link
Contributor

josepharhar commented Aug 29, 2023

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
Copy link

clshortfuse commented Aug 29, 2023

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
Copy link
Collaborator Author

emilio commented Aug 29, 2023

Cross-posting WebKit/standards-positions#136 (comment) 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.

Loirooriol pushed a commit to Loirooriol/servo that referenced this issue Sep 25, 2023
… version of the color

As per w3c/csswg-drafts#7347.

Mostly renaming, doesn't change behavior other than exposing the new
color keywords (tested in wpt).

Differential Revision: https://phabricator.services.mozilla.com/D149876
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue Sep 25, 2023
… version of the color

As per w3c/csswg-drafts#7347.

Mostly renaming, doesn't change behavior other than exposing the new
color keywords (tested in wpt).

Differential Revision: https://phabricator.services.mozilla.com/D149876
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue Sep 25, 2023
… version of the color

As per w3c/csswg-drafts#7347.

Mostly renaming, doesn't change behavior other than exposing the new
color keywords (tested in wpt).

Differential Revision: https://phabricator.services.mozilla.com/D149876
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue Sep 25, 2023
… version of the color

As per w3c/csswg-drafts#7347.

Mostly renaming, doesn't change behavior other than exposing the new
color keywords (tested in wpt).

Differential Revision: https://phabricator.services.mozilla.com/D149876
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue Sep 25, 2023
… version of the color

As per w3c/csswg-drafts#7347.

Mostly renaming, doesn't change behavior other than exposing the new
color keywords (tested in wpt).

Differential Revision: https://phabricator.services.mozilla.com/D149876
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue Sep 28, 2023
… version of the color

As per w3c/csswg-drafts#7347.

Mostly renaming, doesn't change behavior other than exposing the new
color keywords (tested in wpt).

Differential Revision: https://phabricator.services.mozilla.com/D149876
Loirooriol pushed a commit to Loirooriol/servo that referenced this issue Oct 2, 2023
… version of the color

As per w3c/csswg-drafts#7347.

Mostly renaming, doesn't change behavior other than exposing the new
color keywords (tested in wpt).

Differential Revision: https://phabricator.services.mozilla.com/D149876
github-merge-queue bot pushed a commit to servo/servo that referenced this issue Oct 2, 2023
… version of the color

As per w3c/csswg-drafts#7347.

Mostly renaming, doesn't change behavior other than exposing the new
color keywords (tested in wpt).

Differential Revision: https://phabricator.services.mozilla.com/D149876
@josepharhar
Copy link
Contributor

I filed another issue to discuss mitigations: #10372

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-color-4 Current Work
Projects
None yet
Development

No branches or pull requests

7 participants