From 65bbd6cb5f6f1eda4818f9544056a09bd9163885 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Mon, 27 Apr 2026 14:09:49 -0700 Subject: [PATCH 1/2] feat(detector): harden border-radius reads against jsdom CSS regressions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds resolveBorderRadiusPx(el, style, widthPx, win), a helper that walks computed style → longhand → inline DOM → raw style attribute → matching stylesheet rules to recover a pixel value, converting % to px when needed. Three jsdom adapter sites now use it: checkElementBorders (via a new optional resolvedRadius param threaded from detectHtml), the icon-tile sibling check in checkElementIconTile, and isCardLike's hasRadius gate. Browser DOM adapters hit the fast path on the first line since real getComputedStyle resolves both shorthand and percentages. Background: from jsdom 29.0.2 onward, getComputedStyle(el).borderRadius returns "" for the shorthand and "0" for longhand reads when the rule used the shorthand. checkIconTile relied on parseFloat(borderRadius) >= width/2 to exclude circular avatars; that comparison broke and circles got false-flagged as icon-tile-stack. jsdom 29.1.0 has a separate parser crash on + linear-gradient inline style which keeps the pin at exactly 29.0.0 for now, but landing the helper means we can move forward as soon as the gradient crash is fixed upstream without touching detector code again. The change is also strictly more correct than the old parseFloat approach: percentage values now convert to actual pixel sizes, so checkIconTile no longer relies on parseFloat("50%") == 50 happening to satisfy `>= width/2` only for elements <= 100px wide. bun run test passes (174/174); bun run build:browser and bun run build:extension regenerated to mirror the helper into bundled artifacts. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/detect-antipatterns-browser.js | 84 +++++++++++++++++++++++++++-- src/detect-antipatterns.mjs | 87 ++++++++++++++++++++++++++++-- 2 files changed, 162 insertions(+), 9 deletions(-) diff --git a/src/detect-antipatterns-browser.js b/src/detect-antipatterns-browser.js index 76830e57e..f7084ae9b 100644 --- a/src/detect-antipatterns-browser.js +++ b/src/detect-antipatterns-browser.js @@ -885,6 +885,74 @@ function resolveGradientStops(el, win) { return 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. +// Percentages need a `widthPx` reference to convert against. +function parseRadiusToPx(value, widthPx) { + if (!value || typeof value !== 'string') return null; + const trimmed = value.trim(); + if (!trimmed) return null; + const first = trimmed.split(/\s+/)[0]; + const num = parseFloat(first); + if (Number.isNaN(num)) return null; + if (/%$/.test(first)) return (num / 100) * (widthPx || 0); + return num; +} + +// jsdom from 29.0.2 onward returns "" for the `border-radius` shorthand +// in computed style and "0" for longhand reads when the source rule used +// the shorthand. The rule engine relied on parseFloat(style.borderRadius) +// to identify circular avatars (border-radius >= width/2) and rounded +// cards (border-radius > 0); both checks broke silently. This helper +// recovers the radius via a chain of fallbacks. Browsers resolve the +// shorthand correctly and exit on the first line. +function resolveBorderRadiusPx(el, style, widthPx, win) { + const fromComputed = parseRadiusToPx(style.borderRadius, widthPx); + if (fromComputed !== null) return fromComputed; + + if (IS_BROWSER || !win) return 0; + + const fromLonghand = parseRadiusToPx(style.borderTopLeftRadius, widthPx); + if (fromLonghand !== null && fromLonghand > 0) return fromLonghand; + + const fromInlineProp = parseRadiusToPx(el.style?.borderRadius, widthPx); + if (fromInlineProp !== null) return fromInlineProp; + + const rawStyle = el.getAttribute?.('style') || ''; + const inlineMatch = rawStyle.match(/border-radius\s*:\s*([^;]+)/i); + if (inlineMatch) { + const fromRaw = parseRadiusToPx(inlineMatch[1].trim(), widthPx); + if (fromRaw !== null) return fromRaw; + } + + // Walk every stylesheet looking for matching rules. Take the maximum + // pixel value across all matches so a circle declaration overridden by + // a more specific rounded-square selector still registers as a circle + // for the exclusion check (better to under-flag than to false-positive + // on round avatars). + let max = 0; + const sheets = win.document?.styleSheets; + if (sheets) { + for (const sheet of sheets) { + let rules; + try { rules = sheet.cssRules || []; } catch { continue; } + for (const rule of rules) { + if (!rule.style || !rule.selectorText) continue; + let matches = false; + try { matches = el.matches(rule.selectorText); } catch { continue; } + if (!matches) continue; + const ruleValue = rule.style.borderRadius + || (rule.style.getPropertyValue && rule.style.getPropertyValue('border-radius')) + || rule.style.borderTopLeftRadius; + const px = parseRadiusToPx(ruleValue, widthPx); + if (px !== null && px > max) max = px; + } + } + } + return max; +} + // ─── Section 5: Element Adapters ──────────────────────────────────────────── // Browser adapters — call getComputedStyle/getBoundingClientRect on live DOM @@ -1271,7 +1339,7 @@ function checkElementQuality(el, style, tag, window) { return checkQuality({ el, tag, style, hasDirectText, textLen, fontSize, lineHeightPx, letterSpacingPx, rect: null }); } -function checkElementBorders(tag, style, overrides) { +function checkElementBorders(tag, style, overrides, resolvedRadius) { const sides = ['Top', 'Right', 'Bottom', 'Left']; const widths = {}, colors = {}; for (const s of sides) { @@ -1291,7 +1359,14 @@ function checkElementBorders(tag, style, overrides) { colors[s] = overrides[s].color; } } - return checkBorders(tag, widths, colors, parseFloat(style.borderRadius) || 0); + // resolvedRadius lets the caller pre-resolve the radius via + // resolveBorderRadiusPx so the value survives jsdom 29.1.0's broken + // shorthand serialization. Falls back to the computed value for tests + // and browser callers that don't pre-resolve. + const radius = resolvedRadius != null + ? resolvedRadius + : (parseFloat(style.borderRadius) || 0); + return checkBorders(tag, widths, colors, radius); } function checkElementColors(el, style, tag, window) { @@ -1346,7 +1421,7 @@ function checkElementIconTile(el, tag, window) { siblingBgColor: parseRgb(sibStyle.backgroundColor), siblingBgImage: sibStyle.backgroundImage || '', siblingBorderWidth: parseFloat(sibStyle.borderTopWidth) || 0, - siblingBorderRadius: parseFloat(sibStyle.borderRadius) || 0, + siblingBorderRadius: resolveBorderRadiusPx(sibling, sibStyle, sibWidth, window), hasIconChild: !!iconChild || hasInlineEmojiIcon, iconChildWidth: iconWidth, }); @@ -1565,7 +1640,8 @@ function isCardLike(el, win) { const hasShadow = (style.boxShadow && style.boxShadow !== 'none') || /\bshadow(?:-sm|-md|-lg|-xl|-2xl)?\b/.test(cls) || /box-shadow/i.test(rawStyle); const hasBorder = /\bborder\b/.test(cls); - const hasRadius = (parseFloat(style.borderRadius) || 0) > 0 || + const widthPx = parseFloat(style.width) || 0; + const hasRadius = resolveBorderRadiusPx(el, style, widthPx, win) > 0 || /\brounded(?:-sm|-md|-lg|-xl|-2xl|-full)?\b/.test(cls) || /border-radius/i.test(rawStyle); const hasBg = /\bbg-(?:white|gray-\d+|slate-\d+)\b/.test(cls) || /background(?:-color)?\s*:\s*(?!transparent)/i.test(rawStyle); diff --git a/src/detect-antipatterns.mjs b/src/detect-antipatterns.mjs index e196f3a8c..63b1edf7c 100644 --- a/src/detect-antipatterns.mjs +++ b/src/detect-antipatterns.mjs @@ -880,6 +880,74 @@ function resolveGradientStops(el, win) { return 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. +// Percentages need a `widthPx` reference to convert against. +function parseRadiusToPx(value, widthPx) { + if (!value || typeof value !== 'string') return null; + const trimmed = value.trim(); + if (!trimmed) return null; + const first = trimmed.split(/\s+/)[0]; + const num = parseFloat(first); + if (Number.isNaN(num)) return null; + if (/%$/.test(first)) return (num / 100) * (widthPx || 0); + return num; +} + +// jsdom from 29.0.2 onward returns "" for the `border-radius` shorthand +// in computed style and "0" for longhand reads when the source rule used +// the shorthand. The rule engine relied on parseFloat(style.borderRadius) +// to identify circular avatars (border-radius >= width/2) and rounded +// cards (border-radius > 0); both checks broke silently. This helper +// recovers the radius via a chain of fallbacks. Browsers resolve the +// shorthand correctly and exit on the first line. +function resolveBorderRadiusPx(el, style, widthPx, win) { + const fromComputed = parseRadiusToPx(style.borderRadius, widthPx); + if (fromComputed !== null) return fromComputed; + + if (IS_BROWSER || !win) return 0; + + const fromLonghand = parseRadiusToPx(style.borderTopLeftRadius, widthPx); + if (fromLonghand !== null && fromLonghand > 0) return fromLonghand; + + const fromInlineProp = parseRadiusToPx(el.style?.borderRadius, widthPx); + if (fromInlineProp !== null) return fromInlineProp; + + const rawStyle = el.getAttribute?.('style') || ''; + const inlineMatch = rawStyle.match(/border-radius\s*:\s*([^;]+)/i); + if (inlineMatch) { + const fromRaw = parseRadiusToPx(inlineMatch[1].trim(), widthPx); + if (fromRaw !== null) return fromRaw; + } + + // Walk every stylesheet looking for matching rules. Take the maximum + // pixel value across all matches so a circle declaration overridden by + // a more specific rounded-square selector still registers as a circle + // for the exclusion check (better to under-flag than to false-positive + // on round avatars). + let max = 0; + const sheets = win.document?.styleSheets; + if (sheets) { + for (const sheet of sheets) { + let rules; + try { rules = sheet.cssRules || []; } catch { continue; } + for (const rule of rules) { + if (!rule.style || !rule.selectorText) continue; + let matches = false; + try { matches = el.matches(rule.selectorText); } catch { continue; } + if (!matches) continue; + const ruleValue = rule.style.borderRadius + || (rule.style.getPropertyValue && rule.style.getPropertyValue('border-radius')) + || rule.style.borderTopLeftRadius; + const px = parseRadiusToPx(ruleValue, widthPx); + if (px !== null && px > max) max = px; + } + } + } + return max; +} + // ─── Section 5: Element Adapters ──────────────────────────────────────────── // Browser adapters — call getComputedStyle/getBoundingClientRect on live DOM @@ -1266,7 +1334,7 @@ function checkElementQuality(el, style, tag, window) { return checkQuality({ el, tag, style, hasDirectText, textLen, fontSize, lineHeightPx, letterSpacingPx, rect: null }); } -function checkElementBorders(tag, style, overrides) { +function checkElementBorders(tag, style, overrides, resolvedRadius) { const sides = ['Top', 'Right', 'Bottom', 'Left']; const widths = {}, colors = {}; for (const s of sides) { @@ -1286,7 +1354,14 @@ function checkElementBorders(tag, style, overrides) { colors[s] = overrides[s].color; } } - return checkBorders(tag, widths, colors, parseFloat(style.borderRadius) || 0); + // resolvedRadius lets the caller pre-resolve the radius via + // resolveBorderRadiusPx so the value survives jsdom 29.1.0's broken + // shorthand serialization. Falls back to the computed value for tests + // and browser callers that don't pre-resolve. + const radius = resolvedRadius != null + ? resolvedRadius + : (parseFloat(style.borderRadius) || 0); + return checkBorders(tag, widths, colors, radius); } function checkElementColors(el, style, tag, window) { @@ -1341,7 +1416,7 @@ function checkElementIconTile(el, tag, window) { siblingBgColor: parseRgb(sibStyle.backgroundColor), siblingBgImage: sibStyle.backgroundImage || '', siblingBorderWidth: parseFloat(sibStyle.borderTopWidth) || 0, - siblingBorderRadius: parseFloat(sibStyle.borderRadius) || 0, + siblingBorderRadius: resolveBorderRadiusPx(sibling, sibStyle, sibWidth, window), hasIconChild: !!iconChild || hasInlineEmojiIcon, iconChildWidth: iconWidth, }); @@ -1560,7 +1635,8 @@ function isCardLike(el, win) { const hasShadow = (style.boxShadow && style.boxShadow !== 'none') || /\bshadow(?:-sm|-md|-lg|-xl|-2xl)?\b/.test(cls) || /box-shadow/i.test(rawStyle); const hasBorder = /\bborder\b/.test(cls); - const hasRadius = (parseFloat(style.borderRadius) || 0) > 0 || + const widthPx = parseFloat(style.width) || 0; + const hasRadius = resolveBorderRadiusPx(el, style, widthPx, win) > 0 || /\brounded(?:-sm|-md|-lg|-xl|-2xl|-full)?\b/.test(cls) || /border-radius/i.test(rawStyle); const hasBg = /\bbg-(?:white|gray-\d+|slate-\d+)\b/.test(cls) || /background(?:-color)?\s*:\s*(?!transparent)/i.test(rawStyle); @@ -2643,7 +2719,8 @@ async function detectHtml(filePath) { for (const el of document.querySelectorAll('*')) { const tag = el.tagName.toLowerCase(); const style = window.getComputedStyle(el); - for (const f of checkElementBorders(tag, style, borderOverrides.get(el))) { + const resolvedRadius = resolveBorderRadiusPx(el, style, parseFloat(style.width) || 0, window); + for (const f of checkElementBorders(tag, style, borderOverrides.get(el), resolvedRadius)) { findings.push(finding(f.id, filePath, f.snippet)); } for (const f of checkElementColors(el, style, tag, window)) { From 28875097b0529f4a258df8be5fcd9f258fe526a6 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Mon, 27 Apr 2026 14:33:54 -0700 Subject: [PATCH 2/2] fix(detector): preserve percent-radius signal when width is missing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit parseRadiusToPx("50%", 0) used to return 0, and resolveBorderRadiusPx's "if (fromComputed !== null) return fromComputed" guard short-circuited with that 0 before ever consulting longhand / inline / stylesheet fallbacks. Callers that gate on `> 0` (border-accent-on-rounded and isCardLike's hasRadius) silently lost findings the old parseFloat(style.borderRadius) === 50 heuristic happened to keep. In jsdom this is reachable any time style.width resolves to "auto" or an empty string — parseFloat yields NaN, the `|| 0` fallback turns it into 0, and any percent radius collapses to nothing. Real-world cards with `width: 100%` hit this on every load. Fix: when widthPx is 0 / missing, return the raw percentage number instead. The percent-to-px conversion only makes sense with a width reference; without one, the value still serves as a positive presence signal for boolean checks. The icon-tile circle exclusion is unaffected because that rule already gates on `siblingWidth >= 32`. Caught by Cursor Bugbot on PR #115. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/detect-antipatterns-browser.js | 12 ++++++++++-- src/detect-antipatterns.mjs | 12 ++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/src/detect-antipatterns-browser.js b/src/detect-antipatterns-browser.js index f7084ae9b..b147f45fb 100644 --- a/src/detect-antipatterns-browser.js +++ b/src/detect-antipatterns-browser.js @@ -888,7 +888,12 @@ function resolveGradientStops(el, win) { // 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. -// Percentages need a `widthPx` reference to convert against. +// Percentages convert against `widthPx` when one is supplied. Without a +// usable width (jsdom returns "auto" for many real-world elements, +// which parseFloat collapses to 0), fall back to the raw percentage +// number so callers gating on `> 0` (border-accent-on-rounded, +// isCardLike's hasRadius) still see a positive value, matching the +// original parseFloat("50%") === 50 behavior. function parseRadiusToPx(value, widthPx) { if (!value || typeof value !== 'string') return null; const trimmed = value.trim(); @@ -896,7 +901,10 @@ function parseRadiusToPx(value, widthPx) { const first = trimmed.split(/\s+/)[0]; const num = parseFloat(first); if (Number.isNaN(num)) return null; - if (/%$/.test(first)) return (num / 100) * (widthPx || 0); + if (/%$/.test(first)) { + if (widthPx && widthPx > 0) return (num / 100) * widthPx; + return num; + } return num; } diff --git a/src/detect-antipatterns.mjs b/src/detect-antipatterns.mjs index 63b1edf7c..c17bf78dd 100644 --- a/src/detect-antipatterns.mjs +++ b/src/detect-antipatterns.mjs @@ -883,7 +883,12 @@ function resolveGradientStops(el, win) { // 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. -// Percentages need a `widthPx` reference to convert against. +// Percentages convert against `widthPx` when one is supplied. Without a +// usable width (jsdom returns "auto" for many real-world elements, +// which parseFloat collapses to 0), fall back to the raw percentage +// number so callers gating on `> 0` (border-accent-on-rounded, +// isCardLike's hasRadius) still see a positive value, matching the +// original parseFloat("50%") === 50 behavior. function parseRadiusToPx(value, widthPx) { if (!value || typeof value !== 'string') return null; const trimmed = value.trim(); @@ -891,7 +896,10 @@ function parseRadiusToPx(value, widthPx) { const first = trimmed.split(/\s+/)[0]; const num = parseFloat(first); if (Number.isNaN(num)) return null; - if (/%$/.test(first)) return (num / 100) * (widthPx || 0); + if (/%$/.test(first)) { + if (widthPx && widthPx > 0) return (num / 100) * widthPx; + return num; + } return num; }