From 63916dfeff18ce34299827c4356e6cb0d65ca5bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Thu, 9 Jul 2026 12:10:17 +0000 Subject: [PATCH 1/3] fix(cli): read SVG fill instead of CSS color for contrast-audit foreground detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit SVG text (//) is painted via the `fill` property, not `color` — the two are independent in SVG. When a composition sets `fill` without also setting `color`, getComputedStyle(el).color resolves to the inherited/initial value (often black), which does not match what's actually rendered on screen. The contrast audit was reading `cs.color` unconditionally, so any SVG text with fill != color got a fabricated foreground and a false pass/fail verdict. Add an SVG-aware foreground read: for elements inside an (detected via ownerSVGElement), prefer the computed `fill` when it resolves to a solid rgb()/rgba() color, falling back to `color` for paint values that aren't a plain color (none, context-fill, gradient/pattern refs) so we never crash parseColor or report a fabricated black. Mirrored the same fix in skills/hyperframes-creative/scripts/contrast-report.mjs, which duplicates the DOM-walk/foreground-read logic (not just the WCAG math), and updated the header sync note (the referenced path had also drifted to skills/hyperframes-creative). Extracted the pure decision into contrast-fg.ts (mirroring the existing contrast-bg.ts pattern) with unit tests, since the browser-injected script itself can't be unit-tested without a real page. Manually verified with a standalone puppeteer-core repro against the cached chrome-headless-shell binary: an SVG with no `color` set on a black background. Before: fg=rgb(0,0,0) bg=rgb(0,0,0) ratio=1 wcagAA=false (false fail) After: fg=rgb(255,255,255) bg=rgb(0,0,0) ratio=21 wcagAA=true (correct) Also verified fill: none and fill: url(#gradient) fall back to `color` without crashing. Scope: this addresses only the SVG fill-vs-color mismatch. The other 4 reported contrast-audit false-positive patterns (cross-comp color bleed, solid-fill button/pill, translucent glass-text layers, translucent decorative/animated elements) are out of scope and remain open. --- .../src/commands/contrast-audit.browser.js | 45 +++++++++++++- packages/cli/src/commands/contrast-fg.test.ts | 61 +++++++++++++++++++ packages/cli/src/commands/contrast-fg.ts | 42 +++++++++++++ skills-manifest.json | 2 +- .../scripts/contrast-report.mjs | 23 ++++++- 5 files changed, 168 insertions(+), 5 deletions(-) create mode 100644 packages/cli/src/commands/contrast-fg.test.ts create mode 100644 packages/cli/src/commands/contrast-fg.ts diff --git a/packages/cli/src/commands/contrast-audit.browser.js b/packages/cli/src/commands/contrast-audit.browser.js index 3c48e2ef55..ae89ff35e0 100644 --- a/packages/cli/src/commands/contrast-audit.browser.js +++ b/packages/cli/src/commands/contrast-audit.browser.js @@ -2,8 +2,9 @@ // Loaded as a raw string and injected via page.addScriptTag to avoid // esbuild mangling (page.evaluate serializes functions; __name helpers break). // -// NOTE: WCAG math (relLum, wcagRatio, parseColor, median) is duplicated in -// skills/hyperframes/scripts/contrast-report.mjs — keep in sync. +// NOTE: WCAG math (relLum, wcagRatio, parseColor, median) plus the DOM-walk / +// foreground-color-read logic is duplicated in +// skills/hyperframes-creative/scripts/contrast-report.mjs — keep in sync. /* eslint-disable */ window.__contrastAudit = async function (imgBase64, time) { @@ -32,6 +33,36 @@ window.__contrastAudit = async function (imgBase64, time) { return [p[0], p[1], p[2], p[3] != null ? p[3] : 1]; } + // Like parseColor, but returns null instead of defaulting to black when the + // value isn't a solid rgb()/rgba() color — e.g. SVG paint keywords such as + // "none"/"context-fill", or a gradient/pattern reference like + // 'url("#grad")'. Callers should fall back to another source of truth + // rather than trust a fabricated black. + function tryParseSolidColor(c) { + var m = c.match(/rgba?\(([^)]+)\)/); + if (!m) return null; + var p = m[1].split(",").map(function (s) { + return parseFloat(s.trim()); + }); + if ( + p.some(function (v) { + return isNaN(v); + }) + ) + return null; + return [p[0], p[1], p[2], p[3] != null ? p[3] : 1]; + } + + // SVG text (, , ) is painted via the `fill` + // property, not `color` — the two are independent CSS properties in SVG. + // A page can set `fill` (inline style, `fill` attribute, or a CSS rule) + // without ever touching `color`, in which case getComputedStyle(el).color + // resolves to the inherited/initial value (often black) and does not + // reflect what's actually rendered on screen. + function isSvgTextElement(el) { + return !!el.ownerSVGElement; + } + function selectorOf(el) { if (el.id) return "#" + el.id; var cls = Array.from(el.classList).slice(0, 2).join("."); @@ -151,7 +182,15 @@ window.__contrastAudit = async function (imgBase64, time) { if (rect.right <= 0 || rect.bottom <= 0 || rect.left >= w || rect.top >= h) continue; if (isClippedAway(el, rect)) continue; - var fg = parseColor(cs.color); + // For SVG text, `fill` is the paint that's actually rendered; `color` is + // frequently just the inherited/initial value and unrelated to what's on + // screen. Only trust `fill` when it resolves to a solid color — "none", + // "context-fill", and gradient/pattern refs (url(#...)) fall back to + // `color` rather than crashing parseColor or reporting a fabricated + // black. + var fg = isSvgTextElement(el) + ? tryParseSolidColor(cs.fill) || parseColor(cs.color) + : parseColor(cs.color); if (fg[3] <= 0.01) continue; // Prefer an opaque own/ancestor background-color over the pixel ring. A diff --git a/packages/cli/src/commands/contrast-fg.test.ts b/packages/cli/src/commands/contrast-fg.test.ts new file mode 100644 index 0000000000..81e5bfd8fa --- /dev/null +++ b/packages/cli/src/commands/contrast-fg.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from "vitest"; +import { resolveForegroundColor } from "./contrast-fg.js"; + +describe("resolveForegroundColor", () => { + it("uses `color` for ordinary HTML text", () => { + expect( + resolveForegroundColor({ + isSvgText: false, + fill: "rgb(255, 255, 255)", + color: "rgb(0, 0, 0)", + }), + ).toEqual([0, 0, 0, 1]); + }); + + it("uses `fill` for SVG text when fill is set but color is not (the reported bug)", () => { + // fill: white on a dark bg, no `color` set — getComputedStyle(el).color + // resolves to the inherited/initial black, which does not match what's + // actually rendered. The audit must read `fill`, not `color`, here. + expect( + resolveForegroundColor({ + isSvgText: true, + fill: "rgb(255, 255, 255)", + color: "rgb(0, 0, 0)", + }), + ).toEqual([255, 255, 255, 1]); + }); + + it("preserves fill alpha", () => { + expect( + resolveForegroundColor({ + isSvgText: true, + fill: "rgba(10, 20, 30, 0.5)", + color: "rgb(0, 0, 0)", + }), + ).toEqual([10, 20, 30, 0.5]); + }); + + it("falls back to `color` when fill is 'none'", () => { + expect( + resolveForegroundColor({ isSvgText: true, fill: "none", color: "rgb(0, 255, 0)" }), + ).toEqual([0, 255, 0, 1]); + }); + + it("falls back to `color` when fill is 'context-fill'", () => { + expect( + resolveForegroundColor({ isSvgText: true, fill: "context-fill", color: "rgb(0, 255, 0)" }), + ).toEqual([0, 255, 0, 1]); + }); + + it("falls back to `color` when fill is a gradient/pattern reference", () => { + expect( + resolveForegroundColor({ isSvgText: true, fill: 'url("#grad")', color: "rgb(0, 255, 0)" }), + ).toEqual([0, 255, 0, 1]); + }); + + it("never crashes on garbage fill values", () => { + expect(() => + resolveForegroundColor({ isSvgText: true, fill: "currentcolor", color: "rgb(1, 2, 3)" }), + ).not.toThrow(); + }); +}); diff --git a/packages/cli/src/commands/contrast-fg.ts b/packages/cli/src/commands/contrast-fg.ts new file mode 100644 index 0000000000..71e2ad099c --- /dev/null +++ b/packages/cli/src/commands/contrast-fg.ts @@ -0,0 +1,42 @@ +// Pure foreground-color resolution logic for the WCAG contrast audit. +// +// The browser-side audit (contrast-audit.browser.js) reads an element's +// foreground paint color from getComputedStyle(el).color. That is correct for +// ordinary HTML text, but SVG text (, , ) is painted +// via the `fill` property — `fill` and `color` are independent CSS properties +// in SVG. A page can set `fill` (inline style, a `fill` attribute, or a CSS +// rule) without ever touching `color`, in which case `color` resolves to the +// inherited/initial value (often black) and does not reflect what's actually +// rendered on screen. That mismatch was reported as a real false pass/fail +// in the contrast audit. +// +// This module hosts the pure decision so it can be unit-tested without a +// browser. The same logic is inlined into contrast-audit.browser.js (which is +// injected as a raw string and cannot import) and into +// skills/hyperframes-creative/scripts/contrast-report.mjs — keep all three in +// sync, mirroring the existing "WCAG math is duplicated" note at the top of +// contrast-audit.browser.js. + +import { parseColorRGBA, type Rgba } from "./contrast-bg.js"; + +/** + * Resolve the foreground paint color for a text-bearing element. + * + * - For ordinary HTML text, this is always the computed `color`. + * - For SVG text, the rendered glyph color is `fill`, not `color`. `fill` is + * only trusted when it resolves to a solid rgb()/rgba() color — SVG paint + * keywords ("none", "context-fill") and gradient/pattern references + * (`url(#...)`) are not colors, so those fall back to `color` instead of + * fabricating black. + */ +export function resolveForegroundColor(opts: { + isSvgText: boolean; + fill: string; + color: string; +}): Rgba { + if (opts.isSvgText) { + const solid = parseColorRGBA(opts.fill); + if (solid) return solid; + } + return parseColorRGBA(opts.color) ?? [0, 0, 0, 1]; +} diff --git a/skills-manifest.json b/skills-manifest.json index 95f09abfed..e213328018 100644 --- a/skills-manifest.json +++ b/skills-manifest.json @@ -34,7 +34,7 @@ "files": 14 }, "hyperframes-creative": { - "hash": "a5bccc25d291899d", + "hash": "da874dbcebdd84cc", "files": 69 }, "hyperframes-keyframes": { diff --git a/skills/hyperframes-creative/scripts/contrast-report.mjs b/skills/hyperframes-creative/scripts/contrast-report.mjs index ee9b7835b8..43eea07b4d 100644 --- a/skills/hyperframes-creative/scripts/contrast-report.mjs +++ b/skills/hyperframes-creative/scripts/contrast-report.mjs @@ -120,6 +120,24 @@ async function probeTextElements(session, _t) { const parts = m[1].split(",").map((s) => parseFloat(s.trim())); return [parts[0], parts[1], parts[2], parts[3] ?? 1]; }; + // Like parseColor, but returns null instead of defaulting to black when + // the value isn't a solid rgb()/rgba() color — e.g. SVG paint keywords + // such as "none"/"context-fill", or a gradient/pattern reference like + // 'url("#grad")'. Callers should fall back to another source of truth + // rather than trust a fabricated black. + const tryParseSolidColor = (c) => { + const m = c.match(/rgba?\(([^)]+)\)/); + if (!m) return null; + const parts = m[1].split(",").map((s) => parseFloat(s.trim())); + if (parts.some((v) => Number.isNaN(v))) return null; + return [parts[0], parts[1], parts[2], parts[3] ?? 1]; + }; + // SVG text (, , ) is painted via the `fill` + // property, not `color` — the two are independent CSS properties in + // SVG. A page can set `fill` without ever touching `color`, in which + // case getComputedStyle(el).color resolves to the inherited/initial + // value (often black) and does not reflect what's actually rendered. + const isSvgTextElement = (el) => !!el.ownerSVGElement; const selectorOf = (el) => { if (el.id) return `#${el.id}`; const cls = [...el.classList].slice(0, 2).join("."); @@ -137,10 +155,13 @@ async function probeTextElements(session, _t) { if (parseFloat(cs.opacity) <= 0.01) continue; const rect = el.getBoundingClientRect(); if (rect.width < 8 || rect.height < 8) continue; + const fg = isSvgTextElement(el) + ? tryParseSolidColor(cs.fill) || parseColor(cs.color) + : parseColor(cs.color); out.push({ selector: selectorOf(el), text: el.textContent.trim().slice(0, 60), - fg: parseColor(cs.color), + fg, fontSize: parseFloat(cs.fontSize), fontWeight: Number(cs.fontWeight) || 400, bbox: { x: rect.x, y: rect.y, w: rect.width, h: rect.height }, From e0ff3b732c015feb94e4d08abb2c634d80a2a63d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Thu, 9 Jul 2026 15:50:10 +0000 Subject: [PATCH 2/3] fix(cli): sample the real pixels behind hidden text for contrast-audit backgrounds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The contrast audit estimated an element's background by sampling a 4px ring just outside its bounding box. That proximity heuristic is wrong whenever what's immediately outside the text differs from what's actually behind it: - cross-component bleed: text sits near the edge of its own panel/ layer, and a differently-colored sibling panel/layer starts just outside the text's bbox, so the ring samples the neighbor instead of the real background. - backdrop-filter glass text: a translucent blurred panel sized only a couple pixels larger than the text — the ring exits the panel and samples the raw, unblurred, untinted pixels behind it. - partially-overlapping translucent decoration: a decorative shape that only partly overlaps the ring, or sits entirely INSIDE the text's own bbox, which the ring can never see regardless of size. (Solid-fill pill/button backgrounds were already handled correctly by the existing own-background ancestor walk and are unaffected by this change — confirmed via repro, not touched.) Fix: hide each candidate text element's own paint (color/fill set to transparent, layout-neutral — no reflow), take ONE screenshot with the glyphs invisible, then sample the real composited pixels directly INSIDE the element's own bbox. No proximity heuristic needed since we're reading the exact pixels that were behind the glyphs. Same number of screenshots as before (one per sample time), just moved to after the hide instead of before it. This changes contrast-audit.browser.js from a single __contrastAudit call into a two-phase __contrastAuditPrepare / __contrastAuditFinish contract (see validate.ts's runContrastAudit for the calling side, with a try/finally restore-safety-net so a mid-loop failure can't leave later samples auditing a page with stale hidden text). Mirrored the same change in skills/hyperframes-creative/scripts/contrast-report.mjs, which duplicates the same DOM-walk/sampling logic — there the visible frame still comes from the producer's normal capture path (for the overlay image); only the background-sampling capture is a plain page.screenshot() after hiding text, deliberately bypassing the capture pipeline's static-frame dedup cache, which knows nothing about the DOM mutation and would hand back a stale pre-mutation buffer. Added contrast-sample.ts (mirroring the existing contrast-bg.ts / contrast-fg.ts pattern) hosting the pure sample-rect/grid-point computation, unit tested, since the two browser-injected scripts can't import it directly. Verified via a standalone puppeteer-core harness against real fixtures for each pattern, then end-to-end through the actual `hyperframes validate --contrast` CLI command against a real scaffolded project: before this change, a text run sitting mostly on a translucent decorative badge reported as a false PASS (ring never sees decoration fully inside the bbox); after, it correctly reports ~1.1:1 and fails. A cross-component-bleed case and a backdrop-filter glass-text case that previously reported false FAILs (ring bleeding into a neighboring panel / the raw unblurred backdrop) now correctly report as passing, matching their true rendered contrast. --- .../src/commands/contrast-audit.browser.js | 315 +++++++++++------- packages/cli/src/commands/contrast-bg.ts | 26 +- .../cli/src/commands/contrast-sample.test.ts | 64 ++++ packages/cli/src/commands/contrast-sample.ts | 89 +++++ .../src/commands/layout-audit.browser.test.ts | 28 +- packages/cli/src/commands/validate.ts | 59 +++- skills-manifest.json | 2 +- .../scripts/contrast-report.mjs | 183 +++++++--- 8 files changed, 573 insertions(+), 193 deletions(-) create mode 100644 packages/cli/src/commands/contrast-sample.test.ts create mode 100644 packages/cli/src/commands/contrast-sample.ts diff --git a/packages/cli/src/commands/contrast-audit.browser.js b/packages/cli/src/commands/contrast-audit.browser.js index ae89ff35e0..b542f2b135 100644 --- a/packages/cli/src/commands/contrast-audit.browser.js +++ b/packages/cli/src/commands/contrast-audit.browser.js @@ -2,26 +2,59 @@ // Loaded as a raw string and injected via page.addScriptTag to avoid // esbuild mangling (page.evaluate serializes functions; __name helpers break). // -// NOTE: WCAG math (relLum, wcagRatio, parseColor, median) plus the DOM-walk / -// foreground-color-read logic is duplicated in +// Two-phase API — see packages/cli/src/commands/validate.ts's +// runContrastAudit() for the calling contract: +// +// 1. window.__contrastAuditPrepare() walks the DOM for text-bearing +// elements, computes each one's foreground paint (CSS `color`, or SVG +// `fill` for SVG text — fill and color are independent CSS properties +// in SVG), and HIDES that element's own text paint (color/fill set to +// transparent). Hiding is layout-neutral — it doesn't reflow anything — +// so the caller can screenshot the frame right after with the glyphs +// invisible but everything else unchanged. Returns the candidate list +// (selector, text, fg, bbox, font metrics). +// 2. Caller takes ONE screenshot (page.screenshot()) — same number of +// screenshots as before, just moved to after prepare() instead of +// before it. +// 3. window.__contrastAuditFinish(imgBase64, time, candidates) restores +// the original paint FIRST (so a slow/failed decode can't leave the +// page's text stuck invisible), then decodes the screenshot and, for +// each candidate, samples the real composited pixels directly INSIDE +// its own bounding box for the true background. +// +// Why sample inside the box instead of the 4px ring just outside it (the +// previous approach): the ring is a proximity heuristic that's wrong +// whenever what's immediately outside the text differs from what's actually +// behind it — a neighboring panel/component just past the text's edge, a +// rounded pill/button whose corner falls inside the ring, a +// backdrop-filter-blurred glass panel sized only a couple pixels larger +// than the text, or a translucent decoration that only partially overlaps +// the ring (or sits entirely inside the bbox, never touching the ring at +// all). Hiding the glyphs and sampling their own box side-steps all of +// that: it reads the exact pixels that were behind them. +// +// window.__contrastAuditRestoreIfPending() is a safety net: if the caller's +// screenshot or finish() call throws between prepare() and finish(), calling +// this restores any still-hidden paint so the next sample in the loop +// doesn't audit a page with stale invisible text. It's a no-op after a +// normal finish() call. +// +// NOTE: this logic (DOM-walk, foreground/paint-hide, background sampling) +// plus the pure WCAG math (relLum, wcagRatio, median) is duplicated in // skills/hyperframes-creative/scripts/contrast-report.mjs — keep in sync. +// The pure "which rect to sample" decision is also mirrored in +// contrast-sample.ts (unit-tested there since this file can't import). /* eslint-disable */ -window.__contrastAudit = async function (imgBase64, time) { - function relLum(r, g, b) { - function ch(v) { - var s = v / 255; - return s <= 0.03928 ? s / 12.92 : Math.pow((s + 0.055) / 1.055, 2.4); - } - return 0.2126 * ch(r) + 0.7152 * ch(g) + 0.0722 * ch(b); - } - - function wcagRatio(r1, g1, b1, r2, g2, b2) { - var l1 = relLum(r1, g1, b1), - l2 = relLum(r2, g2, b2); - var hi = l1 > l2 ? l1 : l2, - lo = l1 > l2 ? l2 : l1; - return (hi + 0.05) / (lo + 0.05); +window.__contrastAuditPrepare = function () { + // SVG text (, , ) is painted via the `fill` + // property, not `color` — the two are independent CSS properties in SVG. + // A page can set `fill` (inline style, `fill` attribute, or a CSS rule) + // without ever touching `color`, in which case getComputedStyle(el).color + // resolves to the inherited/initial value (often black) and does not + // reflect what's actually rendered on screen. + function isSvgTextElement(el) { + return !!el.ownerSVGElement; } function parseColor(c) { @@ -53,29 +86,12 @@ window.__contrastAudit = async function (imgBase64, time) { return [p[0], p[1], p[2], p[3] != null ? p[3] : 1]; } - // SVG text (, , ) is painted via the `fill` - // property, not `color` — the two are independent CSS properties in SVG. - // A page can set `fill` (inline style, `fill` attribute, or a CSS rule) - // without ever touching `color`, in which case getComputedStyle(el).color - // resolves to the inherited/initial value (often black) and does not - // reflect what's actually rendered on screen. - function isSvgTextElement(el) { - return !!el.ownerSVGElement; - } - function selectorOf(el) { if (el.id) return "#" + el.id; var cls = Array.from(el.classList).slice(0, 2).join("."); return cls ? el.tagName.toLowerCase() + "." + cls : el.tagName.toLowerCase(); } - function median(arr) { - var s = arr.slice().sort(function (a, b) { - return a - b; - }); - return s[Math.floor(s.length / 2)]; - } - function hasClipPath(el) { for (var ce = el; ce; ce = ce.parentElement) { var cp = getComputedStyle(ce).clipPath; @@ -115,28 +131,8 @@ window.__contrastAudit = async function (imgBase64, time) { return !paintsAnyProbePoint(el, rect); } - // Decode screenshot into canvas pixel data - var img = new Image(); - await new Promise(function (resolve) { - img.onload = resolve; - img.onerror = function () { - resolve(); - }; - img.src = "data:image/png;base64," + imgBase64; - }); - if (!img.naturalWidth) return []; - var canvas = document.createElement("canvas"); - canvas.width = img.naturalWidth || 1920; - canvas.height = img.naturalHeight || 1080; - var ctx = canvas.getContext("2d"); - if (!ctx) return []; - ctx.drawImage(img, 0, 0); - var px = ctx.getImageData(0, 0, canvas.width, canvas.height).data; - var w = canvas.width; - var h = canvas.height; - - // Walk DOM for text elements var out = []; + var restores = []; var walker = document.createTreeWalker(document.body, NodeFilter.SHOW_ELEMENT); var node; while ((node = walker.nextNode())) { @@ -179,7 +175,7 @@ window.__contrastAudit = async function (imgBase64, time) { if (ancHidden) continue; var rect = el.getBoundingClientRect(); if (rect.width < 8 || rect.height < 8) continue; - if (rect.right <= 0 || rect.bottom <= 0 || rect.left >= w || rect.top >= h) continue; + if (rect.right <= 0 || rect.bottom <= 0) continue; if (isClippedAway(el, rect)) continue; // For SVG text, `fill` is the paint that's actually rendered; `color` is @@ -188,84 +184,175 @@ window.__contrastAudit = async function (imgBase64, time) { // "context-fill", and gradient/pattern refs (url(#...)) fall back to // `color` rather than crashing parseColor or reporting a fabricated // black. - var fg = isSvgTextElement(el) - ? tryParseSolidColor(cs.fill) || parseColor(cs.color) - : parseColor(cs.color); + var isSvgText = isSvgTextElement(el); + var fg = isSvgText ? tryParseSolidColor(cs.fill) || parseColor(cs.color) : parseColor(cs.color); if (fg[3] <= 0.01) continue; - // Prefer an opaque own/ancestor background-color over the pixel ring. A - // caption/CTA that paints its OWN solid background (a pill, a button, a - // card) composites its text over THAT color, not over whatever surrounds - // the box. Sampling the ring there measured the text against the scene - // behind the element (often a dark photo) and reported false ~1:1 warnings - // for perfectly readable CTAs. Walk up until a fully-opaque background-color - // is found; stop and defer to the ring on the first background-image (text - // over real pixels). Keep this in sync with commands/contrast-bg.ts. - var ownBg = null; - for (var bn = el; bn && bn !== document.body; bn = bn.parentElement) { - var bcs = getComputedStyle(bn); - if (bcs.backgroundImage && bcs.backgroundImage !== "none") break; - var bc = parseColor(bcs.backgroundColor); - if (bc[3] >= 0.999) { - ownBg = [bc[0], bc[1], bc[2]]; - break; - } + var fontSize = parseFloat(cs.fontSize); + var fontWeight = Number(cs.fontWeight) || 400; + var large = fontSize >= 24 || (fontSize >= 19 && fontWeight >= 700); + + // Hide this element's OWN text paint so the caller's next screenshot + // reveals the true pixels behind the glyphs. Layout-neutral: color/fill + // never affect box geometry. `!important` beats any non-!important rule + // that might otherwise win on specificity; we restore the exact prior + // inline value (or remove the property entirely) afterward. + var origColor = el.style.getPropertyValue("color"); + var origColorPriority = el.style.getPropertyPriority("color"); + el.style.setProperty("color", "transparent", "important"); + var origFill = null, + origFillPriority = null; + if (isSvgText) { + origFill = el.style.getPropertyValue("fill"); + origFillPriority = el.style.getPropertyPriority("fill"); + el.style.setProperty("fill", "transparent", "important"); + } + restores.push({ + el: el, + origColor: origColor, + origColorPriority: origColorPriority, + origFill: origFill, + origFillPriority: origFillPriority, + isSvgText: isSvgText, + }); + + out.push({ + selector: selectorOf(el), + text: (el.textContent || "").trim().slice(0, 50), + fg: fg, + fontSize: fontSize, + fontWeight: fontWeight, + large: large, + bbox: { x: rect.x, y: rect.y, w: rect.width, h: rect.height }, + }); + } + + window.__contrastAuditRestores = restores; + return out; +}; + +function __contrastAuditRestoreAll() { + var restores = window.__contrastAuditRestores; + if (!restores) return; + for (var i = 0; i < restores.length; i++) { + var r = restores[i]; + if (r.origColor) r.el.style.setProperty("color", r.origColor, r.origColorPriority); + else r.el.style.removeProperty("color"); + if (r.isSvgText) { + if (r.origFill) r.el.style.setProperty("fill", r.origFill, r.origFillPriority); + else r.el.style.removeProperty("fill"); } + } + window.__contrastAuditRestores = null; +} + +// Safety net for the caller: if the screenshot or finish() call throws +// between prepare() and finish(), call this to restore any still-hidden +// paint so the next sample in the loop isn't auditing a page with stale +// invisible text. No-op if finish() already ran normally. +window.__contrastAuditRestoreIfPending = function () { + __contrastAuditRestoreAll(); +}; + +window.__contrastAuditFinish = async function (imgBase64, time, candidates) { + function relLum(r, g, b) { + function ch(v) { + var s = v / 255; + return s <= 0.03928 ? s / 12.92 : Math.pow((s + 0.055) / 1.055, 2.4); + } + return 0.2126 * ch(r) + 0.7152 * ch(g) + 0.0722 * ch(b); + } + + function wcagRatio(r1, g1, b1, r2, g2, b2) { + var l1 = relLum(r1, g1, b1), + l2 = relLum(r2, g2, b2); + var hi = l1 > l2 ? l1 : l2, + lo = l1 > l2 ? l2 : l1; + return (hi + 0.05) / (lo + 0.05); + } + + function median(arr) { + var s = arr.slice().sort(function (a, b) { + return a - b; + }); + return s[Math.floor(s.length / 2)]; + } + + // Restore original paint first — we already have the screenshot, and we + // never want a decode failure below to leave the page's text invisible. + __contrastAuditRestoreAll(); - var bgR, bgG, bgB; - if (ownBg) { - bgR = ownBg[0]; - bgG = ownBg[1]; - bgB = ownBg[2]; - } else { - // Sample 4px ring outside bbox for background color - var rr = [], - gg = [], - bb = []; - var x0 = Math.max(0, Math.floor(rect.x) - 4); - var x1 = Math.min(w - 1, Math.ceil(rect.x + rect.width) + 4); - var y0 = Math.max(0, Math.floor(rect.y) - 4); - var y1 = Math.min(h - 1, Math.ceil(rect.y + rect.height) + 4); - var sample = function (sx, sy) { - if (sx < 0 || sx >= w || sy < 0 || sy >= h) return; - var idx = (sy * w + sx) * 4; + var img = new Image(); + await new Promise(function (resolve) { + img.onload = resolve; + img.onerror = function () { + resolve(); + }; + img.src = "data:image/png;base64," + imgBase64; + }); + if (!img.naturalWidth) return []; + var canvas = document.createElement("canvas"); + canvas.width = img.naturalWidth || 1920; + canvas.height = img.naturalHeight || 1080; + var ctx = canvas.getContext("2d"); + if (!ctx) return []; + ctx.drawImage(img, 0, 0); + var px = ctx.getImageData(0, 0, canvas.width, canvas.height).data; + var w = canvas.width; + var h = canvas.height; + + var out = []; + for (var ci = 0; ci < candidates.length; ci++) { + var c = candidates[ci]; + var bbox = c.bbox; + + // Sample the element's OWN box (glyphs are hidden in this screenshot), + // inset 1px on each side to dodge anti-aliased edge pixels, clamped to + // the canvas. Mirrors contrast-sample.ts's computeSampleRect. + var x0 = Math.max(0, Math.round(bbox.x) + 1); + var x1 = Math.min(w - 1, Math.round(bbox.x + bbox.w) - 1); + var y0 = Math.max(0, Math.round(bbox.y) + 1); + var y1 = Math.min(h - 1, Math.round(bbox.y + bbox.h) - 1); + if (x1 <= x0 || y1 <= y0) continue; + + // Bounded grid, not a full scan — dense enough to catch a + // partially-overlapping decoration without turning a wide caption bar + // into thousands of samples. Mirrors contrast-sample.ts's + // sampleGridPoints. + var stepX = Math.max(1, Math.floor((x1 - x0) / 12)); + var stepY = Math.max(1, Math.floor((y1 - y0) / 6)); + var rr = [], + gg = [], + bb = []; + for (var y = y0; y <= y1; y += stepY) { + for (var x = x0; x <= x1; x += stepX) { + var idx = (y * w + x) * 4; rr.push(px[idx]); gg.push(px[idx + 1]); bb.push(px[idx + 2]); - }; - for (var x = x0; x <= x1; x++) { - sample(x, y0); - sample(x, y1); } - for (var y = y0; y <= y1; y++) { - sample(x0, y); - sample(x1, y); - } - - if (rr.length === 0) continue; + } + if (rr.length === 0) continue; - bgR = median(rr); - bgG = median(gg); + var bgR = median(rr), + bgG = median(gg), bgB = median(bb); - } - // Composite foreground alpha over measured background + // Composite foreground alpha over the measured background + var fg = c.fg; var compR = Math.round(fg[0] * fg[3] + bgR * (1 - fg[3])); var compG = Math.round(fg[1] * fg[3] + bgG * (1 - fg[3])); var compB = Math.round(fg[2] * fg[3] + bgB * (1 - fg[3])); var ratio = +wcagRatio(compR, compG, compB, bgR, bgG, bgB).toFixed(2); - var fontSize = parseFloat(cs.fontSize); - var fontWeight = Number(cs.fontWeight) || 400; - var large = fontSize >= 24 || (fontSize >= 19 && fontWeight >= 700); out.push({ time: time, - selector: selectorOf(el), - text: (el.textContent || "").trim().slice(0, 50), + selector: c.selector, + text: c.text, ratio: ratio, - wcagAA: large ? ratio >= 3 : ratio >= 4.5, - large: large, + wcagAA: c.large ? ratio >= 3 : ratio >= 4.5, + large: c.large, fg: "rgb(" + compR + "," + compG + "," + compB + ")", bg: "rgb(" + bgR + "," + bgG + "," + bgB + ")", }); diff --git a/packages/cli/src/commands/contrast-bg.ts b/packages/cli/src/commands/contrast-bg.ts index 5aef85f7f5..fa221a2d9a 100644 --- a/packages/cli/src/commands/contrast-bg.ts +++ b/packages/cli/src/commands/contrast-bg.ts @@ -1,17 +1,21 @@ // Pure background-resolution logic for the WCAG contrast audit. // -// The browser-side audit (contrast-audit.browser.js) samples a pixel ring just -// OUTSIDE an element's bounding box to estimate the background the text sits on. -// That is wrong for any element that paints its OWN opaque background (a caption -// pill, a CTA button, a solid card): the text is composited over that solid -// color, not over whatever surrounds the box. Sampling the ring there measures -// the text against the scene behind the element (often a dark photo) and reports -// a false ~1:1 ratio, flagging perfectly readable CTAs. +// HISTORICAL NOTE: contrast-audit.browser.js used to sample a 4px pixel ring +// just OUTSIDE an element's bounding box to estimate its background, with +// pickOpaqueBackground() below as a pre-check: prefer an element's own opaque +// background-color over the ring for solid CTA/pill/card cases. The audit has +// since moved to sampling the ACTUAL composited pixels directly inside each +// element's own bbox (hiding the glyphs first) — see contrast-sample.ts — which +// gets pill/card backgrounds right AND fixes the ring's blind spots (rounded +// corners, backdrop-filter blur, cross-component bleed, partially-overlapping +// translucent decoration). pickOpaqueBackground()/parseColorRGBA() are no +// longer called by the live audit but are kept here — still correct, still +// unit-tested, and contrast-fg.ts imports parseColorRGBA from this module. // -// This module hosts the pure decision so it can be unit-tested without a browser. -// The same logic is inlined into contrast-audit.browser.js (which is injected as -// a raw string and cannot import) — keep the two in sync, mirroring the existing -// "WCAG math is duplicated" note at the top of that file. +// This module hosts pure decisions so they can be unit-tested without a +// browser. Historically the same logic was inlined into +// contrast-audit.browser.js (which is injected as a raw string and cannot +// import) — see contrast-sample.ts for what's actually mirrored today. export type Rgb = [number, number, number]; export type Rgba = [number, number, number, number]; diff --git a/packages/cli/src/commands/contrast-sample.test.ts b/packages/cli/src/commands/contrast-sample.test.ts new file mode 100644 index 0000000000..131c6f7343 --- /dev/null +++ b/packages/cli/src/commands/contrast-sample.test.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from "vitest"; +import { computeSampleRect, sampleGridPoints } from "./contrast-sample.js"; + +describe("computeSampleRect", () => { + it("insets 1px on each side of a normal bbox", () => { + expect(computeSampleRect({ x: 10, y: 20, w: 30, h: 40 }, 640, 480)).toEqual({ + x0: 11, + x1: 39, + y0: 21, + y1: 59, + }); + }); + + it("clamps the left/top edge to 0 when the bbox starts off-canvas", () => { + expect(computeSampleRect({ x: -5, y: -5, w: 20, h: 20 }, 640, 480)).toEqual({ + x0: 0, + x1: 14, + y0: 0, + y1: 14, + }); + }); + + it("clamps the right/bottom edge to the canvas bounds", () => { + expect(computeSampleRect({ x: 620, y: 460, w: 40, h: 40 }, 640, 480)).toEqual({ + x0: 621, + x1: 639, + y0: 461, + y1: 479, + }); + }); + + it("returns null when the bbox is too small to survive the 1px inset", () => { + // width 2 → x0 = x+1, x1 = x+1 - 1 = x → x1 <= x0 + expect(computeSampleRect({ x: 100, y: 100, w: 2, h: 2 }, 640, 480)).toBeNull(); + }); + + it("returns null when the bbox is entirely outside the canvas", () => { + expect(computeSampleRect({ x: 1000, y: 1000, w: 20, h: 20 }, 640, 480)).toBeNull(); + }); +}); + +describe("sampleGridPoints", () => { + it("covers the full rect, starting at its top-left interior corner", () => { + const points = sampleGridPoints({ x0: 10, x1: 22, y0: 10, y1: 16 }, 12, 6); + expect(points[0]).toEqual([10, 10]); + for (const [x, y] of points) { + expect(x).toBeGreaterThanOrEqual(10); + expect(x).toBeLessThanOrEqual(22); + expect(y).toBeGreaterThanOrEqual(10); + expect(y).toBeLessThanOrEqual(16); + } + }); + + it("caps the point count for a large rect instead of scanning every pixel", () => { + const points = sampleGridPoints({ x0: 0, x1: 1199, y0: 0, y1: 59 }, 12, 6); + // (maxCols+1) * (maxRows+1) upper bound — nowhere near a full 1200x60 scan. + expect(points.length).toBeLessThan(13 * 7 + 5); + }); + + it("still returns at least one point for a rect narrower than the grid step", () => { + const points = sampleGridPoints({ x0: 5, x1: 6, y0: 5, y1: 6 }, 12, 6); + expect(points.length).toBeGreaterThan(0); + }); +}); diff --git a/packages/cli/src/commands/contrast-sample.ts b/packages/cli/src/commands/contrast-sample.ts new file mode 100644 index 0000000000..1eebe50286 --- /dev/null +++ b/packages/cli/src/commands/contrast-sample.ts @@ -0,0 +1,89 @@ +// Pure background-sampling-region logic for the WCAG contrast audit. +// +// The audit used to estimate an element's background by sampling a 4px ring +// just OUTSIDE its bounding box (with an own-opaque-background pre-check — +// see the historical note in contrast-bg.ts). That proximity-based estimate +// breaks down whenever what's immediately outside the box differs from +// what's actually behind the text inside it: +// +// - cross-component bleed: text sits near the edge of its own panel/layer, +// and a differently-colored sibling panel/layer starts just outside the +// text's bbox — the ring samples the neighbor, not the true background. +// - solid-fill pill/button with rounded corners achieved via a sibling +// shape (not a CSS background-color on an ancestor of the text) — same +// failure as above; the ownBg ancestor-walk never sees it. +// - translucent "glass" text over a backdrop-filter blur panel sized only +// a couple pixels larger than the text — the ring exits the panel and +// samples the raw, unblurred, untinted pixels behind it. +// - a translucent/decorative shape that only partially overlaps the ring, +// or sits entirely INSIDE the text's own bbox (never touching the ring +// at all) — the ring is structurally blind to it. +// +// The fix: hide the text's own paint (color/fill → transparent), take ONE +// screenshot with the glyphs invisible, then sample the REAL composited +// pixels directly INSIDE the element's own bbox — no proximity heuristic +// needed, because we're reading the exact pixels that were behind the +// glyphs. This module hosts the pure "which rect do we sample" decision +// (inset to dodge anti-aliased edge pixels, clamp to canvas bounds, and +// reject a rect that's too small to sample) so it's unit-testable without a +// browser. The same logic is inlined into contrast-audit.browser.js (which +// is injected as a raw string and cannot import) and into +// skills/hyperframes-creative/scripts/contrast-report.mjs — keep all three +// in sync. + +export interface Rect { + x: number; + y: number; + w: number; + h: number; +} + +export interface PixelRect { + x0: number; + x1: number; + y0: number; + y1: number; +} + +/** + * Compute the region to sample for an element's true background: its own + * bbox, inset by 1px on each side (anti-aliased glyph/box edges bleed into + * the adjacent pixel and shouldn't count as "background"), clamped to the + * screenshot's pixel bounds. + * + * Returns null when nothing usable survives — the bbox is entirely outside + * the canvas, or is too small once inset to contain any interior pixel. + */ +export function computeSampleRect( + bbox: Rect, + canvasWidth: number, + canvasHeight: number, +): PixelRect | null { + const x0 = Math.max(0, Math.round(bbox.x) + 1); + const x1 = Math.min(canvasWidth - 1, Math.round(bbox.x + bbox.w) - 1); + const y0 = Math.max(0, Math.round(bbox.y) + 1); + const y1 = Math.min(canvasHeight - 1, Math.round(bbox.y + bbox.h) - 1); + if (x1 <= x0 || y1 <= y0) return null; + return { x0, x1, y0, y1 }; +} + +/** + * Generate a bounded grid of sample coordinates within a pixel rect — dense + * enough to catch a partially-overlapping decoration, capped so a large + * caption bar doesn't turn into a full pixel scan. + */ +export function sampleGridPoints( + rect: PixelRect, + maxCols = 12, + maxRows = 6, +): Array<[number, number]> { + const stepX = Math.max(1, Math.floor((rect.x1 - rect.x0) / maxCols)); + const stepY = Math.max(1, Math.floor((rect.y1 - rect.y0) / maxRows)); + const points: Array<[number, number]> = []; + for (let y = rect.y0; y <= rect.y1; y += stepY) { + for (let x = rect.x0; x <= rect.x1; x += stepX) { + points.push([x, y]); + } + } + return points; +} diff --git a/packages/cli/src/commands/layout-audit.browser.test.ts b/packages/cli/src/commands/layout-audit.browser.test.ts index e084637f29..074a9a2a7f 100644 --- a/packages/cli/src/commands/layout-audit.browser.test.ts +++ b/packages/cli/src/commands/layout-audit.browser.test.ts @@ -202,7 +202,18 @@ describe("contrast-audit.browser clip-path visibility", () => { vi.unstubAllGlobals(); document.body.innerHTML = ""; delete (document as unknown as { elementFromPoint?: unknown }).elementFromPoint; - delete (window as unknown as { __contrastAudit?: unknown }).__contrastAudit; + delete ( + window as unknown as { + __contrastAuditPrepare?: unknown; + __contrastAuditFinish?: unknown; + __contrastAuditRestoreIfPending?: unknown; + __contrastAuditRestores?: unknown; + } + ).__contrastAuditPrepare; + delete (window as unknown as { __contrastAuditFinish?: unknown }).__contrastAuditFinish; + delete (window as unknown as { __contrastAuditRestoreIfPending?: unknown }) + .__contrastAuditRestoreIfPending; + delete (window as unknown as { __contrastAuditRestores?: unknown }).__contrastAuditRestores; }); it("excludes text clipped to nothing by clip-path from contrast reports", async () => { @@ -472,11 +483,16 @@ function installContrastScript(): void { } async function runContrastAudit(): Promise>> { - return ( - window as unknown as { - __contrastAudit: (imgBase64: string, time: number) => Promise>>; - } - ).__contrastAudit("stub", 0); + const w = window as unknown as { + __contrastAuditPrepare: () => Array>; + __contrastAuditFinish: ( + imgBase64: string, + time: number, + candidates: Array>, + ) => Promise>>; + }; + const candidates = w.__contrastAuditPrepare(); + return w.__contrastAuditFinish("stub", 0, candidates); } function runAudit(): Array<{ diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index 07b5334ba7..f7e762f037 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -281,6 +281,16 @@ async function auditClipDurations( return warnings; } +interface ContrastCandidate { + selector: string; + text: string; + fg: [number, number, number, number]; + fontSize: number; + fontWeight: number; + large: boolean; + bbox: { x: number; y: number; w: number; h: number }; +} + async function runContrastAudit(page: import("puppeteer-core").Page): Promise { const duration = await getCompositionDuration(page); if (duration <= 0) return []; @@ -292,16 +302,45 @@ async function runContrastAudit(page: import("puppeteer-core").Page): Promise - typeof (window as unknown as Record).__contrastAudit === "function" - ? ((window as unknown as Record).__contrastAudit as Function)(b64, time) - : [], - screenshot, - t, - ); - results.push(...(entries as ContrastEntry[])); + // __contrastAuditPrepare() hides each candidate text element's own paint + // (color/fill → transparent, layout-neutral) so this screenshot captures + // the real pixels behind the glyphs — that's what __contrastAuditFinish + // samples directly instead of a proximity-based ring outside the text's + // bbox. See contrast-audit.browser.js for why: it's robust to rounded + // pills, cross-component panel edges, backdrop-filter blur, and + // partially-overlapping translucent decoration in ways a ring isn't. + const candidates = (await page.evaluate(() => + typeof (window as unknown as Record).__contrastAuditPrepare === "function" + ? ((window as unknown as Record).__contrastAuditPrepare as () => unknown)() + : [], + )) as ContrastCandidate[]; + + try { + const screenshot = (await page.screenshot({ encoding: "base64", type: "png" })) as string; + const entries = await page.evaluate( + (b64: string, time: number, cands: ContrastCandidate[]) => + typeof (window as unknown as Record).__contrastAuditFinish === "function" + ? ((window as unknown as Record).__contrastAuditFinish as Function)( + b64, + time, + cands, + ) + : [], + screenshot, + t, + candidates, + ); + results.push(...(entries as ContrastEntry[])); + } finally { + // If the screenshot or finish() call above throws, this restores any + // still-hidden text paint so the NEXT sample in the loop doesn't audit + // a page with stale invisible elements. No-op after a normal finish(). + await page.evaluate(() => { + const restore = (window as unknown as Record) + .__contrastAuditRestoreIfPending; + if (typeof restore === "function") (restore as () => void)(); + }); + } } return results; diff --git a/skills-manifest.json b/skills-manifest.json index e213328018..533b8bae00 100644 --- a/skills-manifest.json +++ b/skills-manifest.json @@ -34,7 +34,7 @@ "files": 14 }, "hyperframes-creative": { - "hash": "da874dbcebdd84cc", + "hash": "ef0391904766a58a", "files": 69 }, "hyperframes-keyframes": { diff --git a/skills/hyperframes-creative/scripts/contrast-report.mjs b/skills/hyperframes-creative/scripts/contrast-report.mjs index 43eea07b4d..e2241fe165 100644 --- a/skills/hyperframes-creative/scripts/contrast-report.mjs +++ b/skills/hyperframes-creative/scripts/contrast-report.mjs @@ -3,7 +3,7 @@ // // Reads a composition, seeks to N sample timestamps, walks the DOM for text // elements, measures the WCAG 2.1 contrast ratio between each element's -// declared foreground color and the pixels behind it, and emits: +// declared foreground color and the ACTUAL pixels behind it, and emits: // // - contrast-report.json (machine-readable, one entry per text element × sample) // - contrast-overlay.png (sprite grid; magenta=fail AA, yellow=pass AA only, green=AAA) @@ -20,6 +20,28 @@ // The composition directory must contain an index.html. Raw authoring HTML // works — the producer's file server auto-injects the runtime at serve time. // Exits 1 if any text element fails WCAG AA. +// +// Background sampling: each sample time is captured TWICE — once via the +// producer's normal (video-accurate) captureFrameToBuffer for the overlay +// image, and once via a plain page.screenshot() taken right after hiding +// every candidate element's own text paint (color/fill → transparent, +// layout-neutral). The second capture reveals the REAL composited pixels +// that were directly behind the glyphs, which this script then samples +// straight from each element's own bbox — no proximity heuristic needed. +// This is deliberately NOT routed through captureFrameToBuffer: that +// pipeline has a static-frame dedup cache keyed by frame index/time that +// knows nothing about our DOM mutation and would happily hand back a +// cached pre-mutation buffer. A direct page.screenshot() bypasses that +// entirely and is the same technique validated in +// packages/cli/src/commands/contrast-audit.browser.js. +// +// The previous approach sampled a 4px ring just OUTSIDE the bbox, which +// breaks down whenever what's immediately outside the text differs from +// what's actually behind it: a neighboring panel/component just past the +// text's edge, a backdrop-filter-blurred glass panel sized only a couple +// pixels larger than the text, or a translucent decoration that only +// partially overlaps the ring (or sits entirely inside the bbox, never +// touching the ring at all). import { mkdir, writeFile } from "node:fs/promises"; import { resolve } from "node:path"; @@ -77,8 +99,21 @@ try { for (let i = 0; i < times.length; i++) { const t = times[i]; + // Visible frame — used only for the human-facing overlay image. const { buffer: pngBuf } = await captureFrameToBuffer(session, i, t); - const elements = await probeTextElements(session, t); + + // Hides each candidate's own text paint and returns its selector/fg/bbox. + const candidates = await prepareTextElements(session); + let elements; + try { + // Deliberately session.page.screenshot(), not captureFrameToBuffer — + // see the header comment for why. + const hiddenB64 = await session.page.screenshot({ encoding: "base64", type: "png" }); + elements = await measureAgainstHiddenTextFrame(hiddenB64, candidates); + } finally { + await restoreTextElements(session); + } + const annotated = await annotateFrame(pngBuf, elements); overlayFrames.push({ t, png: annotated }); for (const el of elements) allEntries.push({ time: t, ...el }); @@ -104,15 +139,18 @@ try { server.close(); } -// ─── DOM probe (runs in the page) ──────────────────────────────────────────── +// ─── DOM probe + text-hide (runs in the page) ──────────────────────────────── -async function probeTextElements(session, _t) { - // `session.page` is the Puppeteer Page owned by the capture session. - // We pass a pure function to `evaluate`: it walks the DOM and returns - // enough info for us to compute a ratio in Node using the frame buffer. +// Walks the DOM for text-bearing elements, computes each one's foreground +// paint, and hides that element's own text (color/fill → transparent, +// !important, layout-neutral) so the caller's next screenshot reveals the +// real pixels behind the glyphs. Returns the candidate list; call +// restoreTextElements() afterward (in a finally) to undo the hide. +async function prepareTextElements(session) { return await session.page.evaluate(() => { /** @type {Array<{selector: string, text: string, fg: [number,number,number,number], fontSize: number, fontWeight: number, bbox: {x:number,y:number,w:number,h:number}}>} */ const out = []; + const restores = []; const walker = document.createTreeWalker(document.body, NodeFilter.SHOW_ELEMENT); const parseColor = (c) => { const m = c.match(/rgba?\(([^)]+)\)/); @@ -155,9 +193,24 @@ async function probeTextElements(session, _t) { if (parseFloat(cs.opacity) <= 0.01) continue; const rect = el.getBoundingClientRect(); if (rect.width < 8 || rect.height < 8) continue; - const fg = isSvgTextElement(el) + const isSvgText = isSvgTextElement(el); + const fg = isSvgText ? tryParseSolidColor(cs.fill) || parseColor(cs.color) : parseColor(cs.color); + if (fg[3] <= 0.01) continue; + + const origColor = el.style.getPropertyValue("color"); + const origColorPriority = el.style.getPropertyPriority("color"); + el.style.setProperty("color", "transparent", "important"); + let origFill = null; + let origFillPriority = null; + if (isSvgText) { + origFill = el.style.getPropertyValue("fill"); + origFillPriority = el.style.getPropertyPriority("fill"); + el.style.setProperty("fill", "transparent", "important"); + } + restores.push({ el, origColor, origColorPriority, origFill, origFillPriority, isSvgText }); + out.push({ selector: selectorOf(el), text: el.textContent.trim().slice(0, 60), @@ -167,77 +220,105 @@ async function probeTextElements(session, _t) { bbox: { x: rect.x, y: rect.y, w: rect.width, h: rect.height }, }); } + window.__contrastReportRestores = restores; return out; }); } +async function restoreTextElements(session) { + await session.page.evaluate(() => { + const restores = window.__contrastReportRestores; + if (!restores) return; + for (const r of restores) { + if (r.origColor) r.el.style.setProperty("color", r.origColor, r.origColorPriority); + else r.el.style.removeProperty("color"); + if (r.isSvgText) { + if (r.origFill) r.el.style.setProperty("fill", r.origFill, r.origFillPriority); + else r.el.style.removeProperty("fill"); + } + } + window.__contrastReportRestores = null; + }); +} + // ─── Pixel sampling + WCAG math ────────────────────────────────────────────── -async function annotateFrame(pngBuf, elements) { - const img = sharp(pngBuf); - const meta = await img.metadata(); - const { width, height } = meta; - const raw = await img.ensureAlpha().raw().toBuffer(); +// Samples the REAL composited background directly inside each candidate's +// own bbox, from a screenshot taken with every candidate's text hidden — +// robust to panel edges, backdrop-filter blur, and translucent decoration +// in ways a proximity-based ring outside the bbox isn't. Mirrors +// packages/cli/src/commands/contrast-sample.ts's computeSampleRect / +// sampleGridPoints (kept in sync, not imported — this script bootstraps +// npm-published packages and can't reach into the cli package's sources). +async function measureAgainstHiddenTextFrame(hiddenImgBase64, candidates) { + const raw = Buffer.from(hiddenImgBase64, "base64"); + const img = sharp(raw); + const { width, height } = await img.metadata(); + const pixels = await img.ensureAlpha().raw().toBuffer(); const channels = 4; const measured = []; - for (const el of elements) { - if (isBBoxOutsideFrame(el.bbox, width, height)) continue; - const bg = sampleRingMedian(raw, width, height, channels, el.bbox); + for (const c of candidates) { + const bg = sampleBboxMedian(pixels, width, height, channels, c.bbox); if (!bg) continue; - const fg = compositeOver(el.fg, bg); // flatten any alpha against measured bg + const fg = compositeOver(c.fg, bg); // flatten any alpha against measured bg const ratio = wcagRatio(fg, bg); - const large = isLargeText(el.fontSize, el.fontWeight); - el.bg = bg; - el.ratio = +ratio.toFixed(2); - el.wcagAA = large ? ratio >= 3 : ratio >= 4.5; - el.wcagAALarge = ratio >= 3; - el.wcagAAA = large ? ratio >= 4.5 : ratio >= 7; - measured.push(el); + const large = isLargeText(c.fontSize, c.fontWeight); + measured.push({ + selector: c.selector, + text: c.text, + fg, + fontSize: c.fontSize, + fontWeight: c.fontWeight, + bbox: c.bbox, + bg, + ratio: +ratio.toFixed(2), + wcagAA: large ? ratio >= 3 : ratio >= 4.5, + wcagAALarge: ratio >= 3, + wcagAAA: large ? ratio >= 4.5 : ratio >= 7, + }); } - elements.length = 0; - elements.push(...measured); + return measured; +} +async function annotateFrame(pngBuf, elements) { + const { width, height } = await sharp(pngBuf).metadata(); // Draw boxes + ratio labels as an SVG overlay (sharp composite). - const svg = buildOverlaySVG(measured, width, height); + const svg = buildOverlaySVG(elements, width, height); return await sharp(pngBuf) .composite([{ input: Buffer.from(svg), top: 0, left: 0 }]) .png() .toBuffer(); } -function sampleRingMedian(raw, width, height, channels, bbox) { - // 4-px ring immediately outside the element bbox. Median of each channel. +function sampleBboxMedian(raw, width, height, channels, bbox) { + // Sample the element's OWN box (glyphs are hidden in this frame), inset + // 1px on each side to dodge anti-aliased edge pixels, clamped to the + // frame bounds. A bounded grid, not a full scan, so a wide caption bar + // doesn't turn into thousands of samples. + const x0 = Math.max(0, Math.round(bbox.x) + 1); + const x1 = Math.min(width - 1, Math.round(bbox.x + bbox.w) - 1); + const y0 = Math.max(0, Math.round(bbox.y) + 1); + const y1 = Math.min(height - 1, Math.round(bbox.y + bbox.h) - 1); + if (x1 <= x0 || y1 <= y0) return null; + + const stepX = Math.max(1, Math.floor((x1 - x0) / 12)); + const stepY = Math.max(1, Math.floor((y1 - y0) / 6)); const r = [], g = [], b = []; - const x0 = Math.max(0, Math.floor(bbox.x) - 4); - const x1 = Math.min(width - 1, Math.ceil(bbox.x + bbox.w) + 4); - const y0 = Math.max(0, Math.floor(bbox.y) - 4); - const y1 = Math.min(height - 1, Math.ceil(bbox.y + bbox.h) + 4); - const pushPixel = (x, y) => { - if (x < 0 || x >= width || y < 0 || y >= height) return; - const i = (y * width + x) * channels; - r.push(raw[i]); - g.push(raw[i + 1]); - b.push(raw[i + 2]); - }; - for (let x = x0; x <= x1; x++) { - pushPixel(x, y0); - pushPixel(x, y1); - } - for (let y = y0; y <= y1; y++) { - pushPixel(x0, y); - pushPixel(x1, y); + for (let y = y0; y <= y1; y += stepY) { + for (let x = x0; x <= x1; x += stepX) { + const i = (y * width + x) * channels; + r.push(raw[i]); + g.push(raw[i + 1]); + b.push(raw[i + 2]); + } } if (r.length === 0) return null; return [median(r), median(g), median(b), 1]; } -function isBBoxOutsideFrame(bbox, width, height) { - return bbox.x + bbox.w <= 0 || bbox.y + bbox.h <= 0 || bbox.x >= width || bbox.y >= height; -} - function median(arr) { const s = [...arr].sort((a, b) => a - b); return s[Math.floor(s.length / 2)]; From 89c5bf5cb7827584542b41c9f64bd2d064fe4fb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miguel=20=C3=81ngel?= Date: Thu, 9 Jul 2026 16:13:07 +0000 Subject: [PATCH 3/3] fix(cli): harden contrast-audit's hide/restore against partial failures and slow transitions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four follow-ups from review of the hide-text/sample-bbox background change: - Register the restores list (window.__contrastAuditRestores / __contrastReportRestores) BEFORE the DOM walk starts, and push into it incrementally as each element is hidden, instead of assigning it only after the walk finishes. If getComputedStyle/ getBoundingClientRect throws partway through (a detached or otherwise pathological element), everything hidden before the throw is now still reachable for restore instead of leaking hidden until the next full audit. Repro'd directly: monkey-patched the second of three text elements' getBoundingClientRect to throw mid-walk — before this fix the first element's hide was unrecoverable since the restores array was never published; after, __contrastAuditRestoreIfPending() correctly restores it. - Force `transition: none` (important) alongside color/fill when hiding text, and restore it alongside them. A `transition` on color/fill otherwise animates the hide instead of applying it instantly, so the screenshot taken right after can land mid-transition and catch a partially-visible glyph, contaminating the background sample. Repro'd with a `transition: color 4s` element: before this fix, direct pixel sampling of the "hidden" screenshot showed a fully unhidden (pure original-color) pixel at the glyph edge on every run; after, zero contamination across repeated runs. - Move the __contrastAuditPrepare() call inside the try block in validate.ts's runContrastAudit, as its first statement, so a throw from prepare() itself is covered by the existing finally-restore-safety-net. Previously prepare() ran before the try, so a mid-prepare failure skipped the restore entirely. - Added a regression test locking in the "solid-fill pill is already correct" investigation finding: constructs a dark pill/button on a bright busy page background with real (non-flat) per-pixel screenshot data, exercises the actual two-phase prepare()/finish() path, and asserts the text isn't flagged — so a future change to the bbox-sampling logic that regressed this case would fail loudly instead of silently. All four changes mirrored between contrast-audit.browser.js and skills/hyperframes-creative/scripts/contrast-report.mjs where applicable (the report script doesn't have a separate caller-side try/finally to fix — its own try/finally around the screenshot+measure call already wraps prepare()). --- .../src/commands/contrast-audit.browser.js | 24 +++- .../src/commands/layout-audit.browser.test.ts | 110 +++++++++++++++++- packages/cli/src/commands/validate.ts | 40 ++++--- skills-manifest.json | 2 +- .../scripts/contrast-report.mjs | 28 ++++- 5 files changed, 182 insertions(+), 22 deletions(-) diff --git a/packages/cli/src/commands/contrast-audit.browser.js b/packages/cli/src/commands/contrast-audit.browser.js index b542f2b135..f89478e502 100644 --- a/packages/cli/src/commands/contrast-audit.browser.js +++ b/packages/cli/src/commands/contrast-audit.browser.js @@ -133,6 +133,13 @@ window.__contrastAuditPrepare = function () { var out = []; var restores = []; + // Registered BEFORE the walk starts (not after it finishes) and pushed to + // incrementally as each element is hidden: if getComputedStyle/ + // getBoundingClientRect/etc. throws partway through the walk (e.g. on a + // detached or otherwise pathological element), everything hidden before + // the throw is still reachable for restore instead of leaking hidden + // indefinitely. + window.__contrastAuditRestores = restores; var walker = document.createTreeWalker(document.body, NodeFilter.SHOW_ELEMENT); var node; while ((node = walker.nextNode())) { @@ -197,6 +204,17 @@ window.__contrastAuditPrepare = function () { // never affect box geometry. `!important` beats any non-!important rule // that might otherwise win on specificity; we restore the exact prior // inline value (or remove the property entirely) afterward. + // + // A `transition` on color/fill would otherwise animate this hide instead + // of applying it instantly — the caller's screenshot lands in the same + // task-queue gap as the transition's own frames, so it can catch a + // partially-transparent (still partly the original color) glyph instead + // of a fully hidden one, contaminating the background sample. Force + // `transition: none` alongside color/fill so the hide is atomic, and + // restore it alongside them. + var origTransition = el.style.getPropertyValue("transition"); + var origTransitionPriority = el.style.getPropertyPriority("transition"); + el.style.setProperty("transition", "none", "important"); var origColor = el.style.getPropertyValue("color"); var origColorPriority = el.style.getPropertyPriority("color"); el.style.setProperty("color", "transparent", "important"); @@ -209,6 +227,8 @@ window.__contrastAuditPrepare = function () { } restores.push({ el: el, + origTransition: origTransition, + origTransitionPriority: origTransitionPriority, origColor: origColor, origColorPriority: origColorPriority, origFill: origFill, @@ -227,7 +247,6 @@ window.__contrastAuditPrepare = function () { }); } - window.__contrastAuditRestores = restores; return out; }; @@ -242,6 +261,9 @@ function __contrastAuditRestoreAll() { if (r.origFill) r.el.style.setProperty("fill", r.origFill, r.origFillPriority); else r.el.style.removeProperty("fill"); } + if (r.origTransition) + r.el.style.setProperty("transition", r.origTransition, r.origTransitionPriority); + else r.el.style.removeProperty("transition"); } window.__contrastAuditRestores = null; } diff --git a/packages/cli/src/commands/layout-audit.browser.test.ts b/packages/cli/src/commands/layout-audit.browser.test.ts index 074a9a2a7f..061e18da99 100644 --- a/packages/cli/src/commands/layout-audit.browser.test.ts +++ b/packages/cli/src/commands/layout-audit.browser.test.ts @@ -248,6 +248,77 @@ describe("contrast-audit.browser clip-path visibility", () => { }); }); +describe("contrast-audit.browser background sampling", () => { + afterEach(() => { + vi.restoreAllMocks(); + vi.unstubAllGlobals(); + document.body.innerHTML = ""; + delete (document as unknown as { elementFromPoint?: unknown }).elementFromPoint; + delete (window as unknown as { __contrastAuditPrepare?: unknown }).__contrastAuditPrepare; + delete (window as unknown as { __contrastAuditFinish?: unknown }).__contrastAuditFinish; + delete (window as unknown as { __contrastAuditRestoreIfPending?: unknown }) + .__contrastAuditRestoreIfPending; + delete (window as unknown as { __contrastAuditRestores?: unknown }).__contrastAuditRestores; + }); + + // Locks in the "already correct" finding from investigating the + // solid-fill-pill/button false-positive report: a rounded pill/button + // with its own solid background, sitting on a busy/bright page + // background, must NOT be flagged even though the two-phase + // prepare()/finish() path (hide text, sample the real pixels directly + // inside the element's own bbox) replaced the ring+own-background-walk + // heuristic this used to rely on. The pixel buffer here is real per-pixel + // data (not the flat-white default), with a dark region standing in for + // the pill sitting inside a bright page background, so this exercises the + // actual bbox-sampling logic in __contrastAuditFinish rather than a fixed + // stub value. + it("does not flag a solid-fill pill/button with adequate contrast", async () => { + document.body.innerHTML = ` +
+
+ Click me +
+
+ `; + + vi.spyOn(window, "getComputedStyle").mockImplementation((element) => { + const id = (element as Element).id; + return { + display: "block", + visibility: "visible", + opacity: "1", + color: id === "label" ? "rgb(255, 255, 255)" : "rgb(0, 0, 0)", + fontSize: "20px", + fontWeight: "400", + clipPath: "none", + } as unknown as CSSStyleDeclaration; + }); + + const labelRect = { left: 50, top: 50, width: 100, height: 30 }; + vi.spyOn(document.getElementById("label")!, "getBoundingClientRect").mockReturnValue( + rect(labelRect), + ); + (document as unknown as { elementFromPoint: () => Element | null }).elementFromPoint = () => + null; + + // Dark pill (rgb 10,10,10) covering the label's bbox and a small margin + // around it; everything else is a bright, busy page background + // (rgb 255,45,85) — the kind of scene that flagged false positives when + // the old algorithm sampled a ring OUTSIDE the bbox instead of the + // pixels actually inside it. + const pixels = pixelsWithRegion( + { left: 30, top: 30, width: 140, height: 70 }, + [10, 10, 10], + [255, 45, 85], + ); + installContrastScript(pixels); + + const result = await runContrastAudit(); + expect(result).toHaveLength(1); + expect(result[0]).toMatchObject({ selector: "#label", wcagAA: true, bg: "rgb(10,10,10)" }); + }); +}); + // Both blocks overlap heavily; only the exemption on block A should suppress // the finding, so a missing exemption would surface as a failure here. function expectExemptFromOverlap(aOverrides: { color?: string; attrs?: string }): void { @@ -457,7 +528,11 @@ function installAuditScript(): void { window.eval(script); } -function installContrastScript(): void { +// `pixels`, when provided, replaces the flat-white default screenshot buffer +// — used by tests that need the "hidden text" screenshot to actually vary by +// position (e.g. a solid-fill pill sitting on a busy page background) so the +// two-phase prepare/finish sampling has something real to distinguish. +function installContrastScript(pixels?: Uint8ClampedArray): void { class MockImage { onload: (() => void) | null = null; onerror: (() => void) | null = null; @@ -476,12 +551,43 @@ function installContrastScript(): void { getContextSpy.mockReturnValue({ drawImage() {}, getImageData() { - return { data: new Uint8ClampedArray(640 * 360 * 4).fill(255) }; + return { data: pixels ?? new Uint8ClampedArray(640 * 360 * 4).fill(255) }; }, } as unknown as CanvasRenderingContext2D); window.eval(contrastScript); } +// Builds a 640×360 RGBA buffer that's `fillColor` inside `insideRect` and +// `outsideColor` everywhere else — models a solid-fill pill/button (a dark +// rounded rect) sitting on a busy/bright page background, so a test can +// assert the two-phase prepare/finish path samples the pill's own pixels +// (inside the element's bbox) rather than whatever's outside it. +function pixelsWithRegion( + insideRect: RectInput, + fillColor: [number, number, number], + outsideColor: [number, number, number], +): Uint8ClampedArray { + const width = 640; + const height = 360; + const data = new Uint8ClampedArray(width * height * 4); + for (let y = 0; y < height; y++) { + for (let x = 0; x < width; x++) { + const inside = + x >= insideRect.left && + x < insideRect.left + insideRect.width && + y >= insideRect.top && + y < insideRect.top + insideRect.height; + const [r, g, b] = inside ? fillColor : outsideColor; + const idx = (y * width + x) * 4; + data[idx] = r; + data[idx + 1] = g; + data[idx + 2] = b; + data[idx + 3] = 255; + } + } + return data; +} + async function runContrastAudit(): Promise>> { const w = window as unknown as { __contrastAuditPrepare: () => Array>; diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index f7e762f037..5f10ae3a9a 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -302,20 +302,27 @@ async function runContrastAudit(page: import("puppeteer-core").Page): Promise - typeof (window as unknown as Record).__contrastAuditPrepare === "function" - ? ((window as unknown as Record).__contrastAuditPrepare as () => unknown)() - : [], - )) as ContrastCandidate[]; - try { + // __contrastAuditPrepare() hides each candidate text element's own + // paint (color/fill → transparent, layout-neutral) so this screenshot + // captures the real pixels behind the glyphs — that's what + // __contrastAuditFinish samples directly instead of a proximity-based + // ring outside the text's bbox. See contrast-audit.browser.js for why: + // it's robust to rounded pills, cross-component panel edges, + // backdrop-filter blur, and partially-overlapping translucent + // decoration in ways a ring isn't. + // + // This call is the FIRST statement inside the try — not before it — + // so if prepare() itself throws partway through hiding elements, the + // finally below still runs and restores whatever it managed to hide. + const candidates = (await page.evaluate(() => + typeof (window as unknown as Record).__contrastAuditPrepare === "function" + ? ( + (window as unknown as Record).__contrastAuditPrepare as () => unknown + )() + : [], + )) as ContrastCandidate[]; + const screenshot = (await page.screenshot({ encoding: "base64", type: "png" })) as string; const entries = await page.evaluate( (b64: string, time: number, cands: ContrastCandidate[]) => @@ -332,9 +339,10 @@ async function runContrastAudit(page: import("puppeteer-core").Page): Promise { const restore = (window as unknown as Record) .__contrastAuditRestoreIfPending; diff --git a/skills-manifest.json b/skills-manifest.json index 533b8bae00..3ea2e6946a 100644 --- a/skills-manifest.json +++ b/skills-manifest.json @@ -34,7 +34,7 @@ "files": 14 }, "hyperframes-creative": { - "hash": "ef0391904766a58a", + "hash": "d60c84ddca9ea6db", "files": 69 }, "hyperframes-keyframes": { diff --git a/skills/hyperframes-creative/scripts/contrast-report.mjs b/skills/hyperframes-creative/scripts/contrast-report.mjs index e2241fe165..1f7db29355 100644 --- a/skills/hyperframes-creative/scripts/contrast-report.mjs +++ b/skills/hyperframes-creative/scripts/contrast-report.mjs @@ -151,6 +151,11 @@ async function prepareTextElements(session) { /** @type {Array<{selector: string, text: string, fg: [number,number,number,number], fontSize: number, fontWeight: number, bbox: {x:number,y:number,w:number,h:number}}>} */ const out = []; const restores = []; + // Registered BEFORE the walk starts and pushed to incrementally as each + // element is hidden: if something in the walk throws partway through, + // everything hidden so far is still reachable for restore instead of + // leaking hidden indefinitely. + window.__contrastReportRestores = restores; const walker = document.createTreeWalker(document.body, NodeFilter.SHOW_ELEMENT); const parseColor = (c) => { const m = c.match(/rgba?\(([^)]+)\)/); @@ -199,6 +204,14 @@ async function prepareTextElements(session) { : parseColor(cs.color); if (fg[3] <= 0.01) continue; + // A `transition` on color/fill would otherwise animate this hide + // instead of applying it instantly — the screenshot taken right after + // can catch a partially-transparent glyph mid-transition instead of a + // fully hidden one, contaminating the background sample. Force + // `transition: none` alongside color/fill so the hide is atomic. + const origTransition = el.style.getPropertyValue("transition"); + const origTransitionPriority = el.style.getPropertyPriority("transition"); + el.style.setProperty("transition", "none", "important"); const origColor = el.style.getPropertyValue("color"); const origColorPriority = el.style.getPropertyPriority("color"); el.style.setProperty("color", "transparent", "important"); @@ -209,7 +222,16 @@ async function prepareTextElements(session) { origFillPriority = el.style.getPropertyPriority("fill"); el.style.setProperty("fill", "transparent", "important"); } - restores.push({ el, origColor, origColorPriority, origFill, origFillPriority, isSvgText }); + restores.push({ + el, + origTransition, + origTransitionPriority, + origColor, + origColorPriority, + origFill, + origFillPriority, + isSvgText, + }); out.push({ selector: selectorOf(el), @@ -220,7 +242,6 @@ async function prepareTextElements(session) { bbox: { x: rect.x, y: rect.y, w: rect.width, h: rect.height }, }); } - window.__contrastReportRestores = restores; return out; }); } @@ -236,6 +257,9 @@ async function restoreTextElements(session) { if (r.origFill) r.el.style.setProperty("fill", r.origFill, r.origFillPriority); else r.el.style.removeProperty("fill"); } + if (r.origTransition) { + r.el.style.setProperty("transition", r.origTransition, r.origTransitionPriority); + } else r.el.style.removeProperty("transition"); } window.__contrastReportRestores = null; });