fix(colors): pick WCAG-higher-contrast text for author colors

[JohnMcLear]

Summary

Fixes #7377. The pre-fix text-colour selector used a plain-luminosity cutoff (luminosity(bg) < 0.5 ? white : black), which produces sub-AA text on a band of mid-saturation author colours. The exact failure from the issue screenshot is #ff0000: luminosity is 0.30 so the old code picked white text, giving a 4.0:1 contrast ratio — below WCAG AA.

Per WCAG 2.1, contrast ratio is symmetric and well-defined in [1, 21]. For any sRGB colour at least one of black or white text clears AA (4.5:1). So the right fix is just to pick whichever of the two gives the higher contrast.

What changed

• src/static/js/colorutils.ts:
  • New relativeLuminance(triple) — WCAG 2.1 relative-luminance formula.
  • New contrastRatio(c1, c2) — returns a value in [1, 21]; ≥4.5 = AA, ≥7.0 = AAA.
  • textColorFromBackgroundColor(bg, skinName) rewritten to pick whichever of black/white produces the higher ratio against the background. Same signature, same return contract (still honours the colibris CSS-variable aliases).
• src/static/js/ace2_inner.ts: unchanged call site; the fix is transparent to the caller.
• src/tests/backend/specs/colorutils.ts: unit tests including the exact #ff0000 failure case, colibris skin handling, and a "for every primary the chosen text hits ≥4.5" sweep that guards the whole WCAG-AA invariant.

Why this shape

First draft of the PR added an iterative ensureReadableBackground helper gated by a padOptions.enforceReadableAuthorColors flag. CI caught that the extra machinery wasn't load-bearing: once the text selector is WCAG-aware, bg lightening is never needed for AA. Dropped the helper, the flag, the settings/Docker env, and the docs — the final diff is +64 / -29 and entirely inside colorutils + its test.

Test plan

• pnpm run ts-check clean locally.
• src/tests/backend/specs/colorutils.ts passes locally.
• CI: backend + playwright.
• Manual: pad with an #ff0000 author has readable (black) text; pad with #111111 author has readable (white) text; pad with pre-existing pastel author colours looks the same as before.

Closes #7377

🤖 Generated with Claude Code

[qodo-free-for-open-source-projects]

Review Summary by Qodo

Clamp author backgrounds to WCAG 2.1 AA on render

✨ Enhancement

Walkthroughs

Description

• Add WCAG 2.1 AA contrast helpers to ensure author background colors are readable
• Implement viewer-side color clamping that lightens dark backgrounds while preserving hue
• Add enforceReadableAuthorColors setting (default true) to control the feature
• Include comprehensive unit tests for luminance, contrast ratio, and background clamping

Diagram

flowchart LR
  A["Author picks color<br/>e.g. #cc0000"] -->|stored unchanged| B["Author's color<br/>in database"]
  B -->|viewer-side render| C["Check contrast<br/>with black text"]
  C -->|fails AA 4.5:1| D["Iteratively blend<br/>toward white"]
  D -->|preserve hue| E["Readable color<br/>e.g. #ff9999"]
  C -->|passes AA| F["Use original color"]
  E -->|render with| G["Black text on<br/>readable background"]
  F -->|render with| G

Loading

File Changes

1. src/node/utils/Settings.ts ⚙️ Configuration changes +2/-0

Add enforceReadableAuthorColors setting

• Add enforceReadableAuthorColors: boolean field to padOptions type definition
• Set default value to true in settings configuration

src/node/utils/Settings.ts

---

2. src/static/js/colorutils.ts ✨ Enhancement +53/-0

Add WCAG 2.1 contrast calculation helpers

• Implement relativeLuminance() helper using WCAG 2.1 formula for sRGB triples
• Implement contrastRatio() helper returning values in [1, 21] range
• Implement ensureReadableBackground() that iteratively blends colors toward white to meet minimum
 contrast
• Add comprehensive documentation explaining the WCAG AA compliance approach

src/static/js/colorutils.ts

---

3. src/static/js/ace2_inner.ts ✨ Enhancement +12/-0

Wire up background color clamping in editor

• Integrate ensureReadableBackground() into setAuthorStyle() function
• Gate the clamping on padOptions.enforceReadableAuthorColors flag
• Guard with colorutils.isCssHex() to skip non-hex values like CSS variables
• Apply clamping after fade-to-inactive step for proper composition

src/static/js/ace2_inner.ts

---

View more (4)

4. src/tests/backend/specs/colorutils.ts 🧪 Tests +78/-0

Add comprehensive unit tests for color utilities

• Add unit tests for relativeLuminance() against black, white, and WCAG reference values
• Add unit tests for contrastRatio() including edge cases and the #cc0000 failure case
• Add unit tests for ensureReadableBackground() covering light, dark, and mid-tone backgrounds
• Test custom minContrast parameter for AAA compliance

src/tests/backend/specs/colorutils.ts

---

5. doc/docker.md 📝 Documentation +1/-0

Document new environment variable

• Add documentation row for PAD_OPTIONS_ENFORCE_READABLE_AUTHOR_COLORS environment variable
• Include description of the feature and default value

doc/docker.md

---

6. settings.json.docker ⚙️ Configuration changes +2/-1

Add setting to Docker configuration

• Add enforceReadableAuthorColors field to padOptions with environment variable substitution
• Set default to true via PAD_OPTIONS_ENFORCE_READABLE_AUTHOR_COLORS env var

settings.json.docker

---

7. settings.json.template ⚙️ Configuration changes +9/-1

Add setting to template configuration

• Add enforceReadableAuthorColors field to padOptions with default value true
• Include detailed comment explaining the feature, use cases, and opt-out mechanism

settings.json.template

---

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📎 Requirement gaps (0)

1. enforceReadableAuthorColors default is true ☑ 📘 Rule violation ≡ Correctness

Description

The new author-color clamping behavior is enabled by default via `enforceReadableAuthorColors:
true`, which changes baseline rendering behavior. This violates the requirement that new features be
feature-flagged and disabled by default.

Code

src/node/utils/Settings.ts[R411-415]

alwaysShowChat: false,
chatAndUsers: false,
lang: null,
+    enforceReadableAuthorColors: true,
},

Evidence

PR Compliance ID 9 requires new features to be behind a flag and disabled by default. The PR
introduces the enforceReadableAuthorColors flag but sets its default to true in Settings and the
default config templates, so the new behavior is on by default.

src/node/utils/Settings.ts[410-415]
settings.json.template[261-273]
settings.json.docker[280-293]
Best Practice: Repository guidelines

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new viewer-side author background clamping feature is enabled by default (`enforceReadableAuthorColors: true`), but compliance requires new features to be disabled by default.
## Issue Context
The PR correctly introduces a flag (`enforceReadableAuthorColors`) and wiring, but the default value is set to `true` in server defaults and the distributed config templates.
## Fix Focus Areas
- src/node/utils/Settings.ts[410-415]
- settings.json.template[261-273]
- settings.json.docker[280-293]
- doc/docker.md[99-113]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Clamp doesn't ensure AA ☑ 🐞 Bug ≡ Correctness

Description

After clamping the background with ensureReadableBackground(), setAuthorStyle() still chooses the
foreground via textColorFromBackgroundColor()’s 0.5 luminosity heuristic, so it can render white (or
#222) text on a background that was only validated against pure black. This breaks the PR’s
guarantee that rendered author highlights meet WCAG AA contrast.

Code

src/static/js/ace2_inner.ts[R248-256]

+      const enforceReadable =
+          window.clientVars.padOptions == null ||
+          window.clientVars.padOptions.enforceReadableAuthorColors !== false;
+      if (enforceReadable && colorutils.isCssHex(bgcolor)) {
+        bgcolor = colorutils.ensureReadableBackground(bgcolor);
+      }
 const textColor =
     colorutils.textColorFromBackgroundColor(bgcolor, window.clientVars.skinName);
 const styles = [

Evidence

ace2_inner clamps bgcolor (hex only) but then immediately computes textColor using the pre-existing
luminosity threshold, with no WCAG-based coordination between chosen text color and the clamp
target. Additionally, ensureReadableBackground() computes contrast against [0,0,0] (pure black), but
textColorFromBackgroundColor()’s “black” for non-colibris is '#222' (and for colibris is a CSS var),
so even “passes vs black” does not imply passes vs the actual rendered foreground.

src/static/js/ace2_inner.ts[234-265]
src/static/js/colorutils.ts[115-120]
src/static/js/colorutils.ts[157-173]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`setAuthorStyle()` clamps the background using WCAG contrast math but then selects the text color using a luminosity heuristic that can pick a foreground that does **not** meet the WCAG threshold with the (possibly clamped) background.
### Issue Context
- `ensureReadableBackground()` currently validates against pure black (`[0,0,0]`), but the actual dark foreground used by `textColorFromBackgroundColor()` is `#222` (or a CSS var for the colibris skin).
- To actually guarantee AA on render, the chosen foreground and the clamped background must be computed together using the same colors.
### Fix Focus Areas
- src/static/js/ace2_inner.ts[242-263]
- src/static/js/colorutils.ts[115-120]
- src/static/js/colorutils.ts[139-173]
### Suggested approach
- Update the author-style rendering to:
1) Determine the actual candidate foreground colors (e.g., `#fff` and `#222` for classic skins; consider handling colibris separately).
2) For hex backgrounds, choose the foreground by **highest WCAG contrastRatio** (not luminosity).
3) If neither candidate meets `minContrast`, clamp the background toward white until it meets `minContrast` against the chosen dark foreground (likely `#222`), then set the foreground explicitly.
- Alternatively, make a helper that returns `{bg, fg}` together (single source of truth), and have `setAuthorStyle()` use it.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

3. PadOption type missing key 🐞 Bug ⚙ Maintainability

Description

The new pad option enforceReadableAuthorColors is added to settings and read on the client, but the
client-side PadOption TypeScript type does not include it. This creates a type-safety gap and will
force casts for any typed consumer accessing the new option.

Code

src/node/utils/Settings.ts[R203-206]

alwaysShowChat: boolean,
chatAndUsers: boolean,
lang: string | null,
+    enforceReadableAuthorColors: boolean,

Evidence

Settings adds padOptions.enforceReadableAuthorColors and the client reads it, but the PadOption type
definition does not declare the property, so typed code cannot safely access it.

src/node/utils/Settings.ts[200-207]
src/static/js/ace2_inner.ts[248-253]
src/static/js/types/SocketIOMessage.ts[243-256]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The new `padOptions.enforceReadableAuthorColors` setting is used by client code but is not represented in the `PadOption` TypeScript type.
### Issue Context
This is not a runtime bug, but it weakens type safety and will cause TS friction for future code that wants to read the option in a typed way.
### Fix Focus Areas
- src/static/js/types/SocketIOMessage.ts[243-256]
### Suggested approach
Add an optional boolean field to `PadOption`:
- `"enforceReadableAuthorColors"?: boolean`
so it matches the settings/clientVars payload.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

ⓘ The new review experience is currently in Beta. Learn more