mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-12 14:16:28 +03:00
fix: address PR review bot findings on background resolution
- Treat a url() image layer stacked above a gradient as an occluding, unreadable surface: resolveBackgroundInfo now returns unresolved so the gradient-stop fallback never measures stops the image hides (greptile-apps finding, reproduced in Chrome). - Route the glow and AI-palette DOM adapters through resolveBackgroundInfo so an unresolved surface makes them abstain instead of hunting gradient ancestors past an unreadable layer (Cursor Bugbot finding). - Resolve background-color keywords jsdom hands through verbatim: inherit now reads as no-paint (the ancestor walk IS its resolution) and currentcolor substitutes the element's own computed text color instead of forcing an abstention (Copilot finding). - Regression coverage in the dark-theme fixture for all three, asserted in both the jsdom and real-Chrome suites; browser detector regenerated. AI-assisted: prepared with Claude Code at the maintainer's direction. Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -1218,10 +1218,18 @@ function parseAnyColor(s) {
|
||||
// True when a computed background-color string names no paint at all. Used to
|
||||
// tell "this layer is see-through" (walk on to the ancestor) apart from "this
|
||||
// layer has a color we could not read" (stop and abstain).
|
||||
//
|
||||
// `inherit` belongs here even though it is not literally see-through: it means
|
||||
// "paint with the parent's background-color", and walking on to the parent IS
|
||||
// that resolution. Real browsers resolve the keyword before getComputedStyle
|
||||
// output; only jsdom's partial cascade hands it through verbatim, and treating
|
||||
// it as unreadable would make the walk abstain on a surface it can know.
|
||||
// (`currentcolor` is NOT here — it is real paint in the element's own text
|
||||
// color; resolveBackgroundInfo substitutes the computed color for it.)
|
||||
function isNoPaintColorValue(value) {
|
||||
const v = String(value || '').trim().toLowerCase();
|
||||
if (!v) return true;
|
||||
return v === 'transparent' || v === 'none' || v === 'initial' || v === 'unset' || v === 'revert' || v === 'revert-layer';
|
||||
return v === 'transparent' || v === 'none' || v === 'initial' || v === 'inherit' || v === 'unset' || v === 'revert' || v === 'revert-layer';
|
||||
}
|
||||
|
||||
// --- cli/engine/shared/fonts.mjs ---
|
||||
@@ -2962,6 +2970,15 @@ function resolveBackgroundInfo(el, win, customPropMap) {
|
||||
}
|
||||
}
|
||||
|
||||
// `background-color: currentcolor` paints with the element's own text
|
||||
// color — real paint whose value we know. Real browsers resolve the
|
||||
// keyword before getComputedStyle output; jsdom hands it through
|
||||
// verbatim, and without this substitution the layer would read as
|
||||
// unparseable and force a needless abstention.
|
||||
if ((!bg || bg.a < 0.1) && /^currentcolor$/i.test(String(style.backgroundColor || '').trim())) {
|
||||
bg = parseRgb(style.color) || parseAnyColor(style.color);
|
||||
}
|
||||
|
||||
if (bg && bg.a > 0.1) {
|
||||
if (bg.a >= 0.99) return { color: flatten(bg), unresolved: false };
|
||||
overlays.push(bg);
|
||||
@@ -2973,10 +2990,27 @@ function resolveBackgroundInfo(el, win, customPropMap) {
|
||||
// reporting an ancestor the visitor never sees.
|
||||
return { color: null, unresolved: true };
|
||||
}
|
||||
// No solid bg-color at this level. A gradient or image at THIS level is
|
||||
// the surface: hand the caller a null color so it falls back to the
|
||||
// gradient's own stops (body grounds, gradient buttons, hero sections).
|
||||
if (hasGradientOrUrl) return { color: null, unresolved: false };
|
||||
// No solid bg-color at this level, but this level paints an image. CSS
|
||||
// stacks background-image layers first-on-top, so which layer leads
|
||||
// decides what the visitor sees:
|
||||
// • gradient on top — the gradient is the surface. Hand the caller a
|
||||
// null color so it falls back to the gradient's own stops (body
|
||||
// grounds, gradient buttons, hero sections).
|
||||
// • url() on top — the surface is an image whose pixels this engine
|
||||
// cannot read, and it may fully cover every layer and ancestor
|
||||
// beneath it. Same contract as an unparseable color: abstain, so
|
||||
// the gradient-stop fallback never measures a gradient the image
|
||||
// hides (the shipped miss: `url(photo), linear-gradient(...)`
|
||||
// reported low-contrast against the invisible gradient's stops).
|
||||
if (hasGradientOrUrl) {
|
||||
const topPaintLayer = splitTopLevelCommas(bgImage).find(
|
||||
(layer) => /gradient\s*\(/i.test(layer) || /url\s*\(/i.test(layer),
|
||||
);
|
||||
const gradientOnTop = !!topPaintLayer
|
||||
&& /gradient\s*\(/i.test(topPaintLayer)
|
||||
&& !/^\s*url\s*\(/i.test(topPaintLayer);
|
||||
return { color: null, unresolved: !gradientOnTop };
|
||||
}
|
||||
current = current.parentElement;
|
||||
}
|
||||
// Every layer up to the document root was genuinely see-through, so the
|
||||
@@ -3770,7 +3804,12 @@ function checkElementGlowDOM(el) {
|
||||
if (!boxShadow && !textShadow) return [];
|
||||
// Use parent's background — glow radiates outward, so the surrounding context matters
|
||||
// If resolveBackground returns null (gradient), try to infer from the gradient colors
|
||||
let parentBg = el.parentElement ? resolveBackground(el.parentElement) : resolveBackground(el);
|
||||
const parentBgInfo = resolveBackgroundInfo(el.parentElement || el);
|
||||
// Unknown surface (an unreadable layer on the way up): abstain. The
|
||||
// gradient hunt below would walk PAST that layer and score the glow
|
||||
// against a background the visitor never sees.
|
||||
if (parentBgInfo.unresolved) return [];
|
||||
let parentBg = parentBgInfo.color;
|
||||
if (!parentBg) {
|
||||
// Gradient background — sample its colors to determine if it's dark
|
||||
let cur = el.parentElement;
|
||||
@@ -3820,10 +3859,13 @@ function checkElementAIPaletteDOM(el) {
|
||||
const hue = getHue(textColor);
|
||||
const isAIPalette = (hue >= 160 && hue <= 200) || (hue >= 260 && hue <= 310);
|
||||
if (isAIPalette) {
|
||||
const parentBg = el.parentElement ? resolveBackground(el.parentElement) : null;
|
||||
// Also check gradient parents
|
||||
let effectiveBg = parentBg;
|
||||
if (!effectiveBg) {
|
||||
const parentBgInfo = el.parentElement
|
||||
? resolveBackgroundInfo(el.parentElement)
|
||||
: { color: null, unresolved: false };
|
||||
// Unknown surface: leave effectiveBg null (no finding) rather than
|
||||
// hunting gradient ancestors past a layer we could not read.
|
||||
let effectiveBg = parentBgInfo.color;
|
||||
if (!effectiveBg && !parentBgInfo.unresolved) {
|
||||
let cur = el.parentElement;
|
||||
while (cur && cur.nodeType === 1) {
|
||||
const gi = getComputedStyle(cur).backgroundImage || '';
|
||||
|
||||
@@ -1736,6 +1736,15 @@ function resolveBackgroundInfo(el, win, customPropMap) {
|
||||
}
|
||||
}
|
||||
|
||||
// `background-color: currentcolor` paints with the element's own text
|
||||
// color — real paint whose value we know. Real browsers resolve the
|
||||
// keyword before getComputedStyle output; jsdom hands it through
|
||||
// verbatim, and without this substitution the layer would read as
|
||||
// unparseable and force a needless abstention.
|
||||
if ((!bg || bg.a < 0.1) && /^currentcolor$/i.test(String(style.backgroundColor || '').trim())) {
|
||||
bg = parseRgb(style.color) || parseAnyColor(style.color);
|
||||
}
|
||||
|
||||
if (bg && bg.a > 0.1) {
|
||||
if (bg.a >= 0.99) return { color: flatten(bg), unresolved: false };
|
||||
overlays.push(bg);
|
||||
@@ -1747,10 +1756,27 @@ function resolveBackgroundInfo(el, win, customPropMap) {
|
||||
// reporting an ancestor the visitor never sees.
|
||||
return { color: null, unresolved: true };
|
||||
}
|
||||
// No solid bg-color at this level. A gradient or image at THIS level is
|
||||
// the surface: hand the caller a null color so it falls back to the
|
||||
// gradient's own stops (body grounds, gradient buttons, hero sections).
|
||||
if (hasGradientOrUrl) return { color: null, unresolved: false };
|
||||
// No solid bg-color at this level, but this level paints an image. CSS
|
||||
// stacks background-image layers first-on-top, so which layer leads
|
||||
// decides what the visitor sees:
|
||||
// • gradient on top — the gradient is the surface. Hand the caller a
|
||||
// null color so it falls back to the gradient's own stops (body
|
||||
// grounds, gradient buttons, hero sections).
|
||||
// • url() on top — the surface is an image whose pixels this engine
|
||||
// cannot read, and it may fully cover every layer and ancestor
|
||||
// beneath it. Same contract as an unparseable color: abstain, so
|
||||
// the gradient-stop fallback never measures a gradient the image
|
||||
// hides (the shipped miss: `url(photo), linear-gradient(...)`
|
||||
// reported low-contrast against the invisible gradient's stops).
|
||||
if (hasGradientOrUrl) {
|
||||
const topPaintLayer = splitTopLevelCommas(bgImage).find(
|
||||
(layer) => /gradient\s*\(/i.test(layer) || /url\s*\(/i.test(layer),
|
||||
);
|
||||
const gradientOnTop = !!topPaintLayer
|
||||
&& /gradient\s*\(/i.test(topPaintLayer)
|
||||
&& !/^\s*url\s*\(/i.test(topPaintLayer);
|
||||
return { color: null, unresolved: !gradientOnTop };
|
||||
}
|
||||
current = current.parentElement;
|
||||
}
|
||||
// Every layer up to the document root was genuinely see-through, so the
|
||||
@@ -2544,7 +2570,12 @@ function checkElementGlowDOM(el) {
|
||||
if (!boxShadow && !textShadow) return [];
|
||||
// Use parent's background — glow radiates outward, so the surrounding context matters
|
||||
// If resolveBackground returns null (gradient), try to infer from the gradient colors
|
||||
let parentBg = el.parentElement ? resolveBackground(el.parentElement) : resolveBackground(el);
|
||||
const parentBgInfo = resolveBackgroundInfo(el.parentElement || el);
|
||||
// Unknown surface (an unreadable layer on the way up): abstain. The
|
||||
// gradient hunt below would walk PAST that layer and score the glow
|
||||
// against a background the visitor never sees.
|
||||
if (parentBgInfo.unresolved) return [];
|
||||
let parentBg = parentBgInfo.color;
|
||||
if (!parentBg) {
|
||||
// Gradient background — sample its colors to determine if it's dark
|
||||
let cur = el.parentElement;
|
||||
@@ -2594,10 +2625,13 @@ function checkElementAIPaletteDOM(el) {
|
||||
const hue = getHue(textColor);
|
||||
const isAIPalette = (hue >= 160 && hue <= 200) || (hue >= 260 && hue <= 310);
|
||||
if (isAIPalette) {
|
||||
const parentBg = el.parentElement ? resolveBackground(el.parentElement) : null;
|
||||
// Also check gradient parents
|
||||
let effectiveBg = parentBg;
|
||||
if (!effectiveBg) {
|
||||
const parentBgInfo = el.parentElement
|
||||
? resolveBackgroundInfo(el.parentElement)
|
||||
: { color: null, unresolved: false };
|
||||
// Unknown surface: leave effectiveBg null (no finding) rather than
|
||||
// hunting gradient ancestors past a layer we could not read.
|
||||
let effectiveBg = parentBgInfo.color;
|
||||
if (!effectiveBg && !parentBgInfo.unresolved) {
|
||||
let cur = el.parentElement;
|
||||
while (cur && cur.nodeType === 1) {
|
||||
const gi = getComputedStyle(cur).backgroundImage || '';
|
||||
|
||||
@@ -548,10 +548,18 @@ function parseAnyColor(s) {
|
||||
// True when a computed background-color string names no paint at all. Used to
|
||||
// tell "this layer is see-through" (walk on to the ancestor) apart from "this
|
||||
// layer has a color we could not read" (stop and abstain).
|
||||
//
|
||||
// `inherit` belongs here even though it is not literally see-through: it means
|
||||
// "paint with the parent's background-color", and walking on to the parent IS
|
||||
// that resolution. Real browsers resolve the keyword before getComputedStyle
|
||||
// output; only jsdom's partial cascade hands it through verbatim, and treating
|
||||
// it as unreadable would make the walk abstain on a surface it can know.
|
||||
// (`currentcolor` is NOT here — it is real paint in the element's own text
|
||||
// color; resolveBackgroundInfo substitutes the computed color for it.)
|
||||
function isNoPaintColorValue(value) {
|
||||
const v = String(value || '').trim().toLowerCase();
|
||||
if (!v) return true;
|
||||
return v === 'transparent' || v === 'none' || v === 'initial' || v === 'unset' || v === 'revert' || v === 'revert-layer';
|
||||
return v === 'transparent' || v === 'none' || v === 'initial' || v === 'inherit' || v === 'unset' || v === 'revert' || v === 'revert-layer';
|
||||
}
|
||||
|
||||
export {
|
||||
|
||||
@@ -963,6 +963,10 @@ describe('detectUrl — browser-only fixtures', () => {
|
||||
['#59595c', '#121215'],
|
||||
['#56514e', '#302b27'],
|
||||
['#bfbdb8', '#faf7f2'],
|
||||
// Chrome resolves inherit / currentcolor before getComputedStyle
|
||||
// output, so these two must flag natively as well.
|
||||
['#c7c4bf', '#faf7f2'],
|
||||
['#bfbdb8', '#f0ede8'],
|
||||
];
|
||||
|
||||
it('reads oklch / color() / lch grounds and never assumes white', async () => {
|
||||
@@ -990,6 +994,14 @@ describe('detectUrl — browser-only fixtures', () => {
|
||||
`expected low-contrast for text ${text} on ${bg}, got: ${snippets.join('; ')}`,
|
||||
);
|
||||
}
|
||||
|
||||
// `url(...), linear-gradient(red, blue)` paints the image on top; no
|
||||
// finding may measure against the occluded gradient's stops.
|
||||
const hidden = f.filter(r => /#ff0000|#0000ff/i.test(r.snippet || ''));
|
||||
assert.equal(
|
||||
hidden.length, 0,
|
||||
`no finding may reference the occluded gradient's stops, got: ${hidden.map(r => r.snippet).join('; ')}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -1380,6 +1380,8 @@ describe('detectHtml — dark themes written in modern color syntax', () => {
|
||||
['#59595c', '#121215'], // Flag Dim On Display P3 Panel
|
||||
['#56514e', '#302b27'], // Flag Dim On Lch Panel
|
||||
['#bfbdb8', '#faf7f2'], // Flag Pale On Light Panel
|
||||
['#c7c4bf', '#faf7f2'], // Flag Pale On Inherited Light Panel
|
||||
['#bfbdb8', '#f0ede8'], // Flag Pale On Currentcolor Panel
|
||||
];
|
||||
|
||||
it('flags text that genuinely fails against a ground the parser can read', async () => {
|
||||
@@ -1413,4 +1415,16 @@ describe('detectHtml — dark themes written in modern color syntax', () => {
|
||||
`ivory copy on dark grounds must not flag, got: ${pale.map(r => r.snippet).join('; ')}`,
|
||||
);
|
||||
});
|
||||
|
||||
it('never measures a gradient hidden beneath an image layer', async () => {
|
||||
// `url(...), linear-gradient(red, blue)` paints the image on top; the
|
||||
// gradient is invisible. Falling back to its stops manufactured
|
||||
// gray-on-color / low-contrast findings against colors nobody sees.
|
||||
const f = await detectHtml(path.join(FIXTURES, 'dark-theme-modern-color.html'));
|
||||
const hidden = f.filter(r => /#ff0000|#0000ff/i.test(r.snippet || ''));
|
||||
assert.equal(
|
||||
hidden.length, 0,
|
||||
`no finding may reference the occluded gradient's stops, got: ${hidden.map(r => r.snippet).join('; ')}`,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -37,6 +37,19 @@
|
||||
light text against the light section and invent a finding. */
|
||||
.panel-unreadable { background-color: color(rec2020 0.05 0.05 0.05); padding: 20px; }
|
||||
|
||||
/* An image layer stacked ABOVE a loud gradient. The image is the visible
|
||||
surface and its pixels are unreadable; a detector that falls back to
|
||||
gradient stops here measures a layer the visitor never sees. */
|
||||
.panel-image-over-gradient { background-image: url("opaque-panel.png"), linear-gradient(#ff0000, #0000ff); }
|
||||
|
||||
/* Keywords a real browser resolves before getComputedStyle output but a
|
||||
partial cascade hands through verbatim: inherit takes the parent's
|
||||
ground, currentcolor paints with the element's own text color. Both are
|
||||
knowable surfaces; abstaining on them would hide real findings. */
|
||||
.panel-inherit { background-color: inherit; padding: 20px; }
|
||||
.panel-current { background-color: currentcolor; color: color(srgb 0.94 0.93 0.91); }
|
||||
.text-pale-2 { color: color(srgb 0.78 0.77 0.75); }
|
||||
|
||||
.text-light { color: oklch(0.92 0.01 90); }
|
||||
.text-dark { color: color(srgb 0.1 0.11 0.12); }
|
||||
|
||||
@@ -71,6 +84,16 @@
|
||||
<p class="text-pale-light">Flag Pale On Light Panel: bone copy on a bone-white srgb surface.</p>
|
||||
</section>
|
||||
|
||||
<section class="panel-light">
|
||||
<div class="panel-inherit">
|
||||
<p class="text-pale-2">Flag Pale On Inherited Light Panel: the ground is inherited from the light section, so the walk must resolve it rather than abstain.</p>
|
||||
</div>
|
||||
</section>
|
||||
|
||||
<section class="panel-current">
|
||||
<p class="text-pale-light">Flag Pale On Currentcolor Panel: the ground is the panel's own bone text color, so the keyword must resolve rather than abstain.</p>
|
||||
</section>
|
||||
|
||||
<h1 class="text-light">Should pass</h1>
|
||||
|
||||
<section>
|
||||
@@ -100,5 +123,9 @@
|
||||
</div>
|
||||
</section>
|
||||
|
||||
<section class="panel-image-over-gradient">
|
||||
<p class="text-muted-ground">Pass Image Over Gradient: the visible ground is an image whose pixels the engine cannot read, so it must abstain instead of measuring the loud gradient hidden beneath it.</p>
|
||||
</section>
|
||||
|
||||
</body>
|
||||
</html>
|
||||
|
||||
Reference in New Issue
Block a user