1591210 - (forced-color-adjust) Add forced-color-adjust property

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P3)

Description

[Bill Goldstein [:aja] (UTC-6)]

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:72.0) Gecko/20100101 Firefox/72.0

Expected results:

Add forced-color-adjust property, as per
https://drafts.csswg.org/css-color-adjust-1/#forced

Comment 1

[Emilio Cobos Álvarez (:emilio)]

There are still quite a few details that are being sorted out in the working group.

Comment 2

[Bill Goldstein [:aja] (UTC-6)]

Note the addition of value 'preserve-parent-color' (which has UA stylesheet additions for SVGs), in addition to existing 'none' and 'auto' values.

Have details been sorted out in the working group over the past few years enough to implement?
Either with or without the newish 'preserve-parent-color' value?
It's been in Blink for quite a while.

Sure would be nice to be able to use in HCM tests, e.g. green/red pass/fail using 'none' value, consistently cross-browser.

Comment 3

[Marco Castelluccio [:marco]]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #1)

There are still quite a few details that are being sorted out in the working group.

Any news?
Asking because of https://github.com/mozilla/pdf.js/issues/13230.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

So to add this we need:

• To add the property in servo/components/style/longhands. This is an inherited property that affects colors, so putting it in the same struct as the color property would be the obvious choice. So that'd be around here.

• You want a keyword type so you probably want to add an pub enum ForcedColorAdjust { ... }. Something like other keywords like this.

• With that and the member added to nsStyleText here you should've added the CSS property.

Now you'd need to make it actually do something. This property affects colors, so you want to prioritize it on top of all the other color properties by adding it to the right cascade group.

Once you have that you'd need to tweak these checks to early-return when forced-color-adjust is none or preserve-parent-color, as needed.

Comment 5

[Fred Chasen [:fchasen]]

Thank so much for the detailed instructions Emilio, was able to get this parsing the keywords so far with those!

I'm wondering if the PrintColorAdjust might be a good example to base this off: https://searchfox.org/mozilla-central/source/servo/components/style/values/specified/color.rs#1309-1329 since this property affects both text and background-colors?

If I mirrored that than it would then be accessible from StyleVisibility()->mForcedColorAdjust.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Yes but not quite, because we do the color forcing at styling time. So the checks will need to happen in cascade.rs (because otherwise the non-forced colors are gone already).

Comment 7

[Fred Chasen [:fchasen]]

Attachment: Bug 1591210 - Add forced-color-adjust property r=emilio

Adds the forced-color-adjust property and ForcedColorAdjust keywords.
Updates tweak_when_ignoring_colors to check for none and preserve-parent-color values of that property when determining if a color adjustment in needed.

Comment 8

[Fred Chasen [:fchasen]]

Going through the WPTs in testing/web-platform/tests/forced-colors-mode/ to see what we could enable once this is supported.

Looks like the selection force adjust none isn't currently working (forced-colors-mode-14.html), so looking into that.

Also the SVG ones still fail, but not sure we currently apply forced colors on SVG?

Comment 9

[Fred Chasen [:fchasen]]

Was able to get the selection to not adjust by checking pseudoStyle->StyleText()->mForcedColorAdjust != StyleForcedColorAdjust::None in ComputeSelectionStyle: https://searchfox.org/mozilla-central/source/layout/generic/nsIFrame.cpp#2401

Not sure that's the best way though?

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f4fe07518400
Add forced-color-adjust property r=emilio

Comment 11

[Marian-Vasile Laza]

Backed out changeset f4fe07518400 (Bug 1591210) for web-platform failures on forced-colors-mode-14.html.
Backout link
Push with failures <--> Wr1
Failure Log
Also c3

Comment 12

[Fred Chasen [:fchasen]]

Thanks, I've updated the unexpectedly passing test to pass in Linux now and fixed the failures with test_animation-type-longhand.html.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1c143c3c235b
Add forced-color-adjust property r=emilio

Comment 14

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/1c143c3c235b