fix: scrolling can be incorrect when fast-forwarding

[juliecheng]

👋 hello! i'm seeing an issue where the scrolling position of the dom can end up being incorrect after skipping when using the virtual dom. here is an example replay to visualize the issue:

https://rrwebdebug.com/play/index.html?url=https%3A%2F%2Fgist.githubusercontent.com%2Fjuliecheng%2F6f264bae8fa3bcfc56b3011d4bf3b737%2Fraw%2F84f6ddc329abb88b6b8d26a76306b4f10a33028a%2Fbroken-scrolling.json&version=2.0.0-alpha.8&virtual-dom=on&play=on

if you let it play regularly, you'll see that about 2 seconds in I scroll down so the number 7 + "click" button is now visible for the remainder of the replay. if you skip all the way back to the beginning of the replay + pause immediately (so that you still see "Test Page"), then skip to some spot in the middle of the replay, the scrolling position is incorrect (it didn't scroll) compared to the regular playback.

if the node we're trying to call applyScroll on has its height affected by the styles of its parent node, and the parent node is missing attributes that prevent the CSS selector rules from matching at the time applyScroll is called, we run into an issue w/ scrolling. the node's clientHeight can equal scrollHeight w/o the parent styles which means scrollTo won't actually do anything. here is an example css:

parent[data-v-12345] { <--- isn't applied yet because it's missing the data attribute at the time applyScroll is called
  position: fixed;
  height: calc(100% - 0px);
}

targetElement[data-v-12345] {
  overflow: auto;
  height: 100%;
}

not sure if there is a more elegant solution in mind for this but my thought process was to run diffProps inside diffBeforeUpdatingChildren so that newly created nodes that are missing attributes will now have them added (and therefore styles applied). then when diffChildren runs and applyScroll is called on one of its child nodes all of the styling and height calculations should be correct

[changeset-bot]

🦋 Changeset detected

Latest commit: 7a7a0d6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 8 packages

Name	Type
rrdom	Patch
rrweb	Patch
rrweb-snapshot	Patch
rrdom-nodejs	Patch
rrweb-player	Patch
@rrweb/types	Patch
@rrweb/web-extension	Patch
rrvideo	Patch

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

[eoghanmurray]

Thank you! Do you think it would be possible to extract a minimal test case from the rrdebug data? We have can fast forward scroll events in rrweb/test/replayer.test.ts already, so a new test based on that but with your failing events would be perfect to exemplify the problem and ensure we don't later get a regression.

[juliecheng]

Thank you! Do you think it would be possible to extract a minimal test case from the rrdebug data? We have can fast forward scroll events in rrweb/test/replayer.test.ts already, so a new test based on that but with your failing events would be perfect to exemplify the problem and ensure we don't later get a regression.

thanks for taking a look! just added a test case and confirmed it passes w/ my changes (and fails without them) 👍

[YunFeng0817]

@juliecheng Thanks for finding out this bug. This is amazing!
I made one minor change on this pull request, can you please review it?
pendo-io#9

[juliecheng]

@YunFeng0817 ready for re-review when you get a chance please 🙏