mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-15 07:36:50 +03:00
fix: keep zero-offset glow findings when the surface is unreadable
The browser glow adapter abstained from the whole element when resolveBackgroundInfo reported an unreadable surface, which also dropped zero-offset chromatic halo findings that do not depend on the background at all. It now skips only the gradient hunt past the unreadable layer and scores the halo tell against a null surface, matching what the static loop already did. Fixture cases pin both sides: the halo over a url() image ancestor flags in both engines, and an offset chromatic shadow on the same unknown surface stays abstained. Also hardens the currentcolor background substitution with the parseColorResolved fallback used by the text-color path, and adds fixture coverage proving tokenized currentcolor surfaces already resolve through the static cascade (flag when knowable, abstain when the token is undefined). Addresses Cursor Bugbot review findings on PR #541. AI-assisted-by: Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -2976,7 +2976,12 @@ function resolveBackgroundInfo(el, win, customPropMap) {
|
||||
// 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);
|
||||
// The static cascade resolves var() text tokens before checks run, so
|
||||
// style.color is normally already an rgb string here; parseColorResolved
|
||||
// is defense in depth for any future caller that passes a live
|
||||
// customPropMap (it matches the text-color path in checkElementColors
|
||||
// and reduces to parseAnyColor when the map is null or absent).
|
||||
bg = parseRgb(style.color) || parseColorResolved(style.color, customPropMap);
|
||||
}
|
||||
|
||||
if (bg && bg.a > 0.1) {
|
||||
@@ -3805,12 +3810,14 @@ function checkElementGlowDOM(el) {
|
||||
// Use parent's background — glow radiates outward, so the surrounding context matters
|
||||
// If resolveBackground returns null (gradient), try to infer from the gradient colors
|
||||
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 [];
|
||||
// Unknown surface (an unreadable layer on the way up): skip only the
|
||||
// gradient hunt below, which would walk PAST that layer and score the
|
||||
// glow against a background the visitor never sees. checkGlow still runs
|
||||
// with a null surface: the zero-offset chromatic halo tell holds on ANY
|
||||
// background, and the static loop already passes the unresolved walk's
|
||||
// null color straight through (detect-html.mjs uses resolveBackground).
|
||||
let parentBg = parentBgInfo.color;
|
||||
if (!parentBg) {
|
||||
if (!parentBg && !parentBgInfo.unresolved) {
|
||||
// Gradient background — sample its colors to determine if it's dark
|
||||
let cur = el.parentElement;
|
||||
while (cur && cur.nodeType === 1) {
|
||||
|
||||
@@ -1742,7 +1742,12 @@ function resolveBackgroundInfo(el, win, customPropMap) {
|
||||
// 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);
|
||||
// The static cascade resolves var() text tokens before checks run, so
|
||||
// style.color is normally already an rgb string here; parseColorResolved
|
||||
// is defense in depth for any future caller that passes a live
|
||||
// customPropMap (it matches the text-color path in checkElementColors
|
||||
// and reduces to parseAnyColor when the map is null or absent).
|
||||
bg = parseRgb(style.color) || parseColorResolved(style.color, customPropMap);
|
||||
}
|
||||
|
||||
if (bg && bg.a > 0.1) {
|
||||
@@ -2571,12 +2576,14 @@ function checkElementGlowDOM(el) {
|
||||
// Use parent's background — glow radiates outward, so the surrounding context matters
|
||||
// If resolveBackground returns null (gradient), try to infer from the gradient colors
|
||||
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 [];
|
||||
// Unknown surface (an unreadable layer on the way up): skip only the
|
||||
// gradient hunt below, which would walk PAST that layer and score the
|
||||
// glow against a background the visitor never sees. checkGlow still runs
|
||||
// with a null surface: the zero-offset chromatic halo tell holds on ANY
|
||||
// background, and the static loop already passes the unresolved walk's
|
||||
// null color straight through (detect-html.mjs uses resolveBackground).
|
||||
let parentBg = parentBgInfo.color;
|
||||
if (!parentBg) {
|
||||
if (!parentBg && !parentBgInfo.unresolved) {
|
||||
// Gradient background — sample its colors to determine if it's dark
|
||||
let cur = el.parentElement;
|
||||
while (cur && cur.nodeType === 1) {
|
||||
|
||||
@@ -112,6 +112,23 @@ describe('detectUrl — browser-only fixtures', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('dark-glow: unreadable url() surface keeps zero-offset halos, abstains on offset chromatic shadows', async () => {
|
||||
// Mirrors the static assertions in detect-antipatterns-fixtures.test.mjs.
|
||||
// The browser adapter (checkElementGlowDOM) once returned [] for the
|
||||
// whole element when the parent surface was unresolved, which also
|
||||
// dropped zero-offset chromatic halos that need no background at all.
|
||||
const f = await detectUrl(`${baseUrl}/fixtures/antipatterns/glow.html`, { visualContrast: false });
|
||||
const glow = f.filter(r => r.antipattern === 'dark-glow');
|
||||
assert.ok(
|
||||
glow.some(g => /Zero-offset box-shadow glow \(#d946ef\)/i.test(g.snippet || '')),
|
||||
'expected zero-offset halo finding under unreadable image surface',
|
||||
);
|
||||
assert.equal(
|
||||
glow.filter(g => /#10b981/i.test(g.snippet || '')).length, 0,
|
||||
'offset chromatic shadow on unknown surface must not be scored',
|
||||
);
|
||||
});
|
||||
|
||||
it('shadowed form.id: a <form> with <input name="id"> does not crash the scan (issue #407)', async () => {
|
||||
// HTMLFormElement named-property shadowing makes form.id / form.className
|
||||
// return the child input element, whose .startsWith throws. Every Shopify
|
||||
|
||||
@@ -224,6 +224,37 @@ describe('detectHtml — static HTML/CSS fixtures', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('color: currentcolor surface resolves var() text color instead of abstaining', async () => {
|
||||
// background-color: currentcolor paints with the element's own text
|
||||
// color, which in jsdom can itself be a var() token. The surface is
|
||||
// knowable through the custom-prop map, so the faint text on it is a
|
||||
// real low-contrast finding — abstention here would hide it.
|
||||
const f = await detectHtml(path.join(FIXTURES, 'color.html'));
|
||||
assert.ok(
|
||||
f.some(r =>
|
||||
r.antipattern === 'low-contrast' &&
|
||||
/#cfc9bd/i.test(r.snippet || '') &&
|
||||
/#e8e2d6/i.test(r.snippet || '')
|
||||
),
|
||||
'expected low-contrast finding on the currentcolor var() surface',
|
||||
);
|
||||
// Good contrast on the same surface must not flag.
|
||||
const goodFP = f.filter(r =>
|
||||
(r.antipattern === 'low-contrast' || r.antipattern === 'gray-on-color') &&
|
||||
/#3a352c/i.test(r.snippet || '')
|
||||
);
|
||||
assert.equal(goodFP.length, 0, `dark ink on bone must pass, got: ${goodFP.map(r => r.snippet).join('; ')}`);
|
||||
// An undefined token keeps the surface unknowable: abstain, don't guess.
|
||||
const unknownFP = f.filter(r =>
|
||||
(r.antipattern === 'low-contrast' || r.antipattern === 'gray-on-color') &&
|
||||
/#efe9dd/i.test(r.snippet || '')
|
||||
);
|
||||
assert.equal(
|
||||
unknownFP.length, 0,
|
||||
`unresolvable currentcolor surface must abstain, got: ${unknownFP.map(r => r.snippet).join('; ')}`,
|
||||
);
|
||||
});
|
||||
|
||||
it('color: white text on background-image url() ancestor is not flagged as low-contrast', async () => {
|
||||
const f = await detectHtml(path.join(FIXTURES, 'color.html'));
|
||||
// The pass column has white text on a div with background-image: url().
|
||||
@@ -987,17 +1018,30 @@ describe('detectHtml — motion', () => {
|
||||
|
||||
describe('detectHtml — dark glow', () => {
|
||||
// Calibrated static baseline — see motion test note above.
|
||||
// 11 element-level findings (glow-blue, glow-purple, glow-cyan, glow-multi,
|
||||
// 12 element-level findings (glow-blue, glow-purple, glow-cyan, glow-multi,
|
||||
// inline pink, glow-oklch, glow-hex, glow-hsl, glow-var, glow-text,
|
||||
// glow-light-oklch) + 1 page-level text-scan finding. Pass column adds none.
|
||||
// glow-light-oklch, glow-photo-halo) + 1 page-level text-scan finding.
|
||||
// Pass column adds none.
|
||||
it('glow: flag column triggers dark-glow, pass column adds none', async () => {
|
||||
const f = await detectHtml(path.join(FIXTURES, 'glow.html'));
|
||||
const glow = f.filter(r => r.antipattern === 'dark-glow');
|
||||
assert.equal(glow.length, 12);
|
||||
assert.equal(glow.length, 13);
|
||||
// Every finding is a glow tell, none reference the pass-column shadows
|
||||
for (const g of glow) {
|
||||
assert.match(g.snippet, /Zero-offset (box|text)-shadow glow|Colored (box|text)-shadow glow/);
|
||||
}
|
||||
// Zero-offset halo under an unreadable url() surface still fires: the
|
||||
// halo tell does not depend on the background at all.
|
||||
assert.ok(
|
||||
glow.some(g => /Zero-offset box-shadow glow \(#d946ef\)/i.test(g.snippet)),
|
||||
'expected zero-offset halo finding under unreadable image surface',
|
||||
);
|
||||
// Offset chromatic shadow under the same unreadable surface abstains:
|
||||
// the dark-background tell needs a surface we can actually read.
|
||||
assert.equal(
|
||||
glow.filter(g => /#10b981/i.test(g.snippet)).length, 0,
|
||||
'offset chromatic shadow on unknown surface must not be scored',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
+33
@@ -36,6 +36,18 @@
|
||||
.ox-glow { background: linear-gradient(160deg, rgba(52,192,168,0.09) 0%, #141419 65%); padding: 20px; }
|
||||
.ox-glow p { color: #e8e6e3; font-size: 18px; }
|
||||
.ox-glow .muted { color: #8e8c89; font-size: 16px; }
|
||||
/* currentcolor surface: background-color paints with the element's own
|
||||
text color, which is itself a var() token here. jsdom hands both
|
||||
through verbatim, so the walk must resolve the token via the
|
||||
custom-prop map instead of abstaining on a knowable surface. */
|
||||
:root { --fixture-bone: #e8e2d6; }
|
||||
.currentcolor-surface { background-color: currentcolor; color: var(--fixture-bone); padding: 14px 16px; border-radius: 10px; margin-bottom: 10px; }
|
||||
.currentcolor-low-text { color: #cfc9bd; font-size: 14px; }
|
||||
.currentcolor-good-text { color: #3a352c; font-size: 14px; }
|
||||
/* Same shape but the token does not exist: the surface truly cannot be
|
||||
read, so the walk must abstain rather than guess. */
|
||||
.currentcolor-unknown { background-color: currentcolor; color: var(--fixture-undefined-token); padding: 14px 16px; border-radius: 10px; }
|
||||
.currentcolor-unknown p { color: #efe9dd; font-size: 14px; }
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
@@ -111,6 +123,14 @@
|
||||
<div class="bg-gradient-to-r from-purple-500 to-indigo-500 text-white p-4 rounded card" style="background: linear-gradient(to right, rgb(168, 85, 247), rgb(99, 102, 241)); color: white;">
|
||||
<p>Purple-to-indigo gradient</p>
|
||||
</div>
|
||||
|
||||
<h3>currentcolor surface via var() token</h3>
|
||||
<!-- background-color: currentcolor with color: var(--fixture-bone).
|
||||
The surface is knowable (bone #e8e2d6), so the faint text on it is
|
||||
a real low-contrast finding, not an abstention. -->
|
||||
<div class="currentcolor-surface" data-test="currentcolor-low">
|
||||
<p class="currentcolor-low-text">Faint warm gray on a bone currentcolor surface</p>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<!-- ════════════════════════════════════════════════════════════
|
||||
@@ -212,6 +232,19 @@
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<h3>currentcolor surface with good contrast</h3>
|
||||
<div class="currentcolor-surface" data-test="currentcolor-good">
|
||||
<p class="currentcolor-good-text">Dark ink text on a bone currentcolor surface</p>
|
||||
</div>
|
||||
|
||||
<h3>currentcolor surface with unresolvable token (must abstain)</h3>
|
||||
<!-- The var() token is undefined, so the surface genuinely cannot be
|
||||
read. The walk must abstain instead of guessing a background for
|
||||
the light text inside. -->
|
||||
<div class="currentcolor-unknown" data-test="currentcolor-unknown">
|
||||
<p>Light text on an unknowable currentcolor surface</p>
|
||||
</div>
|
||||
|
||||
<h3>Emoji on light backgrounds</h3>
|
||||
<!-- Emojis render as multicolor glyphs regardless of CSS color, so the
|
||||
CSS color is irrelevant for contrast. These should NOT be flagged. -->
|
||||
|
||||
+25
@@ -42,6 +42,12 @@
|
||||
.glow-text h4 { text-shadow: 0 0 12px #22d3ee; }
|
||||
/* Zero-offset chromatic halo is slop on light backgrounds too */
|
||||
.glow-light-oklch { box-shadow: 0 0 20px oklch(0.65 0.2 300 / 0.45); }
|
||||
/* Unreadable surface: url() image ancestor. The zero-offset halo tell
|
||||
holds on ANY background, so an unknown surface must not suppress it.
|
||||
Both element loops must agree here (the browser adapter once returned
|
||||
[] for the whole element on an unresolved surface). */
|
||||
.photo-context { background-image: url('/fixtures/antipatterns/missing-photo.png'); padding: 16px; border-radius: 12px; }
|
||||
.glow-photo-halo { box-shadow: 0 0 26px rgba(217, 70, 239, 0.5); }
|
||||
|
||||
/* ── PASS: same dark/light backgrounds, but neutral or no glow ── */
|
||||
.light-colored-shadow { box-shadow: 0 2px 4px rgba(59, 130, 246, 0.15); }
|
||||
@@ -67,6 +73,9 @@
|
||||
.light-offset-colored { box-shadow: 0 8px 20px rgba(59, 130, 246, 0.25); }
|
||||
/* Achromatic zero-offset shadow: soft ambient elevation, stays legal */
|
||||
.light-soft-neutral { box-shadow: 0 0 24px rgba(0, 0, 0, 0.15); }
|
||||
/* Offset chromatic shadow under the same unreadable url() surface:
|
||||
the dark-background glow tell needs a surface we can read — abstain. */
|
||||
.photo-offset-colored { box-shadow: 0 6px 20px rgba(16, 185, 129, 0.35); }
|
||||
/* Offset neutral text-shadow on dark card: legibility aid, not a glow */
|
||||
.dark-text-offset h4 { text-shadow: 0 1px 2px rgba(0, 0, 0, 0.6); }
|
||||
</style>
|
||||
@@ -136,6 +145,14 @@
|
||||
<h4>Chromatic halo on light page</h4>
|
||||
<p>box-shadow: 0 0 20px oklch(0.65 0.2 300 / 0.45)</p>
|
||||
</div>
|
||||
|
||||
<h3>Zero-offset halo on unreadable (image) surface</h3>
|
||||
<div class="photo-context">
|
||||
<div class="card card-light glow-photo-halo">
|
||||
<h4>Halo over photo background</h4>
|
||||
<p>box-shadow: 0 0 26px rgba(217, 70, 239, 0.5)</p>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<!-- ════════════════════════════════════════════════════════════
|
||||
@@ -202,6 +219,14 @@
|
||||
<p>Achromatic zero-offset shadow is ambient elevation.</p>
|
||||
</div>
|
||||
|
||||
<h3>Offset chromatic shadow on unreadable (image) surface</h3>
|
||||
<div class="photo-context">
|
||||
<div class="card card-light photo-offset-colored">
|
||||
<h4>Offset colored shadow over photo background</h4>
|
||||
<p>Surface unknown, offset shadow — abstain, not flag.</p>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<h3>Neutral text-shadow on dark card</h3>
|
||||
<div class="dark-context">
|
||||
<div class="card card-dark dark-text-offset">
|
||||
|
||||
Reference in New Issue
Block a user