fix(parse): match color function names and keywords case-insensitively - #275
maximilliangrand wants to merge 2 commits into
Conversation
CSS function names and keywords are ASCII case-insensitive, but culori only accepted them in lowercase: `RGB(255 0 0)`, `HSL(...)`, `OKLCH(...)`, `COLOR(display-p3 ...)`, uppercase hue units (`120DEG`), and `TRANSPARENT` all returned undefined, while `rgb(...)`/`red`/`RED`/`#FFF` worked. - Normalize identifiers (function names, keywords, color-space names, hue units) to lowercase as they are consumed by the tokenizer. - Add the `i` flag to the legacy comma-syntax rgb()/hsl() regexes and lowercase the captured hue unit before mapping it. - Match `transparent` case-insensitively. - Guard the color() profile lookup with an own-property check so identifiers inherited from Object.prototype (`constructor`, `__proto__`, …) resolve to no profile and return undefined instead of throwing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
danburzo
left a comment
danburzo
left a comment
There was a problem hiding this comment.
Thank you for this PR. I’ve made a few comments, but otherwise looks good!
| if (match[3] !== undefined) { | ||
| res.h = +match[3]; | ||
| } else if (match[1] !== undefined && match[2] !== undefined) { | ||
| res.h = hueToDeg(match[1], match[2]); |
There was a problem hiding this comment.
Just as a stylistic preference, could you move toLowerCase() to the hueToDeg() function directly?
switch (unit?.toLowerCase()) { … }| @@ -1,5 +1,5 @@ | |||
| const parseTransparent = c => | |||
| c === 'transparent' | |||
There was a problem hiding this comment.
It would be useful to guard for undefined colors with c?.toLowerCase().
| while (_i < chars.length && IdentCodePoint.test(chars[_i])) { | ||
| v += chars[_i++]; | ||
| } | ||
| return v; |
There was a problem hiding this comment.
As noted in #276, I think instead of all the changes here we just make the input lowercase in the tokenize() function.
|
Thanks for the review. Addressed in I added coverage for the direct The full suite has 13 existing floating-point exact-comparison failures on this macOS arm64 machine. I verified the same 13 failures on the unchanged published head with the same Node runtime: 235 passing before, 238 after the three added tests. No numerical conversion code changed. Source/test lint and formatting checks pass. The build also passes; I checked uppercase parsing and absent-input handling through both the built ESM and CommonJS exports. |
This branch has not been deployed
2 participants
CSS color function names and keywords are case-insensitive, but forms such as
RGB(255 0 0),TRANSPARENT,COLOR(display-p3 ...), and uppercase hue units were rejected.Normalize the trimmed input once in
tokenize(), make the legacy RGB/HSL patterns case-insensitive, normalize angle units insidehueToDeg(), and accept case-insensitivetransparentwhile guarding absent inputs. These incorporate all three maintainer suggestions. The color-profile lookup retains its own-property guard, so names such asCONSTRUCTORcannot invoke inherited properties.Validation of this follow-up (
c94320a):434b9d7) has the same 13 failures with the same Node runtime (235 passed before these three tests were added). No numerical conversion code changed. The full suite is not being reported as green.AI assistance was used for the review follow-up and validation.