Skip to content

test(rrweb-snapshot): lock in the #1734/#1692 css crash fixes with regression tests#1906

Open
yashrao2607 wants to merge 1 commit into
rrweb-io:mainfrom
yashrao2607:fix/1734-1692-css-parse-regressions
Open

test(rrweb-snapshot): lock in the #1734/#1692 css crash fixes with regression tests#1906
yashrao2607 wants to merge 1 commit into
rrweb-io:mainfrom
yashrao2607:fix/1734-1692-css-parse-regressions

Conversation

@yashrao2607

Copy link
Copy Markdown

Summary

Both #1734 and #1692 report the replayer crashing with an uncaught CssSyntaxError from postcss:

I went looking for the root cause and, checking against current main, both are already fixed by two earlier PRs:

Since I couldn't get the reporters' full page CSS (their repros are truncated CSSOM snippets from a much larger stylesheet), I can't reproduce their exact crash byte-for-byte, but I've reproduced the identical failure mode from each report (same postcss error type, same code path) and confirmed both are handled gracefully on main.

This PR doesn't change any behavior — it adds these cases as permanent regression tests (using the reporters' own snippets where possible) so the #1600/#1580 fixes can't silently regress, and so #1734 and #1692 have a paper trail for closing them.

Test plan

  • New tests in css.test.ts:
    • applyCssSplits css rejoiner > rejoins a split that lands inside a quoted attribute selector without corrupting it (#1692)
    • adaptCssForReplay with malformed css (#1734) > does not throw on empty longhand declarations produced by the browser CSSOM
    • adaptCssForReplay with malformed css (#1734) > falls back to the original text instead of throwing on genuinely unparseable css — this one reproduces the literal "Unclosed string" postcss error from [Bug]: Empty css rules break replay with "CssSyntaxError CssSyntaxError: Unclosed string" error #1734's stack trace
  • Full rrweb-snapshot suite passes: 93/93 (1 pre-existing skip).
  • tsc -noEmit clean.

Addresses #1734, #1692

… crashes

Both issues report an uncaught CssSyntaxError taking down the replayer:
rrweb-io#1734 from empty longhand declarations the browser's CSSOM produces for
shorthand properties (e.g. `border-top-style: ;`), rrweb-io#1692 from a
/* rr_split */ marker landing inside a quoted attribute selector.

Verified against main: both are already handled by the combination of
rrweb-io#1600 (splits are rejoined before being handed to postcss, so a split
never has to parse on its own) and rrweb-io#1580 (postcss failures are caught
and logged instead of thrown). Reproduced the exact "Unclosed string"
error from rrweb-io#1734's stack trace directly against adaptCssForReplay and
confirmed it degrades to a console.warn + original text instead of
propagating.

Adding these as permanent regressions, using the reporters' own
snippets where possible, so the fix in rrweb-io#1600/rrweb-io#1580 doesn't silently
regress and so rrweb-io#1734/rrweb-io#1692 can be closed with a paper trail.
@changeset-bot

changeset-bot Bot commented Jul 17, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 1439ada

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@yashrao2607

Copy link
Copy Markdown
Author

No changeset added here on purpose: this PR only adds regression tests (css.test.ts), it doesn't change any published package's runtime behavior, so it shouldn't trigger a version bump. See the PR description for why — both #1734 and #1692 turned out to already be fixed on main by #1600 and #1580; this just locks that in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant