diff --git a/packages/cli/src/commands/layout-audit.browser.js b/packages/cli/src/commands/layout-audit.browser.js index 919b7ed59e..0655e1650d 100644 --- a/packages/cli/src/commands/layout-audit.browser.js +++ b/packages/cli/src/commands/layout-audit.browser.js @@ -439,7 +439,7 @@ return false; } - function textOverflowIssues(element, root, rootRect, time, tolerance) { + function textOverflowIssues(element, root, rootRect, time, tolerance, clippedIssue) { const textRect = textRectFor(element, true); if (!textRect) return []; const text = textContentFor(element, true); @@ -462,8 +462,15 @@ ? tolerance : Math.max(tolerance, parsePx(elementStyle.fontSize) * 0.2); const containerOverflow = overflowFor(textRect, containerRect, tolerance, verticalTolerance); + const billedAsClippedText = + container === element && + clippedIssue != null && + containerOverflow != null && + containerOverflow.left == null && + containerOverflow.top == null; if ( containerOverflow && + !billedAsClippedText && !hasTextClipOptOut(element) && !clippedByAncestor(element, container) ) { @@ -663,8 +670,8 @@ code: "content_overlap", severity: "warning", time, - selector: selectorFor(a.element), - containerSelector: selectorFor(b.element), + selector: uniqueSelectorFor(a.element), + containerSelector: uniqueSelectorFor(b.element), text: textContentFor(a.element), message: "Two text blocks overlap and may render unreadable.", rect: a.rect, @@ -1021,8 +1028,8 @@ code: "text_occluded", severity: "error", time, - selector: selectorFor(element), - containerSelector: selectorFor(occluder), + selector: uniqueSelectorFor(element), + containerSelector: uniqueSelectorFor(occluder), text, message: "Text is hidden beneath an opaque element.", rect: textRect, @@ -1444,7 +1451,7 @@ if (!hasOwnTextCandidate(element)) continue; const clipped = clippedTextIssue(element, time, tolerance); if (clipped) issues.push(clipped); - issues.push(...textOverflowIssues(element, root, rootRect, time, tolerance)); + issues.push(...textOverflowIssues(element, root, rootRect, time, tolerance, clipped)); const occluded = occludedTextIssue(element, time, proseCoverageFloor); if (occluded) issues.push(occluded); const invisible = invisibleTextIssue(element, time); diff --git a/packages/cli/src/commands/layout-audit.browser.test.ts b/packages/cli/src/commands/layout-audit.browser.test.ts index 783b5e877a..3886d27d77 100644 --- a/packages/cli/src/commands/layout-audit.browser.test.ts +++ b/packages/cli/src/commands/layout-audit.browser.test.ts @@ -292,7 +292,7 @@ describe("layout-audit.browser", () => { .map((issue) => issue.code) .filter((code) => code === "clipped_text" || code === "text_box_overflow"); - expect(textOverflowCodes()).toEqual(["clipped_text", "text_box_overflow"]); + expect(textOverflowCodes()).toEqual(["clipped_text"]); document.querySelector("#overflow-optout")?.setAttribute("data-layout-allow-overflow", ""); expect(textOverflowCodes()).toEqual([]); document.querySelector("#overflow-optout")?.removeAttribute("data-layout-allow-overflow"); @@ -300,6 +300,88 @@ describe("layout-audit.browser", () => { expect(textOverflowCodes()).toEqual([]); }); + it("still flags a clipping self-constraint whose scroll metrics round below tolerance", () => { + document.body.innerHTML = ` +
+
Intentional long truncated label
+
+ `; + const headline = document.querySelector("#headline"); + if (!(headline instanceof HTMLElement)) throw new Error("missing headline"); + Object.defineProperties(headline, { + clientWidth: { configurable: true, value: 200 }, + scrollWidth: { configurable: true, value: 202 }, + clientHeight: { configurable: true, value: 20 }, + scrollHeight: { configurable: true, value: 20 }, + }); + installGeometry( + { + root: rect({ left: 0, top: 0, width: 640, height: 360 }), + headline: rect({ left: 40, top: 60, width: 200, height: 20 }), + text: rect({ left: 40, top: 60, width: 203.4, height: 20 }), + }, + { + headline: { overflow: "hidden", overflowX: "hidden", overflowY: "hidden" }, + }, + ); + installAuditScript(); + const codes = runAudit() + .map((issue) => issue.code) + .filter((code) => code === "clipped_text" || code === "text_box_overflow"); + + expect(codes).toEqual(["text_box_overflow"]); + }); + + it("still flags a clipping self-constraint whose text runs off to the left", () => { + document.body.innerHTML = ` +
+
Intentional long truncated label
+
+ `; + const headline = document.querySelector("#headline"); + if (!(headline instanceof HTMLElement)) throw new Error("missing headline"); + Object.defineProperties(headline, { + clientWidth: { configurable: true, value: 100 }, + scrollWidth: { configurable: true, value: 100 }, + clientHeight: { configurable: true, value: 20 }, + scrollHeight: { configurable: true, value: 20 }, + }); + installGeometry( + { + root: rect({ left: 0, top: 0, width: 640, height: 360 }), + headline: rect({ left: 140, top: 60, width: 100, height: 20 }), + text: rect({ left: 40, top: 60, width: 200, height: 20 }), + }, + { + headline: { overflow: "hidden", overflowX: "hidden", overflowY: "hidden" }, + }, + ); + installAuditScript(); + + const found = runAudit().filter((issue) => issue.code === "text_box_overflow"); + expect(found).toHaveLength(1); + expect(found[0]?.overflow?.left).toBe(100); + expect(runAudit().some((issue) => issue.code === "clipped_text")).toBe(false); + }); + + it("still flags a painted, NON-clipping box that is its own nearest constraint", () => { + document.body.innerHTML = ` +
+
Enterprise plan includes unlimited renders
+
+ `; + installGeometry({ + root: rect({ left: 0, top: 0, width: 640, height: 360 }), + bubble: rect({ left: 40, top: 60, width: 200, height: 40 }), + text: rect({ left: 40, top: 65, width: 520, height: 30 }), + }); + installAuditScript(); + + const found = runAudit().filter((issue) => issue.code === "text_box_overflow"); + expect(found).toHaveLength(1); + expect(found[0]?.selector).toBe("#bubble"); + }); + it("does not flag glyph-ink vertical spill within the font-metric band on a non-clipping box", () => { // A painted, non-clipping caption-word-like box whose glyph ink (text rect) exceeds its snug // line-height box by a few px vertically — normal typography, nothing is clipped. (fontSize diff --git a/packages/cli/src/utils/layoutAudit.test.ts b/packages/cli/src/utils/layoutAudit.test.ts index 13ea482e32..c4048d16e3 100644 --- a/packages/cli/src/utils/layoutAudit.test.ts +++ b/packages/cli/src/utils/layoutAudit.test.ts @@ -203,6 +203,7 @@ describe("layoutAudit helpers", () => { // Sample counts below (9) mirror the CLI's default grid so the "1 sample = // entrance/exit transient, 2+ adjacent samples = held" framing in the // approach doc lines up with the numbers used here. + describe("persistence-tiered severity (#U10)", () => { it("demotes a content_overlap seen at only one sample among several to info", () => { const collapsed = collapseStaticLayoutIssues( @@ -255,6 +256,133 @@ describe("persistence-tiered severity (#U10)", () => { expect(collapsed[0]).toMatchObject({ severity: "warning", occurrences: 2 }); }); + it("promotes a content_overlap whose text changes every sample (count-up over a label)", () => { + const collapsed = collapseStaticLayoutIssues( + [ + { + ...issue("content_overlap", "warning"), + time: 4.0, + containerSelector: ".num", + text: "$1,204", + }, + { + ...issue("content_overlap", "warning"), + time: 4.5, + containerSelector: ".num", + text: "$8,930", + }, + ], + 73, + ); + + expect(collapsed).toHaveLength(1); + expect(collapsed[0]).toMatchObject({ severity: "error", occurrences: 2 }); + }); + + it("keeps a text_occluded over changing text at error, not demoted per sample", () => { + const collapsed = collapseStaticLayoutIssues( + [ + { + ...issue("text_occluded", "error"), + time: 4.0, + containerSelector: ".scrim", + text: "$1,204", + }, + { + ...issue("text_occluded", "error"), + time: 4.5, + containerSelector: ".scrim", + text: "$8,930", + }, + ], + 73, + ); + + expect(collapsed).toHaveLength(1); + expect(collapsed[0]).toMatchObject({ severity: "error", occurrences: 2 }); + }); + + it("keeps two content_overlap pairs on different containers in separate groups", () => { + const collapsed = collapseStaticLayoutIssues( + [ + { ...issue("content_overlap", "warning"), time: 4.0, containerSelector: ".num" }, + { ...issue("content_overlap", "warning"), time: 4.5, containerSelector: ".pct" }, + ], + 73, + ); + + expect(collapsed).toHaveLength(2); + }); + + it("does not bridge two separate transients on one pair into a held collision", () => { + const blip = { ...issue("content_overlap", "warning"), containerSelector: ".label" }; + const collapsed = collapseStaticLayoutIssues( + [ + { ...blip, time: 1.0 }, + { ...blip, time: 1.125 }, + { ...blip, time: 9.0 }, + { ...blip, time: 9.125 }, + ], + 73, + ); + + expect(collapsed).toHaveLength(1); + expect(collapsed[0]).toMatchObject({ severity: "warning", occurrences: 4 }); + }); + + it("promotes only when one contiguous run clears the floor, not the span between runs", () => { + const blip = { ...issue("content_overlap", "warning"), containerSelector: ".label" }; + const held = [1.0, 1.125, 1.25, 1.375, 1.5, 1.625].map((time) => ({ ...blip, time })); + const collapsed = collapseStaticLayoutIssues([...held, { ...blip, time: 9.0 }], 73); + + expect(collapsed).toHaveLength(1); + expect(collapsed[0]).toMatchObject({ severity: "error", occurrences: 7 }); + }); + + it("does not bridge two blips 1.5s apart, whatever grid the collapse was handed", () => { + const blip = { ...issue("content_overlap", "warning"), containerSelector: ".label" }; + for (const sampleCount of [9, 81]) { + const collapsed = collapseStaticLayoutIssues( + [ + { ...blip, time: 2.0 }, + { ...blip, time: 3.5 }, + ], + sampleCount, + ); + expect(collapsed[0]).toMatchObject({ severity: "warning" }); + } + }); + + it("promotes a 625ms contiguous collision even when a tight pair sits elsewhere", () => { + const blip = { ...issue("content_overlap", "warning"), containerSelector: ".label" }; + const held = [5.0, 5.125, 5.25, 5.375, 5.5, 5.625].map((time) => ({ ...blip, time })); + const elsewhere = { ...issue("content_overlap", "warning"), containerSelector: ".other" }; + const collapsed = collapseStaticLayoutIssues( + [ + ...held, + { ...elsewhere, time: 2.0 }, + { ...elsewhere, time: 2.05 }, + { ...elsewhere, time: 2.1 }, + ], + 81, + ); + + const label = collapsed.find((entry) => entry.containerSelector === ".label"); + expect(label).toMatchObject({ severity: "error", heldMs: 625 }); + }); + + it("still separates two distinct text_box_overflow findings that differ only by text", () => { + const collapsed = collapseStaticLayoutIssues( + [ + { ...issue("text_box_overflow", "warning"), time: 4.0, text: "first" }, + { ...issue("text_box_overflow", "warning"), time: 4.5, text: "second" }, + ], + 73, + ); + + expect(collapsed).toHaveLength(2); + }); + it("promotes content_overlap whose two occurrences span exactly 500ms (at the floor)", () => { const collapsed = collapseStaticLayoutIssues( [ diff --git a/packages/cli/src/utils/layoutAudit.ts b/packages/cli/src/utils/layoutAudit.ts index 5f68cb7007..b8f2985f88 100644 --- a/packages/cli/src/utils/layoutAudit.ts +++ b/packages/cli/src/utils/layoutAudit.ts @@ -49,6 +49,7 @@ export interface LayoutIssue { firstSeen?: number; lastSeen?: number; occurrences?: number; + heldMs?: number; selector: string; containerSelector?: string; text?: string; @@ -204,6 +205,13 @@ const PERSISTENCE_TIERED_CODES: ReadonlySet = new Set([ "connector_detached", ]); +const CONTIGUOUS_SAMPLE_GAP_MS = CONTENT_OVERLAP_HELD_ERROR_MS * 2; + +const TEXT_AGNOSTIC_KEY_CODES: ReadonlySet = new Set([ + "content_overlap", + "text_occluded", +]); + export function collapseStaticLayoutIssues( issues: LayoutIssue[], totalSampleCount?: number, @@ -215,6 +223,7 @@ export function collapseStaticLayoutIssues( firstSeen: number; lastSeen: number; occurrences: number; + times: number[]; } >(); @@ -227,6 +236,7 @@ export function collapseStaticLayoutIssues( firstSeen: issue.time, lastSeen: issue.time, occurrences: 1, + times: [issue.time], }); continue; } @@ -234,6 +244,7 @@ export function collapseStaticLayoutIssues( existing.firstSeen = Math.min(existing.firstSeen, issue.time); existing.lastSeen = Math.max(existing.lastSeen, issue.time); existing.occurrences += 1; + existing.times.push(issue.time); } // A run that only ever sampled one point in time can't distinguish a @@ -242,14 +253,39 @@ export function collapseStaticLayoutIssues( const sampleCount = totalSampleCount ?? new Set(issues.map((issue) => issue.time)).size; const multiSampleRun = sampleCount > 1; - return [...groups.values()].map(({ issue, firstSeen, lastSeen, occurrences }) => + return [...groups.values()].map(({ issue, firstSeen, lastSeen, occurrences, times }) => applyPersistenceTier( - { ...issue, time: firstSeen, firstSeen, lastSeen, occurrences }, + { + ...issue, + time: firstSeen, + firstSeen, + lastSeen, + occurrences, + heldMs: longestContiguousRunMs(times), + }, multiSampleRun, ), ); } +function longestContiguousRunMs(times: number[]): number { + const sorted = [...new Set(times)].sort((a, b) => a - b); + const first = sorted[0]; + const last = sorted.at(-1); + if (first === undefined || last === undefined) return 0; + let longest = 0; + let runStart = first; + let previous = first; + for (const current of sorted) { + if ((current - previous) * 1000 > CONTIGUOUS_SAMPLE_GAP_MS) { + longest = Math.max(longest, previous - runStart); + runStart = current; + } + previous = current; + } + return Math.max(longest, last - runStart) * 1000; +} + /** * Held-duration severity tiering (#U10). A finding observed at only one * sample among several (held 0ms) is an entrance/exit transient, not a held @@ -309,7 +345,7 @@ function isContentOverlapHeldLongEnough(issue: LayoutIssue, occurrences: number) if (occurrences < HELD_ACROSS_SAMPLES_MIN_OCCURRENCES) return false; const firstSeen = issue.firstSeen ?? issue.time; const lastSeen = issue.lastSeen ?? issue.time; - const heldMs = (lastSeen - firstSeen) * 1000; + const heldMs = issue.heldMs ?? (lastSeen - firstSeen) * 1000; return heldMs >= CONTENT_OVERLAP_HELD_ERROR_MS; } @@ -342,7 +378,7 @@ function staticIssueKey(issue: LayoutIssue): string { issue.severity, issue.selector, issue.containerSelector ?? "", - issue.text ?? "", + TEXT_AGNOSTIC_KEY_CODES.has(issue.code) ? "" : (issue.text ?? ""), issue.overflow ? formatOverflow(issue.overflow) : "", framePositionKey(issue), ].join("|");