Skip to content

Commit ef584a8

Browse files
Dylan Bradshawclaude
Dylan Bradshaw
and
claude
committed
fix(css): emit the base reset for tags evicted from the defaultStyle cache
generateDedupedBaseCSS built the per-tag base reset by reading cache.defaultStyle directly. That cache is an EvictingMap capped at MAX_DEFAULT_STYLE (30), and the reset is generated at the very end of a capture — so any document using more than 30 distinct tags had its earliest-registered tags already evicted. The lookup returned undefined and `continue` silently emitted no reset rule for them. The base reset is what neutralizes the UA stylesheet inside the foreignObject. Without it those tags fall back to UA defaults, so properties the page had reset to the CSS initial value (and which are therefore diffed out of the element's generated class) reappear: h1..h3/p margins, hr borders, list padding. The capture then reflows taller than the source, shifting every later section down. Which tags are affected depends on registration order, so the corruption is silent and varies from page to page. Observed on a real site: 39 distinct tags, and the tags at first-appearance ranks 15-23 (picture, main, h1, h2, h3, strong, p, hr) all lost their reset while h4 at rank 25 survived. Four <h3>s regained the UA 1em margin-block-start, growing the article by 121px and pushing a button 171px down over the section below it. Resolve through getDefaultStyleForTag instead: it is memoized, idempotent, and re-derives an evicted entry on demand. NO_DEFAULTS_TAGS still yields {} and is dropped by the existing empty-key guard, so SVG/head tags are unaffected. Tests cover both the end-to-end path (a >30-tag capture keeps h3 in the reset) and the unit invariant (every used tag with defaults appears in the output, even from a cleared cache). Both fail without the fix. Note for reviewers: the fillers in the end-to-end test must have distinct UA styling. Tags that compute identically (aside/section/nav/header/…) collide on the style-key memo in styles.js, never call getDefaultStyleForTag, and so never occupy a cache slot — with those the capture registers only 28 tags, stays under the cap and the bug does not trigger. Closes #468 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 3d95744 commit ef584a8

2 files changed

Lines changed: 87 additions & 1 deletion

File tree

‎__tests__/utils.css.base-reset-eviction.test.js‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import { describe, it, expect, afterEach } from 'vitest'
2+
import { snapdom } from '../src/index'
3+
import { cache } from '../src/core/cache.js'
4+
import { generateDedupedBaseCSS, getDefaultStyleForTag } from '../src/utils/css.js'
5+
6+
/**
7+
* The per-tag base reset neutralizes UA defaults inside the foreignObject. It was built by
8+
* reading cache.defaultStyle — an EvictingMap — directly, so a document using more distinct
9+
* tags than MAX_DEFAULT_STYLE had its earliest-registered tags evicted before the reset was
10+
* generated. Those tags got no reset rule and the UA stylesheet applied instead, reflowing
11+
* the capture taller than the source.
12+
*/
13+
14+
/** Tag names appearing in the selector of every non-class rule of the generated CSS. */
15+
function resetTags(css) {
16+
const tags = new Set()
17+
for (const m of css.matchAll(/(?:^|\})\s*([a-z0-9][a-z0-9, ]*?)\s*\{/gi)) {
18+
for (const t of m[1].split(',')) tags.add(t.trim())
19+
}
20+
return tags
21+
}
22+
23+
function svgCss(url) {
24+
const markup = decodeURIComponent(url)
25+
const doc = new DOMParser().parseFromString(markup.slice(markup.indexOf('<')), 'image/svg+xml')
26+
return [...doc.querySelectorAll('style')].map((s) => s.textContent).join('\n')
27+
}
28+
29+
// Comfortably more distinct tags than MAX_DEFAULT_STYLE (30). These must each carry *distinct*
30+
// UA styling: tags that compute identically (aside/section/nav/header/… are all plain blocks)
31+
// collide on the style-key memo, never call getDefaultStyleForTag, and so never occupy a slot.
32+
const MANY_TAGS = [
33+
'h1', 'h2', 'h4', 'h5', 'h6', 'p', 'pre', 'code', 'em', 'strong', 'small', 'sub', 'sup',
34+
'mark', 'del', 'ins', 'a', 'hr', 'blockquote', 'ul', 'ol', 'li', 'dl', 'dt', 'dd',
35+
'table', 'thead', 'tbody', 'tr', 'td', 'th', 'form', 'label', 'input', 'button', 'select',
36+
'textarea', 'fieldset', 'legend', 'img', 'span', 'b', 'i', 'q', 'cite', 'abbr',
37+
]
38+
39+
describe('generateDedupedBaseCSS — tags evicted from the defaultStyle cache', () => {
40+
let root, sheet
41+
afterEach(() => { root?.remove(); sheet?.remove() })
42+
43+
it('emits a base reset for an early tag even when the document exceeds the cache cap', async () => {
44+
sheet = document.createElement('style')
45+
// Reset the heading margin to the CSS initial value, so the property is diffed away and
46+
// the element depends entirely on the base reset to neutralize the UA default.
47+
sheet.textContent = '#sd-cap h3{margin:0;font-size:16px;font-weight:400}'
48+
document.head.appendChild(sheet)
49+
50+
root = document.createElement('div')
51+
root.id = 'sd-cap'
52+
// h3 first, so it is among the earliest tags registered and thus the first evicted.
53+
// Each filler needs real content: an empty, zero-sized element can be culled before the
54+
// style pass and would then never consume a defaultStyle slot.
55+
root.innerHTML = '<h3>Heading</h3>' + MANY_TAGS.map((t) => `<${t}>x</${t}>`).join('')
56+
document.body.appendChild(root)
57+
58+
const distinct = new Set([...root.querySelectorAll('*')].map((e) => e.tagName.toLowerCase()))
59+
expect(distinct.size).toBeGreaterThan(30)
60+
61+
// Start cold, as on a real page load: h3 is registered first and then evicted by the
62+
// tags that follow it. A cache warmed by earlier tests would already hold h3 and hide it.
63+
cache.defaultStyle.clear()
64+
65+
const css = svgCss((await snapdom(root)).url)
66+
expect(resetTags(css)).toContain('h3')
67+
})
68+
69+
it('covers every used tag that has defaults, not just the ones still cached', async () => {
70+
const used = ['h1', 'h2', 'h3', 'p', 'hr', 'strong', 'main', ...MANY_TAGS]
71+
// Simulate the end-of-capture state: the cache has been churned past its cap.
72+
cache.defaultStyle.clear()
73+
const css = generateDedupedBaseCSS([...used].sort())
74+
const emitted = resetTags(css)
75+
const expected = used.filter((t) => Object.keys(getDefaultStyleForTag(t)).length > 0)
76+
for (const tag of expected) expect(emitted).toContain(tag)
77+
})
78+
})

‎src/utils/css.js‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,15 @@ export function generateDedupedBaseCSS(usedTagNames) {
277277
const groups = new Map()
278278

279279
for (let tagName of usedTagNames) {
280-
const styles = cache.defaultStyle.get(tagName)
280+
// Resolve through getDefaultStyleForTag instead of reading cache.defaultStyle directly.
281+
// That cache is an EvictingMap (MAX_DEFAULT_STYLE), and this runs at the very end of a
282+
// capture: a document using more distinct tags than the cap has already had its earliest
283+
// tags evicted. Reading the map raw returned undefined for exactly those tags and the
284+
// `continue` silently emitted no base reset for them, so inside the foreignObject the UA
285+
// stylesheet's defaults applied instead (h1..h3/p margins, hr borders, list padding…) and
286+
// the capture reflowed taller than the source. Re-deriving is memoized and idempotent;
287+
// NO_DEFAULTS_TAGS still yields {} and is dropped by the empty-key guard below.
288+
const styles = getDefaultStyleForTag(tagName)
281289
if (!styles) continue
282290

283291
// Creamos la "firma" del bloque CSS para comparar

0 commit comments

Comments
 (0)