-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Update 2D canvas color serialization #10481
Conversation
@whatwg/canvas @whatwg/css please review. |
I think @mysteryDate did add tests for the serialization of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's get w3c/csswg-drafts#10550 (comment) resolved.
source
Outdated
@@ -67772,8 +67773,9 @@ try { | |||
|
|||
<ol> | |||
<li><p>If <span>this</span>'s <span data-x="concept-CanvasFillStrokeStyles-fill-style">fill | |||
style</span> is a CSS color, then return the <span data-x="serialization of a | |||
color">serialization</span> of that color.</p></li> | |||
style</span> is a CSS color, then return the <span data-x="serialize a CSS <color> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preexisting issue, not necessary to fix: it'd be nice if "CSS color" linked to some definition, in all its usage sites.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it'd be nice if "CSS color" linked to some definition, in all its usage sites.
Agreed, I suggest https://drafts.csswg.org/css-color-4/#typedef-color
which says
Colors in CSS are represented by the
<color>
type:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@annevk what do you think, does that link work for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've decided to not address this for now. I think what we want in HTML is mostly some kind of abstract concept and not a syntax construct. We want to reference what that gets parsed into in a number of places.
I don't believe we have any tests for this for canvas specifically, those would go in the https://source.chromium.org/search?q=f:wpt%20f:canvas%20oklab&sq=package:chromium I think that I didn't add any because I was waiting for the spec to update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This all looks good to me! Thanks for doing this @annevk !
Although fillStyle, strokeStyle, and shadowColor setters accepted all kinds of CSS color values, those could not be serialized. Update that by relying on CSS Color for serialization instead, which now has a HTMLCompatible named parameter to preserve compatibility with 2D canvas and <input type=color> for certain colors. While here, also link the algorithm to be used for color space conversion and correct the reference for 'relative-colorimetric'. Fixes #8917.
4b44ed7
to
ebab7ec
Compare
@domenic it now uses the "with HTML-compatible serialization requested" wording. Good enough? I'll file implementation bugs too. |
…n, a=testonly Automatic update from web-platform-tests 2D canvas color parsing and serialization For whatwg/html#8917, w3c/csswg-drafts#10550, and whatwg/html#10481. -- wpt-commits: 6b71c578c4e9113e56b5445d4697b6a97aedf37d wpt-pr: 47148
…n, a=testonly Automatic update from web-platform-tests 2D canvas color parsing and serialization For whatwg/html#8917, w3c/csswg-drafts#10550, and whatwg/html#10481. -- wpt-commits: 6b71c578c4e9113e56b5445d4697b6a97aedf37d wpt-pr: 47148 UltraBlame original commit: ba0041c24e1332e588896b9fb72ed44e32f1a385
…n, a=testonly Automatic update from web-platform-tests 2D canvas color parsing and serialization For whatwg/html#8917, w3c/csswg-drafts#10550, and whatwg/html#10481. -- wpt-commits: 6b71c578c4e9113e56b5445d4697b6a97aedf37d wpt-pr: 47148 UltraBlame original commit: ba0041c24e1332e588896b9fb72ed44e32f1a385
…n, a=testonly Automatic update from web-platform-tests 2D canvas color parsing and serialization For whatwg/html#8917, w3c/csswg-drafts#10550, and whatwg/html#10481. -- wpt-commits: 6b71c578c4e9113e56b5445d4697b6a97aedf37d wpt-pr: 47148 UltraBlame original commit: ba0041c24e1332e588896b9fb72ed44e32f1a385
…n, a=testonly Automatic update from web-platform-tests 2D canvas color parsing and serialization For whatwg/html#8917, w3c/csswg-drafts#10550, and whatwg/html#10481. -- wpt-commits: 6b71c578c4e9113e56b5445d4697b6a97aedf37d wpt-pr: 47148
…n, a=testonly Automatic update from web-platform-tests 2D canvas color parsing and serialization For whatwg/html#8917, w3c/csswg-drafts#10550, and whatwg/html#10481. -- wpt-commits: 6b71c578c4e9113e56b5445d4697b6a97aedf37d wpt-pr: 47148
Although fillStyle, strokeStyle, and shadowColor setters accepted all kinds of CSS color values, those could not be serialized. Update that by relying on CSS Color for serialization instead, which now has a HTMLCompatible named parameter to preserve compatibility with 2D canvas and for certain colors.
While here, also link the algorithm to be used for color space conversion and correct the reference for 'relative-colorimetric'.
Fixes #8917.
@Kaiido do you happen to know if we already have test coverage for this?
@weinig this might be of interest to you.
(See WHATWG Working Mode: Changes for more details.)
/canvas.html ( diff )
/infrastructure.html ( diff )