From fdb9e7c6f8a401028a741acd1d937ade85e5c276 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Tue, 28 Apr 2026 15:40:24 -0700 Subject: [PATCH] fix(live): screenshot overlay no longer flashes solid black during loading MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same alpha-string trap pattern as the recent detectPageTheme fix, on a different code path. resolveCanvasBackground walks parents looking for an opaque background; on a page that doesn't set its own bg the loop runs out and fell through to: return getComputedStyle(document.body).backgroundColor || getComputedStyle(document.documentElement).backgroundColor || '#ffffff'; `getComputedStyle(body).backgroundColor` for a default-bg page returns the literal string "rgba(0, 0, 0, 0)" — non-empty, truthy — so the `||` chain short-circuits to transparent-black instead of falling through to '#ffffff'. modern-screenshot then composites the capture onto a black canvas; the WebGL shader overlay flashes solid black until the shader finishes loading. Fix: drop the buggy fallback. The while-loop already covered and ; if neither is opaque the only sensible answer is the browser's default canvas color (white). Test coverage: - New tests/live-browser-regression.test.mjs pins the anti-pattern with a static-source check (live-browser.js is an IIFE with no module exports, so this is the cheapest reliable regression guard). Also pins the equivalent guard for detectPageTheme's readOpaque helper added in the prior commit. - Wired the new test file into `bun run test`'s explicit list. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../skills/impeccable/scripts/live-browser.js | 14 +++-- .../skills/impeccable/scripts/live-browser.js | 14 +++-- .../skills/impeccable/scripts/live-browser.js | 14 +++-- .../skills/impeccable/scripts/live-browser.js | 14 +++-- .../skills/impeccable/scripts/live-browser.js | 14 +++-- .../skills/impeccable/scripts/live-browser.js | 14 +++-- .../skills/impeccable/scripts/live-browser.js | 14 +++-- .pi/skills/impeccable/scripts/live-browser.js | 14 +++-- .../skills/impeccable/scripts/live-browser.js | 14 +++-- .../skills/impeccable/scripts/live-browser.js | 14 +++-- .../skills/impeccable/scripts/live-browser.js | 14 +++-- package.json | 2 +- .../skills/impeccable/scripts/live-browser.js | 14 +++-- .../skills/impeccable/scripts/live-browser.js | 14 +++-- tests/live-browser-regression.test.mjs | 58 +++++++++++++++++++ 15 files changed, 176 insertions(+), 66 deletions(-) create mode 100644 tests/live-browser-regression.test.mjs diff --git a/.agents/skills/impeccable/scripts/live-browser.js b/.agents/skills/impeccable/scripts/live-browser.js index ac13e80fa..3da76bb96 100644 --- a/.agents/skills/impeccable/scripts/live-browser.js +++ b/.agents/skills/impeccable/scripts/live-browser.js @@ -2622,11 +2622,15 @@ if (!isTransparentColor(cs.backgroundColor)) return cs.backgroundColor; node = node.parentElement; } - return ( - getComputedStyle(document.body).backgroundColor || - getComputedStyle(document.documentElement).backgroundColor || - '#ffffff' - ); + // The walk already passed through and ; if they had been + // opaque we would have returned. Falling through with the previous + // `getComputedStyle(body).backgroundColor || …` chain is a trap: that + // call returns the literal string `"rgba(0, 0, 0, 0)"` for a page that + // never set its own bg, which is truthy and short-circuits the chain to + // transparent-black — modern-screenshot then renders the capture on a + // black canvas and the shader overlay flashes solid black during load. + // The browser canvas defaults to white, so we do too. + return '#ffffff'; } // Capture the element (with current annotations baked in) and return a PNG diff --git a/.claude/skills/impeccable/scripts/live-browser.js b/.claude/skills/impeccable/scripts/live-browser.js index ac13e80fa..3da76bb96 100644 --- a/.claude/skills/impeccable/scripts/live-browser.js +++ b/.claude/skills/impeccable/scripts/live-browser.js @@ -2622,11 +2622,15 @@ if (!isTransparentColor(cs.backgroundColor)) return cs.backgroundColor; node = node.parentElement; } - return ( - getComputedStyle(document.body).backgroundColor || - getComputedStyle(document.documentElement).backgroundColor || - '#ffffff' - ); + // The walk already passed through and ; if they had been + // opaque we would have returned. Falling through with the previous + // `getComputedStyle(body).backgroundColor || …` chain is a trap: that + // call returns the literal string `"rgba(0, 0, 0, 0)"` for a page that + // never set its own bg, which is truthy and short-circuits the chain to + // transparent-black — modern-screenshot then renders the capture on a + // black canvas and the shader overlay flashes solid black during load. + // The browser canvas defaults to white, so we do too. + return '#ffffff'; } // Capture the element (with current annotations baked in) and return a PNG diff --git a/.cursor/skills/impeccable/scripts/live-browser.js b/.cursor/skills/impeccable/scripts/live-browser.js index ac13e80fa..3da76bb96 100644 --- a/.cursor/skills/impeccable/scripts/live-browser.js +++ b/.cursor/skills/impeccable/scripts/live-browser.js @@ -2622,11 +2622,15 @@ if (!isTransparentColor(cs.backgroundColor)) return cs.backgroundColor; node = node.parentElement; } - return ( - getComputedStyle(document.body).backgroundColor || - getComputedStyle(document.documentElement).backgroundColor || - '#ffffff' - ); + // The walk already passed through and ; if they had been + // opaque we would have returned. Falling through with the previous + // `getComputedStyle(body).backgroundColor || …` chain is a trap: that + // call returns the literal string `"rgba(0, 0, 0, 0)"` for a page that + // never set its own bg, which is truthy and short-circuits the chain to + // transparent-black — modern-screenshot then renders the capture on a + // black canvas and the shader overlay flashes solid black during load. + // The browser canvas defaults to white, so we do too. + return '#ffffff'; } // Capture the element (with current annotations baked in) and return a PNG diff --git a/.gemini/skills/impeccable/scripts/live-browser.js b/.gemini/skills/impeccable/scripts/live-browser.js index ac13e80fa..3da76bb96 100644 --- a/.gemini/skills/impeccable/scripts/live-browser.js +++ b/.gemini/skills/impeccable/scripts/live-browser.js @@ -2622,11 +2622,15 @@ if (!isTransparentColor(cs.backgroundColor)) return cs.backgroundColor; node = node.parentElement; } - return ( - getComputedStyle(document.body).backgroundColor || - getComputedStyle(document.documentElement).backgroundColor || - '#ffffff' - ); + // The walk already passed through and ; if they had been + // opaque we would have returned. Falling through with the previous + // `getComputedStyle(body).backgroundColor || …` chain is a trap: that + // call returns the literal string `"rgba(0, 0, 0, 0)"` for a page that + // never set its own bg, which is truthy and short-circuits the chain to + // transparent-black — modern-screenshot then renders the capture on a + // black canvas and the shader overlay flashes solid black during load. + // The browser canvas defaults to white, so we do too. + return '#ffffff'; } // Capture the element (with current annotations baked in) and return a PNG diff --git a/.github/skills/impeccable/scripts/live-browser.js b/.github/skills/impeccable/scripts/live-browser.js index ac13e80fa..3da76bb96 100644 --- a/.github/skills/impeccable/scripts/live-browser.js +++ b/.github/skills/impeccable/scripts/live-browser.js @@ -2622,11 +2622,15 @@ if (!isTransparentColor(cs.backgroundColor)) return cs.backgroundColor; node = node.parentElement; } - return ( - getComputedStyle(document.body).backgroundColor || - getComputedStyle(document.documentElement).backgroundColor || - '#ffffff' - ); + // The walk already passed through and ; if they had been + // opaque we would have returned. Falling through with the previous + // `getComputedStyle(body).backgroundColor || …` chain is a trap: that + // call returns the literal string `"rgba(0, 0, 0, 0)"` for a page that + // never set its own bg, which is truthy and short-circuits the chain to + // transparent-black — modern-screenshot then renders the capture on a + // black canvas and the shader overlay flashes solid black during load. + // The browser canvas defaults to white, so we do too. + return '#ffffff'; } // Capture the element (with current annotations baked in) and return a PNG diff --git a/.kiro/skills/impeccable/scripts/live-browser.js b/.kiro/skills/impeccable/scripts/live-browser.js index ac13e80fa..3da76bb96 100644 --- a/.kiro/skills/impeccable/scripts/live-browser.js +++ b/.kiro/skills/impeccable/scripts/live-browser.js @@ -2622,11 +2622,15 @@ if (!isTransparentColor(cs.backgroundColor)) return cs.backgroundColor; node = node.parentElement; } - return ( - getComputedStyle(document.body).backgroundColor || - getComputedStyle(document.documentElement).backgroundColor || - '#ffffff' - ); + // The walk already passed through and ; if they had been + // opaque we would have returned. Falling through with the previous + // `getComputedStyle(body).backgroundColor || …` chain is a trap: that + // call returns the literal string `"rgba(0, 0, 0, 0)"` for a page that + // never set its own bg, which is truthy and short-circuits the chain to + // transparent-black — modern-screenshot then renders the capture on a + // black canvas and the shader overlay flashes solid black during load. + // The browser canvas defaults to white, so we do too. + return '#ffffff'; } // Capture the element (with current annotations baked in) and return a PNG diff --git a/.opencode/skills/impeccable/scripts/live-browser.js b/.opencode/skills/impeccable/scripts/live-browser.js index ac13e80fa..3da76bb96 100644 --- a/.opencode/skills/impeccable/scripts/live-browser.js +++ b/.opencode/skills/impeccable/scripts/live-browser.js @@ -2622,11 +2622,15 @@ if (!isTransparentColor(cs.backgroundColor)) return cs.backgroundColor; node = node.parentElement; } - return ( - getComputedStyle(document.body).backgroundColor || - getComputedStyle(document.documentElement).backgroundColor || - '#ffffff' - ); + // The walk already passed through and ; if they had been + // opaque we would have returned. Falling through with the previous + // `getComputedStyle(body).backgroundColor || …` chain is a trap: that + // call returns the literal string `"rgba(0, 0, 0, 0)"` for a page that + // never set its own bg, which is truthy and short-circuits the chain to + // transparent-black — modern-screenshot then renders the capture on a + // black canvas and the shader overlay flashes solid black during load. + // The browser canvas defaults to white, so we do too. + return '#ffffff'; } // Capture the element (with current annotations baked in) and return a PNG diff --git a/.pi/skills/impeccable/scripts/live-browser.js b/.pi/skills/impeccable/scripts/live-browser.js index ac13e80fa..3da76bb96 100644 --- a/.pi/skills/impeccable/scripts/live-browser.js +++ b/.pi/skills/impeccable/scripts/live-browser.js @@ -2622,11 +2622,15 @@ if (!isTransparentColor(cs.backgroundColor)) return cs.backgroundColor; node = node.parentElement; } - return ( - getComputedStyle(document.body).backgroundColor || - getComputedStyle(document.documentElement).backgroundColor || - '#ffffff' - ); + // The walk already passed through and ; if they had been + // opaque we would have returned. Falling through with the previous + // `getComputedStyle(body).backgroundColor || …` chain is a trap: that + // call returns the literal string `"rgba(0, 0, 0, 0)"` for a page that + // never set its own bg, which is truthy and short-circuits the chain to + // transparent-black — modern-screenshot then renders the capture on a + // black canvas and the shader overlay flashes solid black during load. + // The browser canvas defaults to white, so we do too. + return '#ffffff'; } // Capture the element (with current annotations baked in) and return a PNG diff --git a/.rovodev/skills/impeccable/scripts/live-browser.js b/.rovodev/skills/impeccable/scripts/live-browser.js index ac13e80fa..3da76bb96 100644 --- a/.rovodev/skills/impeccable/scripts/live-browser.js +++ b/.rovodev/skills/impeccable/scripts/live-browser.js @@ -2622,11 +2622,15 @@ if (!isTransparentColor(cs.backgroundColor)) return cs.backgroundColor; node = node.parentElement; } - return ( - getComputedStyle(document.body).backgroundColor || - getComputedStyle(document.documentElement).backgroundColor || - '#ffffff' - ); + // The walk already passed through and ; if they had been + // opaque we would have returned. Falling through with the previous + // `getComputedStyle(body).backgroundColor || …` chain is a trap: that + // call returns the literal string `"rgba(0, 0, 0, 0)"` for a page that + // never set its own bg, which is truthy and short-circuits the chain to + // transparent-black — modern-screenshot then renders the capture on a + // black canvas and the shader overlay flashes solid black during load. + // The browser canvas defaults to white, so we do too. + return '#ffffff'; } // Capture the element (with current annotations baked in) and return a PNG diff --git a/.trae-cn/skills/impeccable/scripts/live-browser.js b/.trae-cn/skills/impeccable/scripts/live-browser.js index ac13e80fa..3da76bb96 100644 --- a/.trae-cn/skills/impeccable/scripts/live-browser.js +++ b/.trae-cn/skills/impeccable/scripts/live-browser.js @@ -2622,11 +2622,15 @@ if (!isTransparentColor(cs.backgroundColor)) return cs.backgroundColor; node = node.parentElement; } - return ( - getComputedStyle(document.body).backgroundColor || - getComputedStyle(document.documentElement).backgroundColor || - '#ffffff' - ); + // The walk already passed through and ; if they had been + // opaque we would have returned. Falling through with the previous + // `getComputedStyle(body).backgroundColor || …` chain is a trap: that + // call returns the literal string `"rgba(0, 0, 0, 0)"` for a page that + // never set its own bg, which is truthy and short-circuits the chain to + // transparent-black — modern-screenshot then renders the capture on a + // black canvas and the shader overlay flashes solid black during load. + // The browser canvas defaults to white, so we do too. + return '#ffffff'; } // Capture the element (with current annotations baked in) and return a PNG diff --git a/.trae/skills/impeccable/scripts/live-browser.js b/.trae/skills/impeccable/scripts/live-browser.js index ac13e80fa..3da76bb96 100644 --- a/.trae/skills/impeccable/scripts/live-browser.js +++ b/.trae/skills/impeccable/scripts/live-browser.js @@ -2622,11 +2622,15 @@ if (!isTransparentColor(cs.backgroundColor)) return cs.backgroundColor; node = node.parentElement; } - return ( - getComputedStyle(document.body).backgroundColor || - getComputedStyle(document.documentElement).backgroundColor || - '#ffffff' - ); + // The walk already passed through and ; if they had been + // opaque we would have returned. Falling through with the previous + // `getComputedStyle(body).backgroundColor || …` chain is a trap: that + // call returns the literal string `"rgba(0, 0, 0, 0)"` for a page that + // never set its own bg, which is truthy and short-circuits the chain to + // transparent-black — modern-screenshot then renders the capture on a + // black canvas and the shader overlay flashes solid black during load. + // The browser canvas defaults to white, so we do too. + return '#ffffff'; } // Capture the element (with current annotations baked in) and return a PNG diff --git a/package.json b/package.json index b4c2e3389..69248586b 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,7 @@ "dev": "bun run server/index.js", "preview": "bun run build && wrangler pages dev", "deploy": "bun run build && wrangler pages deploy build/", - "test": "bun test tests/build.test.js tests/detect-antipatterns.test.js tests/windows-path-fix.test.js && node --test tests/detect-antipatterns-fixtures.test.mjs && node --test tests/detect-antipatterns-browser.test.mjs && node --test tests/cleanup-deprecated.test.mjs && node --test tests/live-wrap.test.mjs && node --test tests/live-accept.test.mjs && node --test tests/live-inject.test.mjs && node --test tests/live-server.test.mjs && node --test tests/framework-fixtures.test.mjs", + "test": "bun test tests/build.test.js tests/detect-antipatterns.test.js tests/windows-path-fix.test.js && node --test tests/detect-antipatterns-fixtures.test.mjs && node --test tests/detect-antipatterns-browser.test.mjs && node --test tests/cleanup-deprecated.test.mjs && node --test tests/live-wrap.test.mjs && node --test tests/live-accept.test.mjs && node --test tests/live-inject.test.mjs && node --test tests/live-server.test.mjs && node --test tests/live-browser-regression.test.mjs && node --test tests/framework-fixtures.test.mjs", "test:live-e2e": "node --test --test-timeout=600000 tests/live-e2e.test.mjs", "prepack": "cp README.md README.repo.md && cp README.npm.md README.md", "postpack": "cp README.repo.md README.md && rm README.repo.md", diff --git a/plugin/skills/impeccable/scripts/live-browser.js b/plugin/skills/impeccable/scripts/live-browser.js index ac13e80fa..3da76bb96 100644 --- a/plugin/skills/impeccable/scripts/live-browser.js +++ b/plugin/skills/impeccable/scripts/live-browser.js @@ -2622,11 +2622,15 @@ if (!isTransparentColor(cs.backgroundColor)) return cs.backgroundColor; node = node.parentElement; } - return ( - getComputedStyle(document.body).backgroundColor || - getComputedStyle(document.documentElement).backgroundColor || - '#ffffff' - ); + // The walk already passed through and ; if they had been + // opaque we would have returned. Falling through with the previous + // `getComputedStyle(body).backgroundColor || …` chain is a trap: that + // call returns the literal string `"rgba(0, 0, 0, 0)"` for a page that + // never set its own bg, which is truthy and short-circuits the chain to + // transparent-black — modern-screenshot then renders the capture on a + // black canvas and the shader overlay flashes solid black during load. + // The browser canvas defaults to white, so we do too. + return '#ffffff'; } // Capture the element (with current annotations baked in) and return a PNG diff --git a/source/skills/impeccable/scripts/live-browser.js b/source/skills/impeccable/scripts/live-browser.js index ac13e80fa..3da76bb96 100644 --- a/source/skills/impeccable/scripts/live-browser.js +++ b/source/skills/impeccable/scripts/live-browser.js @@ -2622,11 +2622,15 @@ if (!isTransparentColor(cs.backgroundColor)) return cs.backgroundColor; node = node.parentElement; } - return ( - getComputedStyle(document.body).backgroundColor || - getComputedStyle(document.documentElement).backgroundColor || - '#ffffff' - ); + // The walk already passed through and ; if they had been + // opaque we would have returned. Falling through with the previous + // `getComputedStyle(body).backgroundColor || …` chain is a trap: that + // call returns the literal string `"rgba(0, 0, 0, 0)"` for a page that + // never set its own bg, which is truthy and short-circuits the chain to + // transparent-black — modern-screenshot then renders the capture on a + // black canvas and the shader overlay flashes solid black during load. + // The browser canvas defaults to white, so we do too. + return '#ffffff'; } // Capture the element (with current annotations baked in) and return a PNG diff --git a/tests/live-browser-regression.test.mjs b/tests/live-browser-regression.test.mjs new file mode 100644 index 000000000..b0130d52d --- /dev/null +++ b/tests/live-browser-regression.test.mjs @@ -0,0 +1,58 @@ +/** + * Static-source regression guards for live-browser.js. + * + * `source/skills/impeccable/scripts/live-browser.js` is a self-contained + * IIFE served directly to user pages by live-server.mjs (no bundle step, + * no module exports). That makes its internal helpers untestable via + * normal import — but a few behaviors have failed in real-world live + * sessions in ways that are easy to express as "this exact code shape + * MUST NOT come back." This file pins those down. + * + * Add a guard whenever a bug we fix has a one-line "anti-pattern" cause + * that's easy to reintroduce on an unrelated edit. + */ + +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const LIVE_BROWSER = path.resolve( + __dirname, + '..', + 'source/skills/impeccable/scripts/live-browser.js', +); +const SOURCE = fs.readFileSync(LIVE_BROWSER, 'utf-8'); + +describe('live-browser.js regression guards', () => { + it('resolveCanvasBackground does not fall back to `getComputedStyle(...).backgroundColor || ...`', () => { + // The browser returns the literal string `"rgba(0, 0, 0, 0)"` for an + // unset body/html background. That string is non-empty and truthy, so a + // `||` chain short-circuits to transparent-black, which modern-screenshot + // hands to its WebGL shader as the canvas color and the screenshot + // overlay flashes solid black during loading on any page that doesn't + // explicitly set its own background. Forbid the pattern outright; the + // correct fallback is a literal `'#ffffff'` (the browser's default + // canvas color). + const buggy = + /getComputedStyle\(document\.(?:body|documentElement)\)\.backgroundColor\s*\|\|/; + assert.ok( + !buggy.test(SOURCE), + 'live-browser.js must not chain `getComputedStyle(...).backgroundColor || ...` — that returns transparent-black for default-bg pages and renders the screenshot overlay as solid black during loading. Use a literal fallback (`#ffffff`) instead.', + ); + }); + + it('detectPageTheme honors alpha when reading body / html backgroundColor', () => { + // Equivalent trap: `rgba(0, 0, 0, 0)` parsed naively as `(0,0,0)` makes + // a perfectly white default page register as "dark," which flips the + // chrome to the wrong palette. The fix introduced an alpha guard + // (function readOpaque) — keep that signature in source. + assert.match( + SOURCE, + /function detectPageTheme\b[\s\S]{0,1500}?function readOpaque\b/, + 'detectPageTheme must keep its readOpaque helper that filters out fully-transparent backgrounds before computing luminance', + ); + }); +});