mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-12 06:06:37 +03:00
Harden detector against form.id shadowing and gradient/non-rendered false positives
Fixes three detector bugs that surfaced on real-world (Shopify) URL scans: #407 — DOM named-property shadowing crash. On a <form> with a named control like <input name="id"> (every Shopify product form), HTMLFormElement's [LegacyOverrideBuiltIns] behavior makes `form.id` return the input element, not the id string, so `elId.startsWith(...)` throws and aborts the whole scan. Read the id via getAttribute whenever `el.id` is not a string, at all three sites: checkQuality (checks.mjs) and collectBrowserFindings + generateSelector (browser/injected/index.mjs). Regenerated the browser bundle. #408 — tiny-text / undersized-ui-text flagged non-rendered elements. On sites that set html{font-size:62.5%} the root computes to 10px, so <script>/<style>/ <title>/<noscript> and display:none / visibility:hidden blocks — whose JS/CSS/ JSON-LD text clears the hasDirectText gate — produced dozens of phantom "10px body text" findings. Added isNonRenderedText() (tag list + head descendants + display/visibility) and gated both text-size floors on it. #409 — contrast rules misjudged gradients. Case A: background-clip:text paints its glyphs with the element's own gradient, not a backdrop, so measuring the never-painted `color` against those stops is a guaranteed false positive; skip the backdrop-contrast checks when bgClip is 'text' (the gradient-text pattern flag still fires). Case B: a translucent gradient stop (e.g. a 9%-alpha accent glow) was treated as an opaque accent; composite alpha stops over the resolved surface beneath the gradient in resolveGradientStops(), dropping the stop rather than guessing when that surface is unresolvable. Fixtures + tests: shadowed-form-id.html (browser, #407), nonrendered-text.html (#408), and gradient-clipped + alpha-glow cases added to color.html (#409). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
450d5659c7
commit
507725c935
@@ -530,7 +530,11 @@ if (IS_BROWSER) {
|
||||
function generateSelector(el) {
|
||||
if (el === document.body) return 'body';
|
||||
if (el === document.documentElement) return 'html';
|
||||
if (el.id) return '#' + CSS.escape(el.id);
|
||||
// Read via getAttribute when `el.id` is not a string — a <form> with a
|
||||
// named control (e.g. <input name="id">) shadows the builtin getter and
|
||||
// returns the element, producing a garbage `#[object …]` selector (#407).
|
||||
const elId = typeof el.id === 'string' ? el.id : (el.getAttribute('id') || '');
|
||||
if (elId) return '#' + CSS.escape(elId);
|
||||
|
||||
const parts = [];
|
||||
let current = el;
|
||||
@@ -1467,8 +1471,11 @@ if (IS_BROWSER) {
|
||||
for (const el of document.querySelectorAll('*')) {
|
||||
// Skip impeccable's own elements and any descendants (overlays, labels, banner, nav buttons)
|
||||
if (el.closest('.impeccable-overlay, .impeccable-label, .impeccable-banner, .impeccable-tooltip')) continue;
|
||||
// Skip browser extension elements (Claude, etc.)
|
||||
const elId = el.id || '';
|
||||
// Skip browser extension elements (Claude, etc.). Use getAttribute when
|
||||
// `el.id` is not a string: a <form> with a named control like
|
||||
// <input name="id"> shadows the builtin `id` getter and returns the
|
||||
// element, whose `.startsWith` throws (issue #407).
|
||||
const elId = typeof el.id === 'string' ? el.id : (el.getAttribute('id') || '');
|
||||
if (elId.startsWith('claude-') || elId.startsWith('cic-')) continue;
|
||||
// Skip the impeccable live-mode overlay (highlight, tooltip, bar, picker, toast).
|
||||
// These are inspector chrome, not part of the user's design.
|
||||
|
||||
@@ -902,9 +902,21 @@ function checkColors(opts) {
|
||||
const findings = [];
|
||||
|
||||
if (hasDirectText && textColor && !isEmojiOnly) {
|
||||
// Gradient-clipped text (`background-clip: text`, typically with a
|
||||
// transparent text-fill) paints its glyphs *with* the element's own
|
||||
// gradient. The `color` value the cascade still reports is never painted,
|
||||
// and the gradient is the fill, not a backdrop — so measuring `color`
|
||||
// against that gradient (which resolveGradientStops picks up as the
|
||||
// element's own background-image) is a guaranteed false positive
|
||||
// (issue #409 Case A). Skip the backdrop-contrast checks; the gradient-text
|
||||
// rule below still flags the pattern itself. Skipping a rule beats a false
|
||||
// positive here — the true painted contrast can't be measured from `color`.
|
||||
const isGradientClippedText = bgClip === 'text';
|
||||
// Run background-dependent checks against either a solid bg or, if the
|
||||
// ancestor is a gradient, against every gradient stop (use the worst case).
|
||||
const bgs = effectiveBg ? [effectiveBg] : (effectiveBgStops && effectiveBgStops.length ? effectiveBgStops : null);
|
||||
const bgs = isGradientClippedText
|
||||
? null
|
||||
: (effectiveBg ? [effectiveBg] : (effectiveBgStops && effectiveBgStops.length ? effectiveBgStops : null));
|
||||
if (bgs) {
|
||||
// Gray on colored background — flag if every stop is chromatic
|
||||
const textLum = relativeLuminance(textColor);
|
||||
@@ -2462,29 +2474,54 @@ function resolveBackground(el, win, customPropMap) {
|
||||
// Walk parents looking for a gradient background and return its color stops.
|
||||
// Used as a fallback when resolveBackground() returns null because the
|
||||
// effective background is a gradient (no single solid color to compare against).
|
||||
function resolveGradientStops(el, win) {
|
||||
function resolveGradientStops(el, win, customPropMap) {
|
||||
let current = el;
|
||||
while (current && current.nodeType === 1) {
|
||||
const style = DETECTOR_IS_BROWSER ? getComputedStyle(current) : win.getComputedStyle(current);
|
||||
const bgImage = style.backgroundImage || '';
|
||||
let stops = null;
|
||||
if (bgImage && bgImage !== 'none' && /gradient/i.test(bgImage)) {
|
||||
const stops = parseGradientColors(bgImage);
|
||||
if (stops.length > 0) return stops;
|
||||
const parsed = parseGradientColors(bgImage);
|
||||
if (parsed.length > 0) stops = parsed;
|
||||
}
|
||||
if (!DETECTOR_IS_BROWSER) {
|
||||
if (!stops && !DETECTOR_IS_BROWSER) {
|
||||
// jsdom doesn't decompose `background:` shorthand — peek at the raw inline style
|
||||
const rawStyle = current.getAttribute?.('style') || '';
|
||||
const bgMatch = rawStyle.match(/background(?:-image)?\s*:\s*([^;]+)/i);
|
||||
if (bgMatch && /gradient/i.test(bgMatch[1])) {
|
||||
const stops = parseGradientColors(bgMatch[1]);
|
||||
if (stops.length > 0) return stops;
|
||||
const parsed = parseGradientColors(bgMatch[1]);
|
||||
if (parsed.length > 0) stops = parsed;
|
||||
}
|
||||
}
|
||||
if (stops) return compositeGradientStops(stops, current, win, customPropMap);
|
||||
current = current.parentElement;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
// A translucent gradient stop (e.g. a faint `rgba(52,192,168,0.09)` accent
|
||||
// glow) paints over whatever surface sits beneath the gradient — the browser
|
||||
// composites it, so its effective color is far closer to the base than to the
|
||||
// full-opacity accent. Treating the stop as opaque flags every text child of a
|
||||
// softly-glowing section as low-contrast (issue #409 Case B). Composite each
|
||||
// alpha stop over the resolved surface beneath the gradient element. When that
|
||||
// surface isn't resolvable (another gradient above, no opaque ancestor), drop
|
||||
// the translucent stop rather than guess: a dropped stop can't manufacture a
|
||||
// false finding, and skipping beats a wrong ratio.
|
||||
function compositeGradientStops(stops, gradientEl, win, customPropMap) {
|
||||
const hasAlpha = stops.some(s => (s.a ?? 1) < 0.99);
|
||||
if (!hasAlpha) return stops;
|
||||
const base = resolveBackground(gradientEl.parentElement || gradientEl, win, customPropMap);
|
||||
const out = [];
|
||||
for (const s of stops) {
|
||||
const a = s.a ?? 1;
|
||||
if (a >= 0.99) { out.push(s); continue; }
|
||||
if (base) out.push(compositeColorOver(s, base));
|
||||
// else: unresolvable base — drop the translucent stop (skip, don't guess).
|
||||
}
|
||||
return out.length ? out : null;
|
||||
}
|
||||
|
||||
// Parse a single CSS length token to pixels. Accepts "12px", "50%", a
|
||||
// shorthand like "12px 4px" (uses the first value), or empty / null.
|
||||
// Returns the pixel value, or null when the input is unparseable.
|
||||
@@ -3664,6 +3701,34 @@ function isVisuallyHidden(el, style) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Elements whose text is never painted: document metadata and script/style
|
||||
// payloads. Their JS / CSS / JSON-LD text satisfies `hasDirectText`, and on
|
||||
// sites that set `html { font-size: 62.5% }` their inherited computed size is
|
||||
// 10px — so the text-size floors flag them as tiny body copy even though
|
||||
// nothing renders (issue #408: dozens of phantom "10px body text" findings on
|
||||
// every Shopify page). Exclude them, plus anything the cascade resolves to
|
||||
// display:none / visibility:hidden. The jsdom path can't lay out, so the
|
||||
// tag/attribute-based exclusions carry the weight there; the display checks are
|
||||
// computed-style reads that resolve without layout in both adapters.
|
||||
const NON_RENDERED_TAGS = new Set([
|
||||
'script', 'style', 'title', 'noscript', 'template', 'head',
|
||||
'meta', 'link', 'base', 'param', 'source', 'track', 'datalist',
|
||||
'col', 'colgroup', 'map', 'area',
|
||||
]);
|
||||
function isNonRenderedText(el, tag, style) {
|
||||
const t = (tag || '').toLowerCase();
|
||||
if (NON_RENDERED_TAGS.has(t)) return true;
|
||||
// Descendants of <head> never render even when the tag itself would
|
||||
// (some sites nest <noscript>/<template> content there).
|
||||
if (el && el.closest && el.closest('head')) return true;
|
||||
if (style) {
|
||||
if (style.display === 'none') return true;
|
||||
const vis = style.visibility;
|
||||
if (vis === 'hidden' || vis === 'collapse') return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
// Pure quality checks. Most run on computed CSS and DOM-only inputs (work in
|
||||
// jsdom and the browser). Two checks (line-length, cramped-padding) gate on
|
||||
// element rect dimensions, which jsdom can't compute — pass `rect: null` from
|
||||
@@ -3674,8 +3739,13 @@ function isVisuallyHidden(el, style) {
|
||||
function checkQuality(opts) {
|
||||
const { el, tag, style, hasDirectText, textLen, fontSize, lineHeightPx, letterSpacingPx, rect, lineMax = 80, viewportWidth = 0, win = null } = opts;
|
||||
const findings = [];
|
||||
// Skip browser extension injected elements
|
||||
const elId = el.id || '';
|
||||
// Skip browser extension injected elements. Read the id via getAttribute
|
||||
// whenever `el.id` is not a string: on a <form> (and other
|
||||
// [LegacyOverrideBuiltIns] hosts) a named control like <input name="id">
|
||||
// shadows the builtin `id` getter and returns the control element, whose
|
||||
// `.startsWith` is undefined and throws (issue #407 — every Shopify product
|
||||
// form ships an <input name="id">).
|
||||
const elId = typeof el.id === 'string' ? el.id : (el.getAttribute?.('id') || '');
|
||||
if (elId.startsWith('claude-') || elId.startsWith('cic-')) return findings;
|
||||
|
||||
// --- Line length too long --- (browser-only: needs rect.width)
|
||||
@@ -3943,7 +4013,7 @@ function checkQuality(opts) {
|
||||
const skipTags = ['sub', 'sup', 'code', 'kbd', 'samp', 'var', 'caption', 'figcaption'];
|
||||
const inUIContext = el.closest && el.closest('button, a, label, summary, pre, [role="button"], [role="link"], [role="tab"], [role="menuitem"], [role="option"], nav, footer, [aria-hidden="true"], [class*="badge" i], [class*="caption" i], [class*="chip" i], [class*="code" i], [class*="console" i], [class*="diff" i], [class*="label" i], [class*="meta" i], [class*="mock" i], [class*="pill" i], [class*="preview" i], [class*="tag" i], [class*="terminal" i], [class*="writes" i]');
|
||||
const isUppercase = style.textTransform === 'uppercase';
|
||||
if (!skipTags.includes(tag) && !inUIContext && !isUppercase) {
|
||||
if (!skipTags.includes(tag) && !inUIContext && !isUppercase && !isNonRenderedText(el, tag, style)) {
|
||||
findings.push({ id: 'tiny-text', snippet: `${fontSize}px body text` });
|
||||
}
|
||||
}
|
||||
@@ -3973,13 +4043,15 @@ function checkQuality(opts) {
|
||||
.replace(/\s+/g, ' ')
|
||||
.trim();
|
||||
const dtLen = directText.length;
|
||||
const UI_SKIP_TAGS = new Set(['sub', 'sup', 'script', 'style', 'title', 'option']);
|
||||
const notRendered = style.display === 'none' || style.visibility === 'hidden' || style.visibility === 'collapse';
|
||||
// `option` renders (in native select popups) so it stays a local skip;
|
||||
// script/style/title/noscript/head-descendants and display:none /
|
||||
// visibility:hidden are handled by isNonRenderedText (shared with tiny-text).
|
||||
const UI_SKIP_TAGS = new Set(['sub', 'sup', 'option']);
|
||||
// jsdom resolves the parent chain in resolveFontSizePx, so em/rem/%-sized
|
||||
// text that computes at or above the floor never reaches here. The browser
|
||||
// adapter additionally catches values only resolvable with real layout
|
||||
// (e.g. viewport-relative units, cascade winners set in linked sheets).
|
||||
if (fontSize > 0 && fontSize < 11 && dtLen >= 2 && !UI_SKIP_TAGS.has(tag) && !notRendered) {
|
||||
if (fontSize > 0 && fontSize < 11 && dtLen >= 2 && !UI_SKIP_TAGS.has(tag) && !isNonRenderedText(el, tag, style)) {
|
||||
const EXEMPT_CONTEXT = 'pre, code, kbd, samp, var, svg, [aria-hidden="true"], [class*="terminal" i], [class*="console" i], [class*="code" i], [class*="mock" i], [class*="editor" i], [class*="syntax" i], [class*="diff" i]';
|
||||
const isExemptContext = (el.matches && el.matches(EXEMPT_CONTEXT)) || (el.closest && el.closest(EXEMPT_CONTEXT));
|
||||
if (!isExemptContext && !isVisuallyHidden(el, style)) {
|
||||
@@ -4192,7 +4264,7 @@ function checkElementColors(el, style, tag, window, customPropMap, hasAnchorInhe
|
||||
textColor,
|
||||
bgColor: ownBg,
|
||||
effectiveBg: finalEffectiveBg,
|
||||
effectiveBgStops: finalEffectiveBg ? null : resolveGradientStops(el, window),
|
||||
effectiveBgStops: finalEffectiveBg ? null : resolveGradientStops(el, window, customPropMap),
|
||||
fontSize: parseFloat(style.fontSize) || 16,
|
||||
fontWeight: parseInt(style.fontWeight) || 400,
|
||||
hasDirectText,
|
||||
@@ -6497,7 +6569,11 @@ if (IS_BROWSER) {
|
||||
function generateSelector(el) {
|
||||
if (el === document.body) return 'body';
|
||||
if (el === document.documentElement) return 'html';
|
||||
if (el.id) return '#' + CSS.escape(el.id);
|
||||
// Read via getAttribute when `el.id` is not a string — a <form> with a
|
||||
// named control (e.g. <input name="id">) shadows the builtin getter and
|
||||
// returns the element, producing a garbage `#[object …]` selector (#407).
|
||||
const elId = typeof el.id === 'string' ? el.id : (el.getAttribute('id') || '');
|
||||
if (elId) return '#' + CSS.escape(elId);
|
||||
|
||||
const parts = [];
|
||||
let current = el;
|
||||
@@ -7434,8 +7510,11 @@ if (IS_BROWSER) {
|
||||
for (const el of document.querySelectorAll('*')) {
|
||||
// Skip impeccable's own elements and any descendants (overlays, labels, banner, nav buttons)
|
||||
if (el.closest('.impeccable-overlay, .impeccable-label, .impeccable-banner, .impeccable-tooltip')) continue;
|
||||
// Skip browser extension elements (Claude, etc.)
|
||||
const elId = el.id || '';
|
||||
// Skip browser extension elements (Claude, etc.). Use getAttribute when
|
||||
// `el.id` is not a string: a <form> with a named control like
|
||||
// <input name="id"> shadows the builtin `id` getter and returns the
|
||||
// element, whose `.startsWith` throws (issue #407).
|
||||
const elId = typeof el.id === 'string' ? el.id : (el.getAttribute('id') || '');
|
||||
if (elId.startsWith('claude-') || elId.startsWith('cic-')) continue;
|
||||
// Skip the impeccable live-mode overlay (highlight, tooltip, bar, picker, toast).
|
||||
// These are inspector chrome, not part of the user's design.
|
||||
|
||||
+86
-14
@@ -109,9 +109,21 @@ function checkColors(opts) {
|
||||
const findings = [];
|
||||
|
||||
if (hasDirectText && textColor && !isEmojiOnly) {
|
||||
// Gradient-clipped text (`background-clip: text`, typically with a
|
||||
// transparent text-fill) paints its glyphs *with* the element's own
|
||||
// gradient. The `color` value the cascade still reports is never painted,
|
||||
// and the gradient is the fill, not a backdrop — so measuring `color`
|
||||
// against that gradient (which resolveGradientStops picks up as the
|
||||
// element's own background-image) is a guaranteed false positive
|
||||
// (issue #409 Case A). Skip the backdrop-contrast checks; the gradient-text
|
||||
// rule below still flags the pattern itself. Skipping a rule beats a false
|
||||
// positive here — the true painted contrast can't be measured from `color`.
|
||||
const isGradientClippedText = bgClip === 'text';
|
||||
// Run background-dependent checks against either a solid bg or, if the
|
||||
// ancestor is a gradient, against every gradient stop (use the worst case).
|
||||
const bgs = effectiveBg ? [effectiveBg] : (effectiveBgStops && effectiveBgStops.length ? effectiveBgStops : null);
|
||||
const bgs = isGradientClippedText
|
||||
? null
|
||||
: (effectiveBg ? [effectiveBg] : (effectiveBgStops && effectiveBgStops.length ? effectiveBgStops : null));
|
||||
if (bgs) {
|
||||
// Gray on colored background — flag if every stop is chromatic
|
||||
const textLum = relativeLuminance(textColor);
|
||||
@@ -1669,29 +1681,54 @@ function resolveBackground(el, win, customPropMap) {
|
||||
// Walk parents looking for a gradient background and return its color stops.
|
||||
// Used as a fallback when resolveBackground() returns null because the
|
||||
// effective background is a gradient (no single solid color to compare against).
|
||||
function resolveGradientStops(el, win) {
|
||||
function resolveGradientStops(el, win, customPropMap) {
|
||||
let current = el;
|
||||
while (current && current.nodeType === 1) {
|
||||
const style = DETECTOR_IS_BROWSER ? getComputedStyle(current) : win.getComputedStyle(current);
|
||||
const bgImage = style.backgroundImage || '';
|
||||
let stops = null;
|
||||
if (bgImage && bgImage !== 'none' && /gradient/i.test(bgImage)) {
|
||||
const stops = parseGradientColors(bgImage);
|
||||
if (stops.length > 0) return stops;
|
||||
const parsed = parseGradientColors(bgImage);
|
||||
if (parsed.length > 0) stops = parsed;
|
||||
}
|
||||
if (!DETECTOR_IS_BROWSER) {
|
||||
if (!stops && !DETECTOR_IS_BROWSER) {
|
||||
// jsdom doesn't decompose `background:` shorthand — peek at the raw inline style
|
||||
const rawStyle = current.getAttribute?.('style') || '';
|
||||
const bgMatch = rawStyle.match(/background(?:-image)?\s*:\s*([^;]+)/i);
|
||||
if (bgMatch && /gradient/i.test(bgMatch[1])) {
|
||||
const stops = parseGradientColors(bgMatch[1]);
|
||||
if (stops.length > 0) return stops;
|
||||
const parsed = parseGradientColors(bgMatch[1]);
|
||||
if (parsed.length > 0) stops = parsed;
|
||||
}
|
||||
}
|
||||
if (stops) return compositeGradientStops(stops, current, win, customPropMap);
|
||||
current = current.parentElement;
|
||||
}
|
||||
return null;
|
||||
}
|
||||
|
||||
// A translucent gradient stop (e.g. a faint `rgba(52,192,168,0.09)` accent
|
||||
// glow) paints over whatever surface sits beneath the gradient — the browser
|
||||
// composites it, so its effective color is far closer to the base than to the
|
||||
// full-opacity accent. Treating the stop as opaque flags every text child of a
|
||||
// softly-glowing section as low-contrast (issue #409 Case B). Composite each
|
||||
// alpha stop over the resolved surface beneath the gradient element. When that
|
||||
// surface isn't resolvable (another gradient above, no opaque ancestor), drop
|
||||
// the translucent stop rather than guess: a dropped stop can't manufacture a
|
||||
// false finding, and skipping beats a wrong ratio.
|
||||
function compositeGradientStops(stops, gradientEl, win, customPropMap) {
|
||||
const hasAlpha = stops.some(s => (s.a ?? 1) < 0.99);
|
||||
if (!hasAlpha) return stops;
|
||||
const base = resolveBackground(gradientEl.parentElement || gradientEl, win, customPropMap);
|
||||
const out = [];
|
||||
for (const s of stops) {
|
||||
const a = s.a ?? 1;
|
||||
if (a >= 0.99) { out.push(s); continue; }
|
||||
if (base) out.push(compositeColorOver(s, base));
|
||||
// else: unresolvable base — drop the translucent stop (skip, don't guess).
|
||||
}
|
||||
return out.length ? out : null;
|
||||
}
|
||||
|
||||
// Parse a single CSS length token to pixels. Accepts "12px", "50%", a
|
||||
// shorthand like "12px 4px" (uses the first value), or empty / null.
|
||||
// Returns the pixel value, or null when the input is unparseable.
|
||||
@@ -2871,6 +2908,34 @@ function isVisuallyHidden(el, style) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Elements whose text is never painted: document metadata and script/style
|
||||
// payloads. Their JS / CSS / JSON-LD text satisfies `hasDirectText`, and on
|
||||
// sites that set `html { font-size: 62.5% }` their inherited computed size is
|
||||
// 10px — so the text-size floors flag them as tiny body copy even though
|
||||
// nothing renders (issue #408: dozens of phantom "10px body text" findings on
|
||||
// every Shopify page). Exclude them, plus anything the cascade resolves to
|
||||
// display:none / visibility:hidden. The jsdom path can't lay out, so the
|
||||
// tag/attribute-based exclusions carry the weight there; the display checks are
|
||||
// computed-style reads that resolve without layout in both adapters.
|
||||
const NON_RENDERED_TAGS = new Set([
|
||||
'script', 'style', 'title', 'noscript', 'template', 'head',
|
||||
'meta', 'link', 'base', 'param', 'source', 'track', 'datalist',
|
||||
'col', 'colgroup', 'map', 'area',
|
||||
]);
|
||||
function isNonRenderedText(el, tag, style) {
|
||||
const t = (tag || '').toLowerCase();
|
||||
if (NON_RENDERED_TAGS.has(t)) return true;
|
||||
// Descendants of <head> never render even when the tag itself would
|
||||
// (some sites nest <noscript>/<template> content there).
|
||||
if (el && el.closest && el.closest('head')) return true;
|
||||
if (style) {
|
||||
if (style.display === 'none') return true;
|
||||
const vis = style.visibility;
|
||||
if (vis === 'hidden' || vis === 'collapse') return true;
|
||||
}
|
||||
return false;
|
||||
}
|
||||
|
||||
// Pure quality checks. Most run on computed CSS and DOM-only inputs (work in
|
||||
// jsdom and the browser). Two checks (line-length, cramped-padding) gate on
|
||||
// element rect dimensions, which jsdom can't compute — pass `rect: null` from
|
||||
@@ -2881,8 +2946,13 @@ function isVisuallyHidden(el, style) {
|
||||
function checkQuality(opts) {
|
||||
const { el, tag, style, hasDirectText, textLen, fontSize, lineHeightPx, letterSpacingPx, rect, lineMax = 80, viewportWidth = 0, win = null } = opts;
|
||||
const findings = [];
|
||||
// Skip browser extension injected elements
|
||||
const elId = el.id || '';
|
||||
// Skip browser extension injected elements. Read the id via getAttribute
|
||||
// whenever `el.id` is not a string: on a <form> (and other
|
||||
// [LegacyOverrideBuiltIns] hosts) a named control like <input name="id">
|
||||
// shadows the builtin `id` getter and returns the control element, whose
|
||||
// `.startsWith` is undefined and throws (issue #407 — every Shopify product
|
||||
// form ships an <input name="id">).
|
||||
const elId = typeof el.id === 'string' ? el.id : (el.getAttribute?.('id') || '');
|
||||
if (elId.startsWith('claude-') || elId.startsWith('cic-')) return findings;
|
||||
|
||||
// --- Line length too long --- (browser-only: needs rect.width)
|
||||
@@ -3150,7 +3220,7 @@ function checkQuality(opts) {
|
||||
const skipTags = ['sub', 'sup', 'code', 'kbd', 'samp', 'var', 'caption', 'figcaption'];
|
||||
const inUIContext = el.closest && el.closest('button, a, label, summary, pre, [role="button"], [role="link"], [role="tab"], [role="menuitem"], [role="option"], nav, footer, [aria-hidden="true"], [class*="badge" i], [class*="caption" i], [class*="chip" i], [class*="code" i], [class*="console" i], [class*="diff" i], [class*="label" i], [class*="meta" i], [class*="mock" i], [class*="pill" i], [class*="preview" i], [class*="tag" i], [class*="terminal" i], [class*="writes" i]');
|
||||
const isUppercase = style.textTransform === 'uppercase';
|
||||
if (!skipTags.includes(tag) && !inUIContext && !isUppercase) {
|
||||
if (!skipTags.includes(tag) && !inUIContext && !isUppercase && !isNonRenderedText(el, tag, style)) {
|
||||
findings.push({ id: 'tiny-text', snippet: `${fontSize}px body text` });
|
||||
}
|
||||
}
|
||||
@@ -3180,13 +3250,15 @@ function checkQuality(opts) {
|
||||
.replace(/\s+/g, ' ')
|
||||
.trim();
|
||||
const dtLen = directText.length;
|
||||
const UI_SKIP_TAGS = new Set(['sub', 'sup', 'script', 'style', 'title', 'option']);
|
||||
const notRendered = style.display === 'none' || style.visibility === 'hidden' || style.visibility === 'collapse';
|
||||
// `option` renders (in native select popups) so it stays a local skip;
|
||||
// script/style/title/noscript/head-descendants and display:none /
|
||||
// visibility:hidden are handled by isNonRenderedText (shared with tiny-text).
|
||||
const UI_SKIP_TAGS = new Set(['sub', 'sup', 'option']);
|
||||
// jsdom resolves the parent chain in resolveFontSizePx, so em/rem/%-sized
|
||||
// text that computes at or above the floor never reaches here. The browser
|
||||
// adapter additionally catches values only resolvable with real layout
|
||||
// (e.g. viewport-relative units, cascade winners set in linked sheets).
|
||||
if (fontSize > 0 && fontSize < 11 && dtLen >= 2 && !UI_SKIP_TAGS.has(tag) && !notRendered) {
|
||||
if (fontSize > 0 && fontSize < 11 && dtLen >= 2 && !UI_SKIP_TAGS.has(tag) && !isNonRenderedText(el, tag, style)) {
|
||||
const EXEMPT_CONTEXT = 'pre, code, kbd, samp, var, svg, [aria-hidden="true"], [class*="terminal" i], [class*="console" i], [class*="code" i], [class*="mock" i], [class*="editor" i], [class*="syntax" i], [class*="diff" i]';
|
||||
const isExemptContext = (el.matches && el.matches(EXEMPT_CONTEXT)) || (el.closest && el.closest(EXEMPT_CONTEXT));
|
||||
if (!isExemptContext && !isVisuallyHidden(el, style)) {
|
||||
@@ -3399,7 +3471,7 @@ function checkElementColors(el, style, tag, window, customPropMap, hasAnchorInhe
|
||||
textColor,
|
||||
bgColor: ownBg,
|
||||
effectiveBg: finalEffectiveBg,
|
||||
effectiveBgStops: finalEffectiveBg ? null : resolveGradientStops(el, window),
|
||||
effectiveBgStops: finalEffectiveBg ? null : resolveGradientStops(el, window, customPropMap),
|
||||
fontSize: parseFloat(style.fontSize) || 16,
|
||||
fontWeight: parseInt(style.fontWeight) || 400,
|
||||
hasDirectText,
|
||||
|
||||
@@ -112,6 +112,15 @@ describe('detectUrl — browser-only fixtures', () => {
|
||||
}
|
||||
});
|
||||
|
||||
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
|
||||
// product form ships <input name="id">, so this crashed the URL scan. The
|
||||
// scan must complete and return an array of findings instead of throwing.
|
||||
const f = await detectUrl(`${baseUrl}/fixtures/antipatterns/shadowed-form-id.html`);
|
||||
assert.ok(Array.isArray(f), 'detectUrl must return findings without throwing on a shadowed form.id');
|
||||
});
|
||||
|
||||
it('line-length: flag column triggers, pass column adds none', async () => {
|
||||
const f = await detectUrl(`${baseUrl}/fixtures/antipatterns/quality.html`);
|
||||
assert.equal(f.filter(r => r.antipattern === 'line-length').length, 1);
|
||||
|
||||
@@ -281,6 +281,43 @@ describe('detectHtml — static HTML/CSS fixtures', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('color: gradient-clipped text is not contrast-checked against its own fill (issue #409 Case A)', async () => {
|
||||
// background-clip: text with a transparent fill paints the glyphs with the
|
||||
// gradient; the inherited `color` is never painted, so measuring it against
|
||||
// the element's own gradient stops (#6d8cff / #a78bfa) is a false positive.
|
||||
// The gradient-text pattern flag still fires; the backdrop-contrast rules
|
||||
// (low-contrast / gray-on-color) must stay silent for the clipped element.
|
||||
const f = await detectHtml(path.join(FIXTURES, 'color.html'));
|
||||
const clippedContrastFP = f.filter(r =>
|
||||
(r.antipattern === 'low-contrast' || r.antipattern === 'gray-on-color') &&
|
||||
/#6d8cff|#a78bfa/i.test(r.snippet || '')
|
||||
);
|
||||
assert.equal(
|
||||
clippedContrastFP.length, 0,
|
||||
`gradient-clipped text must not be contrast-checked against its own fill, got: ${clippedContrastFP.map(r => `${r.antipattern}:${r.snippet}`).join('; ')}`
|
||||
);
|
||||
// The pattern itself must still be surfaced.
|
||||
assert.ok(
|
||||
f.some(r => r.antipattern === 'gradient-text'),
|
||||
'gradient-text pattern flag must still fire'
|
||||
);
|
||||
});
|
||||
|
||||
it('color: alpha gradient-glow stops composite against the surface beneath (issue #409 Case B)', async () => {
|
||||
// A 9%-alpha teal glow stop (rgba(52,192,168,0.09)) over a dark section
|
||||
// composites to ~near-black, not the full-opacity #34c0a8. Text on it is
|
||||
// high-contrast; treating the stop as opaque flagged every text child.
|
||||
const f = await detectHtml(path.join(FIXTURES, 'color.html'));
|
||||
const glowFP = f.filter(r =>
|
||||
(r.antipattern === 'low-contrast' || r.antipattern === 'gray-on-color') &&
|
||||
/#34c0a8/i.test(r.snippet || '')
|
||||
);
|
||||
assert.equal(
|
||||
glowFP.length, 0,
|
||||
`alpha glow stops must composite against the underlying surface, got: ${glowFP.map(r => `${r.antipattern}:${r.snippet}`).join('; ')}`
|
||||
);
|
||||
});
|
||||
|
||||
it('legitimate-borders: zero findings', async () => {
|
||||
const f = await detectHtml(path.join(FIXTURES, 'legitimate-borders.html'));
|
||||
assert.equal(f.length, 0, `expected no findings, got: ${f.map(r => `${r.antipattern}:${r.snippet}`).join('; ')}`);
|
||||
@@ -597,6 +634,32 @@ describe('detectHtml — undersized-ui-text', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('detectHtml — non-rendered text (issue #408)', () => {
|
||||
// On sites that set `html { font-size: 62.5% }` the root computes to 10px, so
|
||||
// <script>/<style>/<title>/<noscript> and display:none / visibility:hidden
|
||||
// blocks — whose JS/CSS/JSON-LD text clears the hasDirectText gate — report a
|
||||
// 10px size and used to produce dozens of phantom "10px body text" findings.
|
||||
// Both text-size floors (tiny-text and undersized-ui-text) must skip them and
|
||||
// measure only genuinely rendered text.
|
||||
it('tiny-text / undersized-ui-text: non-rendered elements produce no findings, rendered text still flags', async () => {
|
||||
const f = await detectHtml(path.join(FIXTURES, 'nonrendered-text.html'));
|
||||
const tiny = f.filter(r => r.antipattern === 'tiny-text');
|
||||
const undersized = f.filter(r => r.antipattern === 'undersized-ui-text');
|
||||
|
||||
// Exactly the two genuinely rendered elements flag: the 10px body <p>
|
||||
// (tiny-text) and the 9px interactive nav link (undersized-ui-text).
|
||||
assert.equal(
|
||||
tiny.length, 1,
|
||||
`expected exactly 1 tiny-text finding (rendered body copy), got ${tiny.length}: ${tiny.map(r => r.snippet).join('; ')}`
|
||||
);
|
||||
assert.equal(
|
||||
undersized.length, 1,
|
||||
`expected exactly 1 undersized-ui-text finding (rendered nav link), got ${undersized.length}: ${undersized.map(r => r.snippet).join('; ')}`
|
||||
);
|
||||
assert.match(undersized[0].snippet || '', /Rendered Nav Link/, 'the one undersized finding must be the rendered nav link');
|
||||
});
|
||||
});
|
||||
|
||||
describe('detectHtml — quality (static-compatible rules)', () => {
|
||||
// Six of the eight quality rules can run in static HTML/CSS because they only need
|
||||
// computed CSS values (tight-leading, tiny-text, justified-text,
|
||||
|
||||
+31
@@ -24,6 +24,18 @@
|
||||
.panel-reset { background: rgb(28, 30, 38); color: rgb(230, 232, 237); padding: 12px; }
|
||||
.panel-reset code { background: rgb(246, 242, 244); border-radius: 3px; padding: 1px 4px; }
|
||||
.panel-reset pre code { background: none; }
|
||||
/* issue #409 Case A: gradient-clipped text. The gradient IS the glyph fill
|
||||
(text-fill-color: transparent), not a backdrop. The inherited `color`
|
||||
(#e8e6e3) is never painted, so measuring it against the element's own
|
||||
gradient stops (#6d8cff / #a78bfa) is a false positive. */
|
||||
.ox-grad-text { background: linear-gradient(135deg, #6d8cff 0%, #a78bfa 50%, #6d8cff 100%); -webkit-background-clip: text; background-clip: text; -webkit-text-fill-color: transparent; color: #e8e6e3; font-size: 40px; font-weight: 800; }
|
||||
/* issue #409 Case B: a 9%-alpha accent glow stop over a dark surface. The
|
||||
stop composites to ~#121f1f, not the full-opacity #34c0a8, so text stays
|
||||
high-contrast. The dark wrapper supplies the surface beneath the glow. */
|
||||
.ox-dark-wrap { background: #0f0f11; padding: 16px; }
|
||||
.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; }
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
@@ -181,6 +193,25 @@
|
||||
<pre><code data-test="code-reset">light text over the dark panel, not the light code surface</code></pre>
|
||||
</div>
|
||||
|
||||
<h3>Gradient-clipped text (issue #409 Case A — must not flag contrast)</h3>
|
||||
<!-- background-clip: text with a transparent fill: the gradient paints the
|
||||
glyphs, not a surface behind them. The inherited light `color` is
|
||||
never painted. The gradient-text pattern still flags; the backdrop
|
||||
contrast rules (low-contrast / gray-on-color) must not. -->
|
||||
<h1 class="ox-grad-text" data-test="ox-grad-text">Gradient Clipped Heading Text</h1>
|
||||
|
||||
<h3>Faint accent-glow gradient (issue #409 Case B — must not flag contrast)</h3>
|
||||
<!-- A 9%-alpha teal glow over a dark section. Composited against the dark
|
||||
surface the stop is near-black, so the light and gray text on it are
|
||||
high-contrast. Treating the stop as opaque #34c0a8 was the false
|
||||
positive that flagged every text child. -->
|
||||
<div class="ox-dark-wrap">
|
||||
<div class="ox-glow" data-test="ox-glow">
|
||||
<p>Light body copy on a faint accent glow that composites to near-black</p>
|
||||
<p class="muted">Muted secondary line on the same faint glow area here</p>
|
||||
</div>
|
||||
</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. -->
|
||||
|
||||
@@ -0,0 +1,50 @@
|
||||
<!DOCTYPE html>
|
||||
<!--
|
||||
Regression fixture for issue #408: tiny-text / undersized-ui-text must only
|
||||
measure rendered text. On sites that set `html { font-size: 62.5% }` the root
|
||||
computes to 10px, so every element that inherits the root and carries text
|
||||
content — including <script>, <style>, <title>, <noscript>, and display:none /
|
||||
visibility:hidden blocks — reports a 10px computed size. Their JS / CSS /
|
||||
JSON-LD text satisfies hasDirectText, so before the fix they produced dozens
|
||||
of phantom "10px body text" findings on every Shopify page.
|
||||
|
||||
Explicit pixel sizes throughout because jsdom does no layout.
|
||||
-->
|
||||
<html style="font-size: 10px">
|
||||
<head>
|
||||
<meta charset="utf-8">
|
||||
<!-- SHOULD PASS: a long <title> inherits the 10px root, but nothing renders it. -->
|
||||
<title>This is a fairly long document title that easily exceeds twenty characters</title>
|
||||
<!-- SHOULD PASS: stylesheet text is never painted. -->
|
||||
<style>
|
||||
/* This CSS comment is deliberately longer than twenty characters so the style
|
||||
element's text node clears the hasDirectText / textLen > 20 gate. */
|
||||
body { font-size: 10px; font-family: system-ui, sans-serif; }
|
||||
.rendered-body { font-size: 10px; }
|
||||
.rendered-nav-link { font-size: 9px; }
|
||||
.none-block { display: none; font-size: 10px; }
|
||||
.hidden-block { visibility: hidden; font-size: 10px; }
|
||||
</style>
|
||||
<!-- SHOULD PASS: script payload text is never painted. -->
|
||||
<script>window.__ANALYTICS__ = { id: 1, ts: 0 }; console.log("an analytics payload string that is clearly longer than twenty characters");</script>
|
||||
<!-- SHOULD PASS: JSON-LD schema block, a Shopify staple. -->
|
||||
<script type="application/ld+json">{"@context":"https://schema.org","@type":"Product","name":"A product name long enough to exceed twenty characters"}</script>
|
||||
<!-- SHOULD PASS: noscript fallback prose is only shown without JS; still non-rendered here. -->
|
||||
<noscript>Please enable JavaScript in your browser to view this page content correctly.</noscript>
|
||||
</head>
|
||||
<body>
|
||||
|
||||
<!-- SHOULD PASS: display:none block, its text is never painted. -->
|
||||
<div class="none-block">Hidden display-none block of body text long enough to exceed twenty characters.</div>
|
||||
|
||||
<!-- SHOULD PASS: visibility:hidden paragraph, its text is never painted. -->
|
||||
<p class="hidden-block">Invisible visibility-hidden paragraph copy that is longer than the twenty char gate.</p>
|
||||
|
||||
<!-- SHOULD FLAG (tiny-text): genuinely rendered body copy at 10px. -->
|
||||
<p class="rendered-body">This is real rendered body copy at 10px that is definitely long enough to flag.</p>
|
||||
|
||||
<!-- SHOULD FLAG (undersized-ui-text): genuinely rendered interactive text at 9px. -->
|
||||
<nav aria-label="primary"><a href="/docs" class="rendered-nav-link">Rendered Nav Link</a></nav>
|
||||
|
||||
</body>
|
||||
</html>
|
||||
@@ -0,0 +1,50 @@
|
||||
<!DOCTYPE html>
|
||||
<!--
|
||||
Regression fixture for issue #407: DOM named-property shadowing.
|
||||
|
||||
HTMLFormElement is [LegacyOverrideBuiltIns], so a named control shadows even
|
||||
builtin getters: a <form> containing <input name="id"> makes `form.id` return
|
||||
the INPUT ELEMENT, not the id string. Reading `.startsWith` on it throws
|
||||
"elId.startsWith is not a function". Every Shopify product form ships an
|
||||
<input name="id"> (the variant id), so this crashed the URL scan of essentially
|
||||
every Shopify page. `<input name="className">` shadows `form.className` the same
|
||||
way. The detector must read these via getAttribute (immune to shadowing).
|
||||
-->
|
||||
<html lang="en">
|
||||
<head>
|
||||
<meta charset="utf-8">
|
||||
<title>Shadowed form.id regression fixture</title>
|
||||
<style>
|
||||
body { font-family: system-ui, sans-serif; background: #ffffff; color: #1a1a1a; margin: 0; padding: 24px; }
|
||||
.product { max-width: 640px; margin: 0 auto; }
|
||||
h1 { font-size: 28px; margin: 0 0 12px; }
|
||||
.price { font-size: 20px; font-weight: 600; }
|
||||
form { margin-top: 16px; }
|
||||
.add-to-cart { background: #1a1a1a; color: #ffffff; border: 0; padding: 12px 24px; border-radius: 6px; font-size: 16px; cursor: pointer; }
|
||||
label { display: block; margin: 8px 0 4px; font-size: 14px; }
|
||||
select, input[type="number"] { padding: 8px; font-size: 14px; }
|
||||
</style>
|
||||
</head>
|
||||
<body>
|
||||
<main class="product">
|
||||
<h1>Impeccable Test Product</h1>
|
||||
<p class="price">$49.00</p>
|
||||
|
||||
<!-- Shopify-style product form: the <input name="id"> shadows form.id. -->
|
||||
<form method="post" action="/cart/add" id="product-form">
|
||||
<input type="hidden" name="id" value="4001">
|
||||
<input type="hidden" name="className" value="variant-default">
|
||||
<label for="qty">Quantity</label>
|
||||
<input type="number" id="qty" name="quantity" value="1" min="1">
|
||||
<label for="variant">Variant</label>
|
||||
<select id="variant" name="options[Size]">
|
||||
<option value="s">Small</option>
|
||||
<option value="m">Medium</option>
|
||||
<option value="l">Large</option>
|
||||
</select>
|
||||
<button type="submit" class="add-to-cart">Add to cart</button>
|
||||
</form>
|
||||
</main>
|
||||
<script src="/js/detect-antipatterns-browser.js"></script>
|
||||
</body>
|
||||
</html>
|
||||
Reference in New Issue
Block a user