mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-11 21:57:14 +03:00
fix(detector): contrast checks run on styled <a> and <button> (v1.0.3)
SAFE_TAGS skipped <a> and <button> categorically to avoid noise on inline links and unstyled controls. The blanket skip overshot: a pill-style anchor or styled button with its own opaque background was silently exempted from the contrast check, so a "Get started" button with charcoal text on near-black background (~2:1) read as fine to both the CLI and the browser overlay. The bail in checkColors now permits <a> and <button> when they have their own opaque background AND direct text. Inline links and bare controls keep skipping. checkElementColorsDOM no longer short-circuits before reaching checkColors so the exception fires on the browser path. Adds readOwnBackgroundColor() helper to handle jsdom's missing shorthand decomposition; falls back to parsing the inline style attr when getComputedStyle returns empty (real browsers always decompose, so the fallback is a no-op there). Fixture gains four cases: pill-style <a> low-contrast (flag), <button> low-contrast (flag), inline <a> with no own bg (pass), pill-style <a> with high contrast (pass). Three new tests assert the right flags fire and the no-regression cases stay clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
d154a5feb3
commit
64c6df216b
@@ -2,7 +2,7 @@
|
||||
"manifest_version": 3,
|
||||
"name": "Impeccable",
|
||||
"description": "Detect common UI anti-patterns in any web page",
|
||||
"version": "1.0.2",
|
||||
"version": "1.0.3",
|
||||
"permissions": ["activeTab", "scripting", "storage", "webNavigation"],
|
||||
"host_permissions": ["<all_urls>"],
|
||||
"background": {
|
||||
|
||||
@@ -933,6 +933,16 @@
|
||||
</div>
|
||||
|
||||
<div class="changelog-list" data-reveal>
|
||||
<div class="changelog-entry">
|
||||
<div class="changelog-version-header">
|
||||
<span class="changelog-version">Extension v1.0.3</span>
|
||||
<span class="changelog-date">April 29, 2026</span>
|
||||
</div>
|
||||
<ul class="changelog-items">
|
||||
<li><strong>Contrast checks now run on styled buttons.</strong> A styled <code><a></code> or <code><button></code> with its own opaque background was silently skipped by the contrast rule, so a "Get started" pill with charcoal text on a near-black background read as fine to the detector. Buttons with their own background and direct text now flow through the same contrast and palette checks as every other text element. Inline links inside paragraphs continue to skip, as before.</li>
|
||||
</ul>
|
||||
</div>
|
||||
|
||||
<div class="changelog-entry">
|
||||
<div class="changelog-version-header">
|
||||
<span class="changelog-version">Extension v1.0.2</span>
|
||||
|
||||
@@ -477,7 +477,18 @@ function isEmojiOnlyText(text) {
|
||||
|
||||
function checkColors(opts) {
|
||||
const { tag, textColor, bgColor, effectiveBg, effectiveBgStops, fontSize, fontWeight, hasDirectText, isEmojiOnly, bgClip, bgImage, classList } = opts;
|
||||
if (SAFE_TAGS.has(tag)) return [];
|
||||
if (SAFE_TAGS.has(tag)) {
|
||||
// Exception for <a> and <button> elements styled as buttons. SAFE_TAGS
|
||||
// exists to suppress contrast noise on inline links and unstyled controls,
|
||||
// where the element has no own background and the contrast against the
|
||||
// ancestor surface is already the intended visual. When the element has
|
||||
// its own opaque background and direct text, it is a styled button — and
|
||||
// contrast on its own surface is a real, frequent bug worth flagging.
|
||||
const isStyledButton = (tag === 'a' || tag === 'button')
|
||||
&& hasDirectText
|
||||
&& bgColor && bgColor.a > 0.5;
|
||||
if (!isStyledButton) return [];
|
||||
}
|
||||
const findings = [];
|
||||
|
||||
// Pure black background (only solid or near-solid, not semi-transparent overlays)
|
||||
@@ -829,6 +840,34 @@ function checkHtmlPatterns(html) {
|
||||
|
||||
// ─── Section 4: resolveBackground (unified) ─────────────────────────────────
|
||||
|
||||
// Read the element's own background color, computed-style first, with a
|
||||
// jsdom-friendly fallback that parses the inline `background:` shorthand
|
||||
// from the raw style attribute. jsdom (~v29) does not decompose the
|
||||
// shorthand into `backgroundColor`, so without this fallback the CLI silently
|
||||
// returns null for any element styled via `background: rgb(...)` or
|
||||
// `background: #abc`. Real browsers always decompose, so the fallback is
|
||||
// a no-op there.
|
||||
function readOwnBackgroundColor(el, computedStyle) {
|
||||
const bg = parseRgb(computedStyle.backgroundColor);
|
||||
if (IS_BROWSER || (bg && bg.a >= 0.1)) return bg;
|
||||
const rawStyle = el.getAttribute?.('style') || '';
|
||||
const bgMatch = rawStyle.match(/background(?:-color)?\s*:\s*([^;]+)/i);
|
||||
const inlineBg = bgMatch ? bgMatch[1].trim() : '';
|
||||
if (!inlineBg) return bg;
|
||||
if (/gradient/i.test(inlineBg) || /url\s*\(/i.test(inlineBg)) return bg;
|
||||
const fromRgb = parseRgb(inlineBg);
|
||||
if (fromRgb) return fromRgb;
|
||||
const hexMatch = inlineBg.match(/#([0-9a-f]{6}|[0-9a-f]{3})\b/i);
|
||||
if (hexMatch) {
|
||||
const h = hexMatch[1];
|
||||
if (h.length === 6) {
|
||||
return { r: parseInt(h.slice(0, 2), 16), g: parseInt(h.slice(2, 4), 16), b: parseInt(h.slice(4, 6), 16), a: 1 };
|
||||
}
|
||||
return { r: parseInt(h[0] + h[0], 16), g: parseInt(h[1] + h[1], 16), b: parseInt(h[2] + h[2], 16), a: 1 };
|
||||
}
|
||||
return bg;
|
||||
}
|
||||
|
||||
function resolveBackground(el, win) {
|
||||
let current = el;
|
||||
while (current && current.nodeType === 1) {
|
||||
@@ -994,7 +1033,9 @@ function checkElementBordersDOM(el) {
|
||||
|
||||
function checkElementColorsDOM(el) {
|
||||
const tag = el.tagName.toLowerCase();
|
||||
if (SAFE_TAGS.has(tag)) return [];
|
||||
// No early SAFE_TAGS bail here — checkColors() does its own gating that
|
||||
// includes the styled-button exception for <a> / <button> with their own
|
||||
// opaque background. Bailing here would prevent that exception from firing.
|
||||
const rect = el.getBoundingClientRect();
|
||||
if (rect.width < 10 || rect.height < 10) return [];
|
||||
const style = getComputedStyle(el);
|
||||
@@ -1004,7 +1045,7 @@ function checkElementColorsDOM(el) {
|
||||
return checkColors({
|
||||
tag,
|
||||
textColor: parseRgb(style.color),
|
||||
bgColor: parseRgb(style.backgroundColor),
|
||||
bgColor: readOwnBackgroundColor(el, style),
|
||||
effectiveBg,
|
||||
effectiveBgStops: effectiveBg ? null : resolveGradientStops(el),
|
||||
fontSize: parseFloat(style.fontSize) || 16,
|
||||
@@ -1397,7 +1438,7 @@ function checkElementColors(el, style, tag, window) {
|
||||
return checkColors({
|
||||
tag,
|
||||
textColor: parseRgb(style.color),
|
||||
bgColor: parseRgb(style.backgroundColor),
|
||||
bgColor: readOwnBackgroundColor(el, style),
|
||||
effectiveBg,
|
||||
effectiveBgStops: effectiveBg ? null : resolveGradientStops(el, window),
|
||||
fontSize: parseFloat(style.fontSize) || 16,
|
||||
|
||||
@@ -473,7 +473,18 @@ function isEmojiOnlyText(text) {
|
||||
|
||||
function checkColors(opts) {
|
||||
const { tag, textColor, bgColor, effectiveBg, effectiveBgStops, fontSize, fontWeight, hasDirectText, isEmojiOnly, bgClip, bgImage, classList } = opts;
|
||||
if (SAFE_TAGS.has(tag)) return [];
|
||||
if (SAFE_TAGS.has(tag)) {
|
||||
// Exception for <a> and <button> elements styled as buttons. SAFE_TAGS
|
||||
// exists to suppress contrast noise on inline links and unstyled controls,
|
||||
// where the element has no own background and the contrast against the
|
||||
// ancestor surface is already the intended visual. When the element has
|
||||
// its own opaque background and direct text, it is a styled button — and
|
||||
// contrast on its own surface is a real, frequent bug worth flagging.
|
||||
const isStyledButton = (tag === 'a' || tag === 'button')
|
||||
&& hasDirectText
|
||||
&& bgColor && bgColor.a > 0.5;
|
||||
if (!isStyledButton) return [];
|
||||
}
|
||||
const findings = [];
|
||||
|
||||
// Pure black background (only solid or near-solid, not semi-transparent overlays)
|
||||
@@ -825,6 +836,34 @@ function checkHtmlPatterns(html) {
|
||||
|
||||
// ─── Section 4: resolveBackground (unified) ─────────────────────────────────
|
||||
|
||||
// Read the element's own background color, computed-style first, with a
|
||||
// jsdom-friendly fallback that parses the inline `background:` shorthand
|
||||
// from the raw style attribute. jsdom (~v29) does not decompose the
|
||||
// shorthand into `backgroundColor`, so without this fallback the CLI silently
|
||||
// returns null for any element styled via `background: rgb(...)` or
|
||||
// `background: #abc`. Real browsers always decompose, so the fallback is
|
||||
// a no-op there.
|
||||
function readOwnBackgroundColor(el, computedStyle) {
|
||||
const bg = parseRgb(computedStyle.backgroundColor);
|
||||
if (IS_BROWSER || (bg && bg.a >= 0.1)) return bg;
|
||||
const rawStyle = el.getAttribute?.('style') || '';
|
||||
const bgMatch = rawStyle.match(/background(?:-color)?\s*:\s*([^;]+)/i);
|
||||
const inlineBg = bgMatch ? bgMatch[1].trim() : '';
|
||||
if (!inlineBg) return bg;
|
||||
if (/gradient/i.test(inlineBg) || /url\s*\(/i.test(inlineBg)) return bg;
|
||||
const fromRgb = parseRgb(inlineBg);
|
||||
if (fromRgb) return fromRgb;
|
||||
const hexMatch = inlineBg.match(/#([0-9a-f]{6}|[0-9a-f]{3})\b/i);
|
||||
if (hexMatch) {
|
||||
const h = hexMatch[1];
|
||||
if (h.length === 6) {
|
||||
return { r: parseInt(h.slice(0, 2), 16), g: parseInt(h.slice(2, 4), 16), b: parseInt(h.slice(4, 6), 16), a: 1 };
|
||||
}
|
||||
return { r: parseInt(h[0] + h[0], 16), g: parseInt(h[1] + h[1], 16), b: parseInt(h[2] + h[2], 16), a: 1 };
|
||||
}
|
||||
return bg;
|
||||
}
|
||||
|
||||
function resolveBackground(el, win) {
|
||||
let current = el;
|
||||
while (current && current.nodeType === 1) {
|
||||
@@ -990,7 +1029,9 @@ function checkElementBordersDOM(el) {
|
||||
|
||||
function checkElementColorsDOM(el) {
|
||||
const tag = el.tagName.toLowerCase();
|
||||
if (SAFE_TAGS.has(tag)) return [];
|
||||
// No early SAFE_TAGS bail here — checkColors() does its own gating that
|
||||
// includes the styled-button exception for <a> / <button> with their own
|
||||
// opaque background. Bailing here would prevent that exception from firing.
|
||||
const rect = el.getBoundingClientRect();
|
||||
if (rect.width < 10 || rect.height < 10) return [];
|
||||
const style = getComputedStyle(el);
|
||||
@@ -1000,7 +1041,7 @@ function checkElementColorsDOM(el) {
|
||||
return checkColors({
|
||||
tag,
|
||||
textColor: parseRgb(style.color),
|
||||
bgColor: parseRgb(style.backgroundColor),
|
||||
bgColor: readOwnBackgroundColor(el, style),
|
||||
effectiveBg,
|
||||
effectiveBgStops: effectiveBg ? null : resolveGradientStops(el),
|
||||
fontSize: parseFloat(style.fontSize) || 16,
|
||||
@@ -1393,7 +1434,7 @@ function checkElementColors(el, style, tag, window) {
|
||||
return checkColors({
|
||||
tag,
|
||||
textColor: parseRgb(style.color),
|
||||
bgColor: parseRgb(style.backgroundColor),
|
||||
bgColor: readOwnBackgroundColor(el, style),
|
||||
effectiveBg,
|
||||
effectiveBgStops: effectiveBg ? null : resolveGradientStops(el, window),
|
||||
fontSize: parseFloat(style.fontSize) || 16,
|
||||
|
||||
@@ -94,6 +94,58 @@ describe('detectHtml — jsdom fixtures', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('color: styled <a> and <button> with their own background get contrast checks', async () => {
|
||||
// SAFE_TAGS skips <a> and <button> by default to avoid noise on inline links
|
||||
// (text links inside paragraphs). When these elements are styled as buttons
|
||||
// (own opaque background, padding, direct text), the contrast check must run.
|
||||
// Mirrors a real bug from the landing-demo: a pill-style <a> with
|
||||
// warm-charcoal text on near-black bg, ~2:1 contrast, was missed by both
|
||||
// the CLI and browser overlay paths because <a> was categorically skipped.
|
||||
const f = await detectHtml(path.join(FIXTURES, 'color.html'));
|
||||
const pillBtnFlag = f.some(r =>
|
||||
r.antipattern === 'low-contrast' &&
|
||||
/#5b4f44/i.test(r.snippet || '') &&
|
||||
/#1f1a15/i.test(r.snippet || '')
|
||||
);
|
||||
assert.ok(pillBtnFlag, 'expected low-contrast finding for styled <a> pill button');
|
||||
const styledButtonFlag = f.some(r =>
|
||||
r.antipattern === 'low-contrast' &&
|
||||
/#6c7280/i.test(r.snippet || '') &&
|
||||
/#374151/i.test(r.snippet || '')
|
||||
);
|
||||
assert.ok(styledButtonFlag, 'expected low-contrast finding for styled <button>');
|
||||
});
|
||||
|
||||
it('color: inline <a> without own background remains skipped (no regression)', async () => {
|
||||
// The exception for styled buttons must not regress to flagging plain
|
||||
// inline text links — those would create noise on essentially every
|
||||
// page on the web.
|
||||
const f = await detectHtml(path.join(FIXTURES, 'color.html'));
|
||||
const inlineLinkFalsePositive = f.some(r =>
|
||||
r.antipattern === 'low-contrast' &&
|
||||
/#aaaaaa/i.test(r.snippet || '')
|
||||
);
|
||||
assert.equal(
|
||||
inlineLinkFalsePositive, false,
|
||||
'inline <a> without own background must remain skipped'
|
||||
);
|
||||
});
|
||||
|
||||
it('color: styled <a> with good contrast does not flag', async () => {
|
||||
// The detector exception must let the check run, but a properly contrasted
|
||||
// styled button must obviously pass.
|
||||
const f = await detectHtml(path.join(FIXTURES, 'color.html'));
|
||||
const goodPillFalsePositive = f.some(r =>
|
||||
r.antipattern === 'low-contrast' &&
|
||||
/#f5f0e8/i.test(r.snippet || '') &&
|
||||
/#141419/i.test(r.snippet || '')
|
||||
);
|
||||
assert.equal(
|
||||
goodPillFalsePositive, false,
|
||||
'styled <a> with high contrast must not flag'
|
||||
);
|
||||
});
|
||||
|
||||
it('color: emoji-only text is never flagged as low-contrast', async () => {
|
||||
// Emojis render as multicolor glyphs regardless of CSS `color`, so the
|
||||
// CSS text color is irrelevant for contrast. The fixture's emoji cards
|
||||
|
||||
+31
@@ -10,6 +10,13 @@
|
||||
.col h2 { font-size: 14px; text-transform: uppercase; letter-spacing: 0.05em; margin: 0 0 16px; color: #475569; }
|
||||
.col h3 { font-size: 11px; text-transform: uppercase; letter-spacing: 0.05em; margin: 24px 0 8px; color: #64748b; }
|
||||
.card { padding: 14px 16px; border-radius: 10px; margin-bottom: 10px; }
|
||||
/* Styled button color cases (declared via stylesheet so jsdom resolves them
|
||||
reliably — inline color doesn't always win over user-agent rules in jsdom
|
||||
for <a> and <button>). */
|
||||
.styled-pill-low { background-color: rgb(31, 26, 21); color: rgb(91, 79, 68); display: inline-block; padding: 9px 18px; border-radius: 999px; font-weight: 500; font-size: 14px; text-decoration: none; }
|
||||
.low-contrast-button { background-color: rgb(55, 65, 81); color: rgb(108, 114, 128); display: inline-block; padding: 10px 20px; border-radius: 8px; border: 0; font-size: 14px; }
|
||||
.good-pill-high { background-color: rgb(20, 20, 25); color: rgb(245, 240, 232); display: inline-block; padding: 9px 18px; border-radius: 999px; font-weight: 500; font-size: 14px; text-decoration: none; }
|
||||
.inline-link-low { color: rgb(170, 170, 170); }
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
@@ -58,6 +65,17 @@
|
||||
<h3>AI color palette</h3>
|
||||
<h1 style="color: rgb(139, 92, 246); font-size: 1.5rem; margin: 0;">Purple heading text</h1>
|
||||
|
||||
<h3>Styled <a> and <button> with low contrast</h3>
|
||||
<!-- A pill-style <a> with its own opaque background and low-contrast text.
|
||||
Currently SAFE_TAGS skips <a> entirely; this case asserts the contrast
|
||||
check still runs when the element is styled as a button. Mirrors a real
|
||||
bug from the landing-demo: warm-charcoal text on near-black bg. -->
|
||||
<a href="#" class="styled-pill-low" data-test="styled-pill">Get started</a>
|
||||
<!-- A <button> element with its own opaque background and low-contrast text.
|
||||
Same SAFE_TAGS exception — buttons styled with their own background should
|
||||
still pass through the contrast check. -->
|
||||
<button class="low-contrast-button" data-test="low-contrast-button">Submit</button>
|
||||
|
||||
<h3>Tailwind color anti-patterns</h3>
|
||||
<div class="bg-black text-white p-4 rounded card" style="background: black; color: white;">
|
||||
<p>bg-black — pure black bg</p>
|
||||
@@ -118,6 +136,19 @@
|
||||
<p style="color: white;">bg-black/50 — 50% opacity, not pure black</p>
|
||||
</div>
|
||||
|
||||
<h3>Inline <a> without own background (must remain skipped)</h3>
|
||||
<!-- A regular text link inside a paragraph. No own background. SAFE_TAGS
|
||||
must continue to skip these — flagging them would create noise on
|
||||
every text link on the web. The detector exception for styled
|
||||
buttons must not regress to flagging plain inline links. -->
|
||||
<p>Read more about this <a href="#" class="inline-link-low" data-test="inline-link">here</a> if you want to learn more about the topic.</p>
|
||||
|
||||
<h3>Styled <a> with good contrast (must not flag)</h3>
|
||||
<!-- A pill-style <a> styled correctly: cream text on near-black bg,
|
||||
high contrast. The detector exception for styled buttons must
|
||||
let the contrast check run, but this case must clearly pass. -->
|
||||
<a href="#" class="good-pill-high" data-test="good-pill">High contrast pill</a>
|
||||
|
||||
<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. -->
|
||||
|
||||
Reference in New Issue
Block a user