fix: color contrast fails for oklch and oklab with none #4959
Conversation
…e parsed into a color
…e to be parsed into a color
…arse issue but was not being caught for foreground color
|
|
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where color contrast checks fail when encountering OKLCH or OKLAB colors with none values. The fix adds error handling to gracefully report parsing failures as incomplete results rather than crashing the check.
- Wraps color parsing operations in try-catch blocks to handle parsing errors
- Updates the Color class to throw errors with structured cause information
- Adds new "colorParse" message key to report unparseable color strings across all locales
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/commons/color/color.js | Enhanced error throwing to include cause with color string value |
| lib/commons/color/get-foreground-color.js | Added try-catch to handle foreground color parsing failures |
| lib/commons/color/get-background-color.js | Added try-catch to handle background color parsing failures |
| lib/checks/color/color-contrast-evaluate.js | Added logic to detect and report colorParse errors from incomplete data |
| lib/checks/color/color-contrast.json | Added colorParse error message template |
| lib/checks/color/color-contrast-enhanced.json | Added colorParse error message template |
| locales/_template.json | Added colorParse error message template to both color-contrast and color-contrast-enhanced |
| test/checks/color/color-contrast.js | Added three test cases validating error handling for unparseable colors in foreground, background, and text-shadow |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch (error) { | ||
| if (error.cause) { | ||
| incompleteData.set('colorParse', error.cause.value); | ||
| return null; |
There was a problem hiding this comment.
The catch block silently suppresses errors that don't have a cause property. Consider either re-throwing unexpected errors or logging them to aid debugging.
| return null; | |
| return null; | |
| } else { | |
| console.error('Unexpected error in getForegroundColor:', error); | |
| throw error; |
| } catch (error) { | ||
| if (error.cause) { | ||
| incompleteData.set('colorParse', error.cause.value); | ||
| return null; |
There was a problem hiding this comment.
The catch block silently suppresses errors that don't have a cause property. Consider either re-throwing unexpected errors or logging them to aid debugging.
| return null; | |
| return null; | |
| } else { | |
| // Log unexpected errors to aid debugging | |
| console.error('Unexpected error in getOwnBackgroundColor:', error); |
Adds error handling for color-contrast checks. This now will report an 'incomplete' error message when the foreground and/or background color strings were not able to be parsed into an actual color.
Tests also verify that text-shadow color is handled should that occur.
fixes: 4894
Developer Notes:
I was not able to have a test case trigger the colorParse error on text-shadow colorParse issues. This case was getting caught already but in the form of the 'complexTextShadows' error.