fix(lint): consolidate lint and audit correctness (#2413)

* fix(lint): stop CSS comments in <style> from manufacturing phantom root tags

extractOpenTags scans raw source text with a flat regex that has no
concept of <style>/<script> block boundaries, so a CSS comment like
`/* <g> wrapper */` inside a <style> block reads as a real open tag.
findRootTag consumes that flat tag list and only skips tags literally
named script/style/meta/link/title, so the phantom <g> tag (not in
that skip list) wins the "first non-ignored body tag" search and gets
returned as the composition root instead of the real one that follows.

This manufactured root_missing_composition_id and root_missing_dimensions
(the phantom tag has neither) plus head_leaked_text (the leaked-text
scan slices up to the phantom tag's position, landing inside the
<style> block before its real closing tag, so the raw CSS text reads
as leaked markup) on an otherwise valid sub-composition — reported
with an exact bisected repro: a <template>-wrapped SVG sub-composition
whose <style> block comments reference an inner <g> element.

Fix: compute <style>/<script> content spans up front (reusing the
existing extractBlocks + STYLE_BLOCK_PATTERN/SCRIPT_BLOCK_PATTERN) and
skip any TAG_PATTERN match that falls inside one, before it ever
reaches findRootTag or any other extractOpenTags consumer. Same shape
as the prior fix for a leading <svg> defs block being mistaken for the
root (8ee4b7df) — this closes a sibling gap in the same function.

Test: new regression case with a <style> block containing a `/* <g> */`
comment ahead of an <svg data-composition-id> root, asserting none of
the three findings fire. Full lint package suite (318 tests) passes.

* feat(lint): flag duplicate data-composition-id values

Declaring data-composition-id on more than one element (commonly the <meta>
tag from the quickstart template AND the root <div> added to satisfy
root_missing_composition_id) is a silent collision: `compositions --json`
returns two entries for the same id (one duration:0) and inspect/snapshot
crash with "Cannot read properties of undefined (reading totalDuration)".
Lint passed clean through all of it.

New rule `duplicate_composition_id`: group elements by data-composition-id
value and error on any value shared by 2+ elements, naming the id and calling
out the meta-vs-root collision in the fixHint. 3 tests: dup fires, single id
passes, two distinct ids don't collide. (Implemented via Codex; verified
independently: 111 lint tests pass, oxfmt/oxlint clean.)

* fix(audits): avoid caption false positives

* fix(lint): ignore proxy-label tween overlaps

* fix(cli): preserve the five-percent text audit floor

* fix(lint): preserve proxy identity across lexical scopes

* fix(cli): audit only directly painted text

* fix(lint): compare live composition ids canonically

* fix(lint): preserve expanded proxy identities

* fix(cli): measure directly painted text geometry

* fix(lint): preserve first duplicate attribute value

* fix(lint): keep shared proxy identity across helpers

* fix(parsers): preserve expanded proxy identity

* fix(lint): decode composition IDs consistently
This commit is contained in:
Miguel Ángel
2026-07-14 20:13:01 -04:00
committed by GitHub
parent 37e1b26434
commit ada878fdcd
16 changed files with 759 additions and 64 deletions
@@ -219,10 +219,10 @@
const text = textContentFor(element, directOnly);
if (!text) return false;
if (directOnly) return true;
for (const child of Array.from(element.children)) {
if (isVisibleElement(child) && textContentFor(child)) return false;
}
return true;
// Aggregate text may come exclusively from descendants (including hidden
// captions). The container itself does not paint that text and must not be
// audited as though it did.
return textContentFor(element, true).length > 0;
}
function textClientRects(element, directOnly) {
@@ -436,9 +436,9 @@
}
function textOverflowIssues(element, root, rootRect, time, tolerance) {
const textRect = textRectFor(element);
const textRect = textRectFor(element, true);
if (!textRect) return [];
const text = textContentFor(element);
const text = textContentFor(element, true);
const selector = selectorFor(element);
const issues = [];
@@ -571,7 +571,7 @@
// (low colour alpha) is decorative and exempt, as are elements opted out with
// data-layout-allow-overlap.
function isSolidTextBlock(element) {
if (!isVisibleElement(element) || !hasOwnTextCandidate(element)) return false;
if (!isVisibleElement(element) || !hasOwnTextCandidate(element, true)) return false;
if (hasAllowOverlapFlag(element)) return false;
return colorAlpha(getComputedStyle(element).color) >= 0.35;
}
@@ -580,7 +580,7 @@
const blocks = [];
for (const element of Array.from(root.querySelectorAll("*"))) {
if (!isSolidTextBlock(element)) continue;
const rect = textRectFor(element);
const rect = textRectFor(element, true);
if (rect) blocks.push({ element, rect });
}
return blocks;
@@ -983,9 +983,9 @@
function occludedTextIssue(element, time) {
if (hasAllowOcclusionFlag(element)) return null;
if (!hasVisibleTextInk(element)) return null;
const textRect = textRectFor(element);
const textRect = textRectFor(element, true);
if (!textRect) return null;
const text = textContentFor(element);
const text = textContentFor(element, true);
const { occluder, coveredFraction } = occlusionCoverage(element, textRect);
if (!occluder) return null;
if (!isAtomicLabel(text) && coveredFraction < PROSE_COVERAGE_FLOOR) return null;
@@ -1016,9 +1016,9 @@
// paints the glyphs; a `background-clip: text` with no gradient/image and no
// opaque background-color paints nothing, so it stays reportable.
function invisibleTextIssue(element, time) {
const textRect = textRectFor(element);
const textRect = textRectFor(element, true);
if (!textRect) return null;
const text = textContentFor(element);
const text = textContentFor(element, true);
if (!text) return null;
const cs = getComputedStyle(element);
// Vendor computed-style props are read by property (camelCase), matching
@@ -1141,8 +1141,8 @@
if (escapedElements.has(element)) continue;
// Ownership is geometric and strict-mutex: any text breach past canvas_overflow's own
// tolerance cedes the element to canvas_overflow; in-bounds text leaves the panel finding.
if (hasOwnTextCandidate(element)) {
const textRect = textRectFor(element);
if (hasOwnTextCandidate(element, true)) {
const textRect = textRectFor(element, true);
if (textRect && overflowFor(textRect, rootRect, tolerance)) continue;
}
const rect = toRect(element.getBoundingClientRect());
@@ -1372,7 +1372,7 @@
document.body;
const rootRect = rootRectFor(root);
const elements = Array.from(root.querySelectorAll("*")).filter((element) =>
isVisibleElement(element),
isVisibleElement(element, 0.05),
);
const issues = [];
@@ -242,6 +242,31 @@ describe("layout-audit.browser", () => {
]),
);
});
it("does not expand a parent's overflow geometry to a positioned descendant", () => {
document.body.innerHTML = `
<div id="root" data-composition-id="main" data-width="640" data-height="360">
<div id="headline">Visible copy<span id="positioned-copy">Positioned copy</span></div>
</div>
`;
installGeometry(
{
root: rect({ left: 0, top: 0, width: 640, height: 360 }),
headline: rect({ left: 40, top: 60, width: 200, height: 40 }),
"positioned-copy": rect({ left: 700, top: 60, width: 160, height: 40 }),
headlineText: rect({ left: 40, top: 60, width: 120, height: 40 }),
"positioned-copyText": rect({ left: 700, top: 60, width: 160, height: 40 }),
text: rect({ left: 40, top: 60, width: 820, height: 40 }),
},
{ "positioned-copy": { position: "absolute" } },
);
installAuditScript();
const parentOverflow = runAudit().find(
(issue) => issue.code === "canvas_overflow" && issue.selector === "#headline",
);
expect(parentOverflow).toBeUndefined();
});
});
it("is inert unless text or media candidates are explicitly requested", () => {
@@ -1309,7 +1334,11 @@ function auditOverlapScene(options: {
selected = node;
},
getClientRects() {
const id = (selected as Element | null)?.id ?? "";
const element =
selected?.nodeType === Node.TEXT_NODE
? selected.parentElement
: (selected as Element | null);
const id = element?.id ?? "";
return textRects[id]
? ([textRects[id]] as unknown as DOMRectList)
: ([] as unknown as DOMRectList);
@@ -1389,6 +1418,28 @@ describe("layout-audit.browser occlusion", () => {
expect(issues.some((issue) => issue.code === "text_occluded")).toBe(false);
});
it("does not treat a visible container as painted text when its only text child is hidden", () => {
document.body.innerHTML = `
<div id="root" data-composition-id="main" data-width="1920" data-height="1080">
<div id="caption-container"><span id="caption">Hidden caption</span></div>
<div id="overlay"></div>
</div>
`;
installOcclusionGeometry({
styleOverrides: {
caption: { opacity: "0" },
overlay: { backgroundColor: "rgb(10, 10, 10)" },
},
headlineTextRect: rect({ left: 200, top: 500, width: 600, height: 80 }),
topmostId: "overlay",
textRectElementId: "caption-container",
});
installAuditScript();
const issues = runAudit();
expect(issues.some((issue) => issue.code === "text_occluded")).toBe(false);
});
it("carries the fully-covered fraction when the occluder hits every probe point", () => {
const occluded = auditOcclusionScene({
overlayStyle: { backgroundColor: "rgb(10, 10, 10)" },
@@ -1450,6 +1501,47 @@ describe("layout-audit.browser occlusion", () => {
expect(runAudit().some((issue) => issue.code === "text_occluded")).toBe(false);
});
it("audits only a container's direct text when a hidden descendant also has text", () => {
document.body.innerHTML = `
<div id="root" data-composition-id="main" data-width="1920" data-height="1080">
<div id="headline">Visible copy<span id="hidden-copy">Hidden copy</span></div>
<div id="overlay"></div>
</div>
`;
installOcclusionGeometry({
styleOverrides: {
"hidden-copy": { opacity: "0" },
overlay: { backgroundColor: "rgb(10, 10, 10)" },
},
headlineTextRect: rect({ left: 200, top: 500, width: 600, height: 80 }),
topmostId: "overlay",
});
installAuditScript();
const issue = runAudit().find((candidate) => candidate.code === "text_occluded");
expect(issue?.text).toBe("Visible copy");
});
it("does not expand a container's text audit to a positioned descendant", () => {
document.body.innerHTML = `
<div id="root" data-composition-id="main" data-width="1920" data-height="1080">
<div id="headline">Visible copy<span id="positioned-copy">Positioned copy</span></div>
<div id="overlay"></div>
</div>
`;
installOcclusionGeometry({
styleOverrides: {
"positioned-copy": { position: "absolute" },
overlay: { backgroundColor: "rgb(10, 10, 10)" },
},
headlineTextRect: rect({ left: 200, top: 500, width: 600, height: 80 }),
topmostId: "overlay",
});
installAuditScript();
const issues = runAudit().filter((candidate) => candidate.code === "text_occluded");
const headlineIssue = issues.find((candidate) => candidate.selector === "#headline");
expect(headlineIssue?.text).toBe("Visible copy");
});
it("does not count a low-alpha gradient overlay (grid/scrim) as an opaque occluder", () => {
const issues = auditOcclusionScene({
overlayStyle: {
@@ -1520,6 +1612,7 @@ describe("layout-audit.browser occlusion", () => {
},
headlineTextRect: rect({ left: 200, top: 500, width: 600, height: 80 }),
topmostId: "overlay",
textRectElementId: "inner",
});
installAuditScript();
expect(runAudit().some((issue) => issue.code === "text_occluded")).toBe(true);
@@ -1687,6 +1780,7 @@ function installOcclusionGeometry(options: {
styleOverrides: Record<string, Partial<Record<string, string>>>;
headlineTextRect: DOMRect;
topmostId: string;
textRectElementId?: string;
}): void {
const baseStyle: Record<string, string> = {
display: "block",
@@ -1733,7 +1827,11 @@ function installOcclusionGeometry(options: {
selected = node;
},
getClientRects() {
return (selected as Element | null)?.id === "headline"
const selectedElement =
selected?.nodeType === Node.TEXT_NODE
? (selected.parentElement as Element | null)
: (selected as Element | null);
return selectedElement?.id === (options.textRectElementId ?? "headline")
? ([options.headlineTextRect] as unknown as DOMRectList)
: ([] as unknown as DOMRectList);
},
@@ -1864,6 +1962,7 @@ async function runContrastAudit(): Promise<Array<Record<string, unknown>>> {
interface AuditIssue {
code: string;
selector: string;
text?: string;
containerSelector?: string;
overflow?: Record<string, number>;
message?: string;
@@ -1880,6 +1979,20 @@ function runAudit(): AuditIssue[] {
return audit({ time: 1, tolerance: 2 });
}
function selectedRangeElement(selected: Node | null): Element | null {
return selected?.nodeType === Node.TEXT_NODE
? (selected.parentElement as Element | null)
: (selected as Element | null);
}
function rangeTextRect(selected: Node | null, rects: Record<string, DOMRect>): DOMRect | undefined {
const element = selectedRangeElement(selected);
if (element?.id === "ignored") return rects.ignored;
if (selected?.nodeType === Node.TEXT_NODE && element?.id)
return rects[`${element.id}Text`] ?? rects.text;
return rects.text;
}
function installGeometry(
rects: Record<string, DOMRect>,
styleOverrides: Record<string, Partial<CSSStyleDeclaration>> = {},
@@ -1934,8 +2047,7 @@ function installGeometry(
selected = node;
},
getClientRects() {
const element = selected as Element | null;
const textRect = element?.id === "ignored" ? rects.ignored : rects.text;
const textRect = rangeTextRect(selected, rects);
return textRect ? ([textRect] as unknown as DOMRectList) : ([] as unknown as DOMRectList);
},
detach() {},