From 65bbd6cb5f6f1eda4818f9544056a09bd9163885 Mon Sep 17 00:00:00 2001 From: Paul Bakaus Date: Mon, 27 Apr 2026 14:09:49 -0700 Subject: [PATCH] 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)) {