Support unresolved calc() values in Canvas 2D color parsing [chromium/src : main]
Felipe Erias (Gerrit)
Felipe Erias added 1 comment![]( "Open in Gerrit")
Hello,
This CL fixes a bug with color functions using calc() in Canvas.
See https://issues.chromium.org/issues/459135661 for details and an example.
The fix itself is very small. I've added a thorough test file to catch similar issues in the future.
Thank you for your feedback.
Best,
Felipe
Related details
- Aaron Krajeski
- Jean-Philippe Gravel
- Justin Novosad
- Rune Lillesveen
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rune Lillesveen (Gerrit)
Rune Lillesveen voted and added 1 comment![]( "Open in Gerrit")
Votes added by Rune Lillesveen
| Code-Review | +1 |
1 comment
- Aaron Krajeski
- Felipe Erias
- Jean-Philippe Gravel
- Justin Novosad
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jean-Philippe Gravel (Gerrit)
Jean-Philippe Gravel voted and added 3 comments![]( "Open in Gerrit")
Votes added by Jean-Philippe Gravel
| Code-Review | +1 |
3 comments
Thanks!
// Helper function to render a color and return its pixel value as rgba stringNit: consider ending comment sentences with a period. We often view one-line code snippets in tools like code search. Seeing this line without the next one might trick a reader into thinking that the sentence isn't complete and spans over multiple lines. Proper punctuation can avoid this confusion.
I can't find a guideline specific to JavaScript, but some Google style guides call this out explicitly, for instance:
“Comments should be as readable as narrative text, with proper capitalization and punctuation. In many cases, complete sentences are more readable than sentence fragments. Shorter comments, such as comments at the end of a line of code, can sometimes be less formal, but you should be consistent with your style.
Although it can be frustrating to have a code reviewer point out that you are using a comma when you should be using a semicolon, it is very important that source code maintain a high level of clarity and readability. Proper punctuation, spelling, and grammar help with that goal.”
https://google.github.io/styleguide/cppguide.html#Punctuation_Spelling_and_Grammar
function assertColorsEqual(ctx, colorGroup, description) {`description` is unused. Remove?
Related details
- Aaron Krajeski
- Felipe Erias
- Justin Novosad
Code-Coverage
Code-Owners
Code-Review
No-Unresolved-Comments
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Felipe Erias (Gerrit)
Felipe Erias added 3 comments![]( "Open in Gerrit")
Thank you for the feedback, I have updated the test file as suggested.
// Helper function to render a color and return its pixel value as rgba stringNit: consider ending comment sentences with a period. We often view one-line code snippets in tools like code search. Seeing this line without the next one might trick a reader into thinking that the sentence isn't complete and spans over multiple lines. Proper punctuation can avoid this confusion.
I can't find a guideline specific to JavaScript, but some Google style guides call this out explicitly, for instance:
“Comments should be as readable as narrative text, with proper capitalization and punctuation. In many cases, complete sentences are more readable than sentence fragments. Shorter comments, such as comments at the end of a line of code, can sometimes be less formal, but you should be consistent with your style.
Although it can be frustrating to have a code reviewer point out that you are using a comma when you should be using a semicolon, it is very important that source code maintain a high level of clarity and readability. Proper punctuation, spelling, and grammar help with that goal.”
https://google.github.io/styleguide/cppguide.html#Punctuation_Spelling_and_Grammar
Done
function assertColorsEqual(ctx, colorGroup, description) {Felipe Erias`description` is unused. Remove?
Done
Related details
- Aaron Krajeski
- Jean-Philippe Gravel
- Justin Novosad
- Rune Lillesveen
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Rune Lillesveen (Gerrit)
Rune Lillesveen voted Code-Review+1![]( "Open in Gerrit")
- Aaron Krajeski
- Felipe Erias
- Jean-Philippe Gravel
- Justin Novosad
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Jean-Philippe Gravel (Gerrit)
Jean-Philippe Gravel voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Aaron Krajeski
- Felipe Erias
- Justin Novosad
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Felipe Erias (Gerrit)
Felipe Erias voted Commit-Queue+2![]( "Open in Gerrit")
- Aaron Krajeski
- Justin Novosad
Code-Coverage
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Chromium LUCI CQ submitted the change![]( "Open in Gerrit")
Change information
Support unresolved calc() values in Canvas 2D color parsing
Add IsUnresolvedColorValue() check in canvas color parsing to properly
handle CSS color functions containing calc() expressions. Without this,
colors with calc() were being rendered inaccurately.
- M third_party/blink/renderer/modules/canvas/canvas2d/canvas_style.cc
- A third_party/blink/web_tests/fast/canvas/canvas-fillStyle-color-functions-with-calc.html
Code-Review: +1 by Rune Lillesveen, +1 by Jean-Philippe Gravel
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |