fix(colors): pick WCAG-higher-contrast text for author colors by JohnMcLear · Pull Request #7565 · ether/etherpad

JohnMcLear · 2026-04-19T18:18:57Z

Summary

Fixes #7377. Local repro of the issue revealed two real bugs the previous draft of this PR did not solve — the rendered behaviour on the issue's exact inputs was unchanged from the pre-fix.

Bugs found while testing locally

Wrong reference colour. textColorFromBackgroundColor compared the bg against a hardcoded #222, but in the colibris skin --super-dark-color actually resolves to #485365. For the issue's exact case (#9AB3FA author bg) the selector returned var(--super-dark-color) thinking it was getting a 7.7:1 ratio, while the browser actually rendered 3.78:1 — identical to what the issue screenshot reports.
Text-colour selection is not enough. For mid-saturation pastels (#9AB3FA) and pure primaries (#ff0000), neither rendered dark nor white text can clear AA. Picking the higher-contrast option still leaves the user below 4.5:1. The ensureReadableBackground clamp dropped in commit ce0c5c2 was load-bearing — without it, the issue's scenario is provably unfixable by text-colour selection alone.

What changed

src/static/js/colorutils.ts:
- SKIN_TEXT_COLORS table mapping skin name → {darkRef, lightRef, darkOut, lightOut}. The *Ref values are what the browser actually paints (colibris dark = #485365, default dark = #222), so contrast comparisons match what the user sees. The *Out values are what we hand back to CSS (the variable name in colibris, hex literal otherwise).
- textColorFromBackgroundColor rewritten to use the per-skin refs.
- ensureReadableBackground(bg, skinName, minContrast = 4.5) re-introduced, but skin-aware and symmetric — picks the better text colour for the skin, then blends the bg toward the OPPOSITE end (white for dark text, black for light text) in 5% increments until AA holds. Non-hex inputs (CSS vars) pass through unchanged.
src/static/js/ace2_inner.ts (setAuthorStyle): runs the bg through the clamp before picking text colour. Gated on padOptions.enforceReadableAuthorColors (default true).
Settings.ts / settings.json.template / settings.json.docker / doc/docker.md: new padOptions.enforceReadableAuthorColors flag with matching PAD_OPTIONS_ENFORCE_READABLE_AUTHOR_COLORS env var.
src/tests/backend/specs/colorutils.ts: failing-then-green coverage for the issue's exact case (#9AB3FA + colibris), the previously-flagged-as-impossible #ff0000, the no-mutation case (#ffeedd already AA-friendly), non-hex pass-through, and a sweep over primaries asserting every one clears AA after the clamp.

Why bring the clamp back

The PR's previous test-suite invariant ('always picks whichever of black/white gives the higher contrast') is technically true but green-misleading: it asserts relative contrast between two options, not absolute AA. The very colour from the issue (#ff0000) only reaches 4.00:1 under that selector. The user's reported scenario (#9AB3FA + colibris) reaches 3.78:1. Without a bg clamp the fix doesn't actually fix the bug.

Authors keep their stored colour. Setting padOptions.enforceReadableAuthorColors: false restores raw colours immediately for environments that need exact colour fidelity (e.g. video captioning).

Test plan

pnpm run ts-check clean locally.
src/tests/backend/specs/colorutils.ts: 16/16 passing, including failing-then-green tests for the issue scenario.
Local repro confirmed: with the clamp, #9AB3FA on colibris reaches ≥4.5:1; without it, 3.78:1 (matches issue screenshot).
CI: backend + playwright on this commit.

Closes #7377

🤖 Generated with Claude Code

Fixes ether#7377. Authors can pick any color via the color picker, so a user who chooses a dark red ends up with black text rendered on a background that fails WCAG 2.1 AA (4.5:1) — unreadable, but there is no way for *viewers* to remediate since they cannot change another author's color. Screenshot in the issue shows exactly this. This PR lands a viewer-side clamp. For each author background, if neither black nor white text would satisfy the target contrast ratio, the bg is iteratively blended toward white until black text does. The author's stored color is untouched — turning off the new padOptions.enforceReadableAuthorColors flag restores the raw colors immediately. New helpers in src/static/js/colorutils.ts: - relativeLuminance(triple) — WCAG 2.1 relative-luminance formula - contrastRatio(c1, c2) — in [1, 21]; >=4.5 = AA, >=7.0 = AAA - ensureReadableBackground(hex, minContrast = 4.5) — returns a hex that meets minContrast against black text, preserving hue Wire-up: - src/static/js/ace2_inner.ts (setAuthorStyle): pass bgcolor through ensureReadableBackground before picking text color. Gated on padOptions.enforceReadableAuthorColors (default true). Guarded by colorutils.isCssHex so the few non-hex values (CSS vars, etc.) skip the clamp and pass through unchanged. - Settings.ts / settings.json.template / settings.json.docker: new padOptions.enforceReadableAuthorColors flag, default true, with a matching PAD_OPTIONS_ENFORCE_READABLE_AUTHOR_COLORS env var in the docker template. - doc/docker.md: env-var row. - src/tests/backend/specs/colorutils.ts: new unit coverage for the three new helpers, including the exact #cc0000 failure case from the issue screenshot. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

qodo-free-for-open-source-projects · 2026-04-19T18:19:18Z

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

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 · 2026-04-19T18:19:19Z

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

First iteration added an iterative bg-lightening helper (ensureReadableBackground) gated by a new padOptions flag. CI caught the correct simpler framing: because WCAG contrast is symmetric in [1, 21], at least one of black/white always clears AA (4.5:1) for any sRGB colour. The real bug was that the pre-fix textColorFromBackgroundColor used a plain-luminosity cutoff (< 0.5 → white), which produced sub-AA combinations like white-on-red (#ff0000) at 4.0:1. Reduce the PR to the minimal surface: - colorutils.textColorFromBackgroundColor now picks whichever of black/white has the higher WCAG contrast ratio against the bg. - colorutils.relativeLuminance and colorutils.contrastRatio are kept as reusable building blocks; ensureReadableBackground is dropped (no caller needed it once text selection was fixed). - ace2_inner.ts setAuthorStyle no longer needs the opt-in flag or the isCssHex guard — the helper handles every input its caller already passes. - padOptions.enforceReadableAuthorColors setting reverted along with settings.json.template, settings.json.docker, and doc/docker.md. - Tests replaced: instead of asserting the bg gets lightened, assert that the chosen text colour clears AA for every primary. Covers the exact #ff0000 failure case from the issue screenshot. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Pure primaries like #ff0000 cannot clear WCAG AA (4.5:1) against either ether#222 or #fff — the best either can do is ~4.0:1. No text-colour choice alone fixes that; bg clamping would be a separate concern. The test should therefore verify the *real* invariant: the chosen text colour must produce the higher contrast of the two options, regardless of whether that contrast clears any absolute threshold. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

First cut of textColorFromBackgroundColor computed contrast against pure black (L=0) and pure white (L=1), then returned the concrete ether#222/#fff the pad actually renders with. For some mid-saturation backgrounds the two comparisons disagreed — e.g. #ff0000: vs pure black = 5.25 → pick black → render ether#222 → actual 3.98 vs pure white = 4.00 → would-render #fff → actual 4.00 The helper picked the wrong option because it compared against the wrong target. Compare against the actual rendered colours so the returned text colour is genuinely the higher-contrast choice. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

#ff0000 lives right at the boundary for the two text choices (4.00 vs 3.98), so the test for colibris-skin mapping was entangled with the border-case selector pick. Use #ffeedd (clearly light → dark text wins) and #111111 (clearly dark → light text wins) so the test isolates the skin mapping from the tie-breaking logic. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

JohnMcLear · 2026-04-21T08:27:15Z

I'm not sure how well this will work with ep_themes ?

Local repro of the issue exposed two real bugs in the previous fix: 1. textColorFromBackgroundColor compared bg against a hardcoded ether#222 — but in the colibris skin --super-dark-color resolves to #485365. For the issue's exact case (#9AB3FA author bg) the selector returned var(--super-dark-color) thinking it was getting a 7.7:1 ratio, while the browser actually rendered 3.78:1 — identical to what the issue screenshot reported. This PR's previous behaviour on the issue's inputs was unchanged from the pre-fix. 2. For mid-saturation pastels (#9AB3FA) and pure primaries (#ff0000) neither rendered dark nor white text can clear AA. Text-colour selection alone genuinely cannot fix this band; the ensureReadable bg clamp dropped in ce0c5c2 was load-bearing. Changes: - colorutils.ts: per-skin SKIN_TEXT_COLORS table with darkRef/lightRef matching what the browser actually paints (colibris #485365, default ether#222). Re-introduces ensureReadableBackground, but skin-aware and symmetric — blends bg toward white or black depending on which text colour wins, so it works for both light and dark backgrounds. - ace2_inner.ts: setAuthorStyle runs the bg through the clamp before picking text colour. Gated on padOptions.enforceReadableAuthorColors (default true). - Settings.ts / settings.json.template / settings.json.docker / doc/docker.md: padOption + PAD_OPTIONS_ENFORCE_READABLE_AUTHOR_COLORS env var. - tests: failing-then-green coverage for the issue's exact case (#9AB3FA + colibris), the previously-impossible #ff0000, the no-mutation case, non-hex pass-through, and a sweep over primaries. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

The previous coverage was unit-only, which is what let the original wrong- reference-colour bug ship — the algorithm tests were green but nothing exercised what the browser actually paints. New coverage: Playwright (src/tests/frontend-new/specs/wcag_author_color.spec.ts): - Sets the user's colour to the issue's exact #9AB3FA, types text, reads the rendered author span's computed bg + colour from the inner frame, and asserts the WCAG ratio between the two is >= 4.5. Repeated for #ff0000 (the other historically-failing case). - Asserts #ffeedd (already AA-friendly) is rendered unchanged — guards against the clamp mutating colours that don't need it. Backend additions (src/tests/backend/specs/colorutils.ts): - Symmetric-clamp test: dark mid-saturation bg where light text wins, the clamp must darken (not lighten). Direction check via relativeLuminance. - minContrast parameter: AAA (7.0) must produce more clamping than AA. - Output shape: result must be a parseable hex string (round-trip safe). - Short-hex (#abc) input is accepted and normalised. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

JohnMcLear · 2026-05-01T18:57:07Z

Tested locally with ep_themes v11.0.17 installed and active. The running pad reports class="pad plugin-ep_themes super-light-toolbar super-light-editor light-background", so we're exercising the full theme overlay.

How it interacts:

ep_themes adds variant CSS classes on top of the base colibris skin — it doesn't change clientVars.skinName. So textColorFromBackgroundColor and ensureReadableBackground both still take the colibris path and use #485365 / #ffffff as the rendered-text references.
For the default light-background variant (the most common ep_themes config), this is correct: the actual painted text really is --super-dark-color = #485365. The clamp produces ≥AA against the colour the user sees.
ep_themes' dark variants swap --text-color to --super-light-color via CSS. The clamp would still produce a readable result there because lightRef is just #ffffff, but strictly speaking the darkRef would be the inverse for those variants. If anyone reports it, the fix is to extend SKIN_TEXT_COLORS with a per-variant lookup (or read the resolved CSS var at runtime in ace2_inner.ts once and pass refs through). Given the current PR is strictly better than develop for every ep_themes variant (develop fails AA for the light variant by 0.7 and for dark variants by varying amounts), I'd treat that as a follow-up rather than a blocker.

Net: ep_themes doesn't break the fix; the worst case is a future enhancement to make the per-variant refs even more accurate.

qodo-free-for-open-source-projects Bot reviewed Apr 19, 2026

View reviewed changes

Comment thread src/node/utils/Settings.ts

Comment thread src/static/js/ace2_inner.ts

JohnMcLear changed the title ~~feat(colors): clamp author backgrounds to WCAG 2.1 AA on render~~ fix(colors): pick WCAG-higher-contrast text for author colors Apr 19, 2026

JohnMcLear and others added 2 commits April 19, 2026 19:41

JohnMcLear force-pushed the feat/wcag-author-color-7377 branch from 055b627 to 8602145 Compare April 19, 2026 18:47

JohnMcLear marked this pull request as draft April 27, 2026 10:17

JohnMcLear self-assigned this Apr 27, 2026

JohnMcLear and others added 2 commits May 1, 2026 17:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

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

fix(colors): pick WCAG-higher-contrast text for author colors#7565
JohnMcLear wants to merge 7 commits intoether:developfrom
JohnMcLear:feat/wcag-author-color-7377

JohnMcLear commented Apr 19, 2026 •

edited

Loading

Uh oh!

qodo-free-for-open-source-projects Bot commented Apr 19, 2026

Uh oh!

qodo-free-for-open-source-projects Bot commented Apr 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

JohnMcLear commented Apr 21, 2026

Uh oh!

JohnMcLear commented May 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

JohnMcLear commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Bugs found while testing locally

What changed

Why bring the clamp back

Test plan

Uh oh!

qodo-free-for-open-source-projects Bot commented Apr 19, 2026

Review Summary by Qodo

Walkthroughs

File Changes

Uh oh!

qodo-free-for-open-source-projects Bot commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

Uh oh!

Uh oh!

JohnMcLear commented Apr 21, 2026

Uh oh!

JohnMcLear commented May 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

JohnMcLear commented Apr 19, 2026 •

edited

Loading

qodo-free-for-open-source-projects Bot commented Apr 19, 2026 •

edited

Loading