fix(css): emit the base reset for tags evicted from the defaultStyle cache (#468) - #470
Merged
tinchox5 merged 1 commit intoJul 24, 2026
Conversation
…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 zumerlab#468
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #468.
Problem
generateDedupedBaseCSSbuilt the per-tag base reset by reading the cache directly:cache.defaultStyleis anEvictingMapcapped atMAX_DEFAULT_STYLE = 30, evicting FIFO. The reset is generated at the very end of a capture, so any document using more than 30 distinct tags has already evicted its earliest-registered tags. The lookup returnsundefinedandcontinuesilently emits no reset rule for exactly those tags.The base reset is what neutralizes the UA stylesheet inside the
foreignObject. Properties the page reset to the CSS initial value get diffed out of the element's generated class precisely because the base reset is expected to cover them — so when it's missing there is nothing left to hold the UA default off:h1..h3/pmargins,hrborders, list padding all come back, and the capture reflows taller than the source.Which tags are hit depends on registration order, so the corruption is silent and varies per 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 whileh4at rank 25 survived. Four<h3>s regained the UA1emmargin, growing the article by 121px and pushing a button 171px down over the section below it.Fix
Resolve through
getDefaultStyleForTag(tagName)instead of reading the map raw. It's memoized, idempotent, and re-derives an evicted entry on demand.NO_DEFAULTS_TAGSstill yields{}and is dropped by the existing empty-key guard, so SVG/head tags are unaffected.One line, plus a comment recording why the raw map read is wrong here.
Cost
Each evicted tag now re-derives once at end of capture — a sandbox element plus a computed-style read. It's a single bounded pass (each tag is visited once, so ~9 derivations on the 39-tag page above), negligible against a full capture.
Raising
MAX_DEFAULT_STYLEwould also make this particular page work, but it only moves the cap rather than removing the failure mode — the next document with more tags hits it again. Happy to go that route instead if you'd prefer.Tests
__tests__/utils.css.base-reset-eviction.test.js, 2 cases:h3Both fail on
main.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 instyles.js, never callgetDefaultStyleForTag, and so never occupy a cache slot — with those the capture registers only 28 tags, stays under the cap, and the bug doesn't trigger.Full suite: 691 passed / 1 skipped, against 689 / 1 on
main— no regressions.npm run lintandnpm run test:typesclean. Verified on Chromium; I don't have the Firefox/WebKit Playwright binaries locally.