fix(rrweb-snapshot): omit srcdoc attribute when rebuilding iframe elements - #1905
Open
yashrao2607 wants to merge 2 commits into
Open
fix(rrweb-snapshot): omit srcdoc attribute when rebuilding iframe elements#1905yashrao2607 wants to merge 2 commits into
yashrao2607 wants to merge 2 commits into
Conversation
Setting `srcdoc` on a live iframe makes the browser asynchronously parse and load its own document into the iframe's contentDocument, racing against rrweb's own reconstruction of that iframe's document (built separately from recorded child nodes/mutations). The race can desync the mirror from the live DOM, so a later mutation ends up targeting a node that no longer exists, throwing e.g. "Failed to execute 'insertBefore' on 'Node': parameter 1 is not of type 'Node'" and taking down the replayer. rrdom's diff-based renderer already special-cases this (see packages/rrdom/src/diff.ts and its CHANGELOG), but the plain browser-DOM rebuild path in rrweb-snapshot did not. Fixes rrweb-io#1736
🦋 Changeset detectedLatest commit: fc59324 The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Co-Authored-By: Claude Sonnet 5 <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
1 participant
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.
Summary
When rebuilding a serialized
<iframe>that has asrcdocattribute,buildNodewas settingsrcdocon the live element vianode.setAttribute(name, value.toString())like any other attribute. Settingsrcdocmakes the browser asynchronously parse and load its own document into the iframe'scontentDocument— but rrweb also separately reconstructs that iframe's document from its own recorded child nodes/mutations (attachDocumentToIframe/ the mirror). These two reconstructions race, and the browser's native srcdoc load can clobber (or be clobbered by) rrweb's own tree, desyncing the mirror from the live DOM. A later mutation then targets a node that no longer exists, throwing e.g.:...which crashes the replayer (per the report, wrapping it in try/catch just breaks the player instead).
rrdom's diff-based renderer already special-cases this exact issue (seepackages/rrdom/src/diff.ts:354and its CHANGELOG: "Omit the 'srcdoc' attribute of iframes to avoid overwriting content"), but the plain browser-DOM rebuild path inrrweb-snapshot(used by the default web replayer) never got the same treatment. This PR applies the same fix there: skip settingsrcdoc, since rrweb's own reconstruction is the source of truth for the iframe's contents.Test plan
rebuild.test.tsthat builds an<iframe srcdoc="...">node viabuildNodeWithSNand asserts the resulting live element has nosrcdocattribute. Verified it fails without the fix and passes with it.rrweb-snapshottest suite (rebuild.test.ts,css.test.ts,snapshot.test.ts) passes: 72/72.tsc -noEmitclean.Fixes #1736