mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-11 21:57:14 +03:00
fix(live): screenshot overlay no longer flashes solid black during loading
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 <body> and
<html>; 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) <noreply@anthropic.com>
This commit is contained in:
Paul Bakaus
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
9ec904302b
commit
fdb9e7c6f8
@@ -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 <body> and <html>; 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
|
||||
|
||||
@@ -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 <body> and <html>; 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
|
||||
|
||||
@@ -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 <body> and <html>; 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
|
||||
|
||||
@@ -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 <body> and <html>; 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
|
||||
|
||||
@@ -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 <body> and <html>; 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
|
||||
|
||||
@@ -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 <body> and <html>; 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
|
||||
|
||||
@@ -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 <body> and <html>; 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
|
||||
|
||||
@@ -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 <body> and <html>; 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
|
||||
|
||||
@@ -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 <body> and <html>; 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
|
||||
|
||||
@@ -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 <body> and <html>; 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
|
||||
|
||||
@@ -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 <body> and <html>; 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
|
||||
|
||||
+1
-1
@@ -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",
|
||||
|
||||
@@ -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 <body> and <html>; 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
|
||||
|
||||
@@ -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 <body> and <html>; 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
|
||||
|
||||
@@ -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',
|
||||
);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user