Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try to produce a clean/sequential DOM walk for the initial snapshot #60

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

eoghanmurray
Copy link
Contributor

I'm detecting a lot of out-of-order-id full snapshots in the wild but find them very hard to recreate (out of order in the sense that ids don't follow a sequential pattern as you walk the snapshot depth first).

The only thing I can imagine is that it has something to do with DOM mutations happening during the snapshotting (so that an already snapshotted DOM node appears later in the walk under another branch of the DOM). The initial commit in this pull request has the effect of fixing the out of order IDs, but maybe it could introduce other problems, so it would be great to get someone else's input on this!

@Juice10
Copy link
Contributor

Juice10 commented May 31, 2021

@eoghanmurray I noticed that whenever rrweb records an iframe that is already loaded it skips a number of ids (which the incremental snapshot then uses to add the iframe's child elements). The fix I added #78 stops that from happening, as shown in the updated iframe 1 snapshot: rrweb-io/rrweb@5948865
Is this what you are referring to with regards to the out of order ids, or are you referring to something else?

@eoghanmurray
Copy link
Contributor Author

Don't think there were any iframes in play in the websites I was looking at (and it was prior to the support for nested contexts), just simple mutations.

See also #61 as a way to discuss the idea of a 'stable' DOM walk, i.e. one that disallows including the same node twice because it has been moved (mutated) from an already-visited spot to a new position.

@eoghanmurray
Copy link
Contributor Author

Just so it's clear, I now believe this approach can result in duplicate content (with different rrweb ids) in the snapshot — if a node is mutated and moved during the DOM walk from before the current walk position to after it.
#61 could be considered the successor to this, although it doesn't propose a solution yet!

…ons during snapshotting, we can continue afterwards with a freshly built DOM tree which has sequential ids according to the DOM walk (which are thus predictable and can potentially be stripped out during storage of full snapshot event)
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.

2 participants