User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/79.0.3945.0 Safari/537.36 Edg/79.0.313.0

Steps to reproduce:

Support is needed for new CSS4 system color keywords: https://drafts.csswg.org/css-color-4/#css-system-colors

Missing keywords include:

• ActiveText
• Canvas
• Field
• FieldText
• LinkText
• Text
• VisitedText

At least five of them we already have -moz- prefixed variants of: https://searchfox.org/mozilla-central/rev/088e2cf29c59d733d57af43903eb0267dbf72e2a/servo/components/style/values/specified/color.rs#557

I think the remaining would be Field and FieldText, which we also have apparently!

https://searchfox.org/mozilla-central/rev/088e2cf29c59d733d57af43903eb0267dbf72e2a/servo/components/style/values/specified/color.rs#142

So this should be pretty easy. Mostly renaming and adding #[parse(aliases)] declarations like https://searchfox.org/mozilla-central/rev/088e2cf29c59d733d57af43903eb0267dbf72e2a/servo/components/style/values/computed/length.rs#875.

If someone is interested in giving this a shot, I'd be happy to mentor or help if something is unclear or if they get stuck!

Hi, I'm new to both Rust and contributing to Servo, and I'm interested in picking this up.

Could you help me figure out what exactly needs to be done for this?

My best guess for starting is that I need to remove the moz prefix from Mozfield and Mozfieldtext and add aliases for the moz prefixed version

e.g.

pub enum Color {
    #[parse(aliases = "Mozfield")]
    Field
}

However, I'm not really sure how this interacts with the rest of the codebase - where it'd need to be renamed and where this color is defined in the first place.

Thanks for your help.

Hi! The aliases field is not camel-case, so should be something like #[parse(aliases = "-moz-field")].

But yeah, generally that sums it up. After changing the name you'll get a few build errors, that will tell you next steps. The Rust enum automagically generates a C++ enum at build time, with the same variants, so for example these uses of MozField need to be renamed to just Field:

• https://searchfox.org/mozilla-central/rev/74cc0f4dce444fe0757e2a6b8307d19e4d0e0212/layout/generic/nsTextFrame.cpp#4081

There are a couple others, but generally, yeah, you also need to rename the ColorID::MozField to be ColorID::Field in that example, same with the other variants.

These colors are queried to the system via the various implementtions of LookAndFeel for different platforms.

Removing the Moz prefix to support the CSS4 System Colors Draft
An alias has been added to keep support for the -moz-field(text) variant

Hmm, actually, I have a few questions, and I suspect David will have opinions for them so I want him to take a look... And I suspect that the patch may need revisions / followup work to deal with some of these.

Thinking a bit harder about this, we should probably actually expose the system color, even if the default text and background are overriden by user preferences.

That is:

• linktext should probably map to -moz-nativehyperlinktext, not -moz-hyperlinktext.
• canvas / text should probably map to WindowBackground / WindowForeground, not -moz-default-background-color / -moz-default-color.
• ActiveLinkText / VisitedLinkText need some new implementation that pokes at the system, probably.
• We should probably use the "standins" mechanism to provide the privacy-preserving alternative to these.

Davide, what's your thought? should we override the system with the user preference? or the other way around?

In any case this is probably worth landing, and workign on top, or at least we should split the field / fieldtext changes as those are straight renames even with these changes.

Note renaming in css-color-4 of new system color Text to CanvasText
https://github.com/w3c/csswg-drafts/issues/4465#issuecomment-547994485

Setting keyword "dev-doc-needed" (should be set ideally together with reporting this kind of bugs). Thanks!

Sorry for the delay responding here.

I filed https://github.com/w3c/csswg-drafts/issues/4533 to ask the spec to clarify. I think my preference is probably leaning slightly the other way: that it probably makes more sense for the keywords to be Gecko's preference, since pages would likely want to use them to reset things to the defaults for links, or to style things that aren't links to look like the browser's defaults for links, etc. (I think some of the use cases for the existing -moz- variants are in browser chrome, which is somewhat different since it wants to fit in to the system.)

I agree that the Text->CanvasText should be fixed, and the anti-fingerprinting preference honored.

I'm ok with that, I think that argument makes sense, but let's wait for the WG to clarify.

There's a r+ patch which didn't land and no activity in this bug for 2 weeks.
:Sam, could you have a look please?
For more information, please visit auto_nag documentation.

Are we still waiting for clarification?

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

I'm ok with that, I think that argument makes sense, but let's wait for the WG to clarify.

(In reply to Sam Mauldin (:Sam) from comment #11)

Are we still waiting for clarification?

Emilio, I think the assignee meant to flag a needinfo to you. See above for the question.

Yes, as other vendors haven't replied... I've added it to the agenda so it gets discussed.

There's a resolution now in that issue that it should reflect the browser settings.

Sam, I think your patch is alright based on that, do you mind rebasing it and update the test expectations? I suspect at least https://searchfox.org/mozilla-central/source/testing/web-platform/tests/css/css-color/color-initial-canvastext.html and https://searchfox.org/mozilla-central/source/testing/web-platform/tests/css/css-color/parsing/system-color-valid.html should have unexpected passes.

If you don't have time let me know and I'm happy to do that.

Hey!

I've rebased this patch, changed Text to Canvastext, and update test expectations. I added expected fails for some forced-colors-mode-backplate, which I suspect were only passing because the backplate of Canvastext wasn't actually being applied since we didn't have that color, but I'd appreciate if you could confirm that this is expected.

I replied in phab, but yeah those tests are passing for the wrong reasons so it is fine to annotate them as such with your patch.

Backed out changeset a2030f611422 (Bug 1590894) for causing mochitest failure at layout/style/test/test_value_computation.html

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&collapsedPushes=659858&selectedJob=293236916&resultStatus=testfailed%2Cbusted%2Cexception&revision=a2030f611422814034b8c2ddf7014aea8b05bb56

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=293236916&repo=autoland&lineNumber=6709

Backout link: https://treeherder.mozilla.org/#/jobs?repo=autoland&collapsedPushes=659858&resultStatus=testfailed%2Cbusted%2Cexception&revision=f14f678590b7b71de62740c3ea540b48eb17397b

[task 2020-03-15T20:35:08.977Z] 20:35:08     INFO -  3570 INFO TEST-PASS | layout/style/test/test_value_computation.html | should not get empty value for 'color:canvastext'
[task 2020-03-15T20:35:08.977Z] 20:35:08     INFO -  Buffered messages finished
[task 2020-03-15T20:35:08.986Z] 20:35:08  WARNING -  3571 INFO TEST-UNEXPECTED-FAIL | layout/style/test/test_value_computation.html | should not get initial value for 'color:canvastext' on elementn. - didn't expect "rgb(0, 0, 0)", but got it
[task 2020-03-15T20:35:08.986Z] 20:35:08     INFO -      SimpleTest.isnot@SimpleTest/SimpleTest.js:402:14
[task 2020-03-15T20:35:08.986Z] 20:35:08     INFO -      test_value@layout/style/test/test_value_computation.html:174:73
[task 2020-03-15T20:35:08.986Z] 20:35:08     INFO -      test_property@layout/style/test/test_value_computation.html:206:15
[task 2020-03-15T20:35:08.986Z] 20:35:08     INFO -      do_one@layout/style/test/test_value_computation.html:221:18
[task 2020-03-15T20:35:08.987Z] 20:35:08  WARNING -  3572 INFO TEST-UNEXPECTED-FAIL | layout/style/test/test_value_computation.html | should not get initial value for 'color:canvastext' on elementf. - didn't expect "rgb(0, 0, 0)", but got it
[task 2020-03-15T20:35:08.987Z] 20:35:08     INFO -      SimpleTest.isnot@SimpleTest/SimpleTest.js:402:14
[task 2020-03-15T20:35:08.987Z] 20:35:08     INFO -      test_value@layout/style/test/test_value_computation.html:178:86
[task 2020-03-15T20:35:08.987Z] 20:35:08     INFO -      test_property@layout/style/test/test_value_computation.html:206:15
[task 2020-03-15T20:35:08.987Z] 20:35:08     INFO -      do_one@layout/style/test/test_value_computation.html:221:18

Hmm, that's a bit annoying. I guess canvastext should be in the initial_values array.

Commented in Phabricator, I think this should be good to try and land again if everything's alright with you.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=21765b7681e14d88c93f9dae515f183cb75d116f

Yeah, agreed. Sorry I missed that comment. Thank you!

https://hg.mozilla.org/mozilla-central/rev/3f2a791d050a

Am I understanding correctly that Firefox does not yet support forced-colors (per bug 1591204)? And that therefore the dev-doc-needed here is for documenting the standard keywords, but not for Firefox doing something with them?

Updated the System colors section of the color data type page, and moved the old values into a "deprecated" subsection. Linked to the system colors section from the forced-colors page.

Hi Janet,

(In reply to Janet Swisher from comment #31)

Am I understanding correctly that Firefox does not yet support forced-colors (per bug 1591204)? And that therefore the dev-doc-needed here is for documenting the standard keywords, but not for Firefox doing something with them?

That's right. We do support some of forced-colors (as in, windows high-contrast mode does most if not all of that). But we don't support the media feature or the forced-color-adjust css property.

The documentation is for the new CSS color keywords.

(In reply to Janet Swisher from comment #32)

Updated the System colors section of the color data type page, and moved the old values into a "deprecated" subsection. Linked to the system colors section from the forced-colors page.

Thanks! That sounds great.