Skip to content

Commit

Permalink
fix: scrolling can be incorrect when fast-forwarding (#1352)
Browse files Browse the repository at this point in the history 
* fix: scrolling can be incorrect when fast-forwarding

* add test case

* add changeset and remove duplicate diffProps process

---------

Co-authored-by: Yun Feng <yun.feng0817@gmail.com>
  • Loading branch information
@juliecheng @YunFeng0817
juliecheng and YunFeng0817 committed Feb 27, 2024
1 parent cc6c687 commit e607e83
Show file tree
Hide file tree
Showing 4 changed files with 387 additions and 3 deletions.
5 changes: 5 additions & 0 deletions .changeset/spotty-bees-destroy.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'rrdom': patch
---

fix: scrolling may not be applied when fast-forwarding
13 changes: 10 additions & 3 deletions packages/rrdom/src/diff.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export function diff(

diffChildren(oldTree, newTree, replayer, rrnodeMirror);

diffAfterUpdatingChildren(oldTree, newTree, replayer, rrnodeMirror);
diffAfterUpdatingChildren(oldTree, newTree, replayer);
}

/**
Expand Down Expand Up @@ -194,6 +194,15 @@ function diffBeforeUpdatingChildren(
rrnodeMirror,
);
}
/**
* Attributes and styles of the old element need to be updated before updating its children because of an edge case:
* `applyScroll` may fail in `diffAfterUpdatingChildren` when the height of a node when `applyScroll` is called may be incorrect if
* 1. its parent node contains styles that affects the targeted node's height
* 2. the CSS selector is targeting an attribute of the parent node
* by running `diffProps` on the parent node before `diffChildren` is called,
* we can ensure that the correct attributes (and therefore styles) have applied to parent nodes
*/
diffProps(oldElement, newRRElement, rrnodeMirror);
break;
}
}
Expand All @@ -207,7 +216,6 @@ function diffAfterUpdatingChildren(
oldTree: Node,
newTree: IRRNode,
replayer: ReplayerHandler,
rrnodeMirror: Mirror,
) {
switch (newTree.RRNodeType) {
case RRNodeType.Document: {
Expand All @@ -218,7 +226,6 @@ function diffAfterUpdatingChildren(
case RRNodeType.Element: {
const oldElement = oldTree as HTMLElement;
const newRRElement = newTree as RRElement;
diffProps(oldElement, newRRElement, rrnodeMirror);
newRRElement.scrollData &&
replayer.applyScroll(newRRElement.scrollData, true);
/**
Expand Down
326 changes: 326 additions & 0 deletions packages/rrweb/test/events/scroll-with-parent-styles.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,326 @@
import { EventType, IncrementalSource } from '@rrweb/types';
import type { eventWithTime } from '@rrweb/types';

const now = Date.now();
const events: eventWithTime[] = [
{
type: EventType.DomContentLoaded,
data: {},
timestamp: now,
},
{
type: EventType.Load,
data: {},
timestamp: now + 100,
},
{
type: EventType.Meta,
data: {
href: 'http://localhost',
width: 1200,
height: 500,
},
timestamp: now + 100,
},
// full snapshot:
{
type: EventType.FullSnapshot,
data: {
node: {
id: 1,
type: 0,
childNodes: [
{
type: 1,
name: 'html',
publicId: '',
systemId: '',
id: 2,
},
{
id: 3,
type: 2,
tagName: 'html',
attributes: {
lang: 'en',
},
childNodes: [
{
id: 4,
type: 2,
tagName: 'head',
attributes: {},
childNodes: [
{
id: 5,
type: 2,
tagName: 'style',
attributes: {
type: 'text/css',
},
childNodes: [
{
id: 6,
type: 3,
textContent:
'main[data-v-7231068e] { position: fixed; top: 0px; right: 0px; height: calc(100% - 0px); overflow: auto; left: 0px; }.container[data-v-7231068e] { overflow: auto; overscroll-behavior-y: contain; position: relative; height: 100%; }.container .card[data-v-7231068e] { min-height: 170px; height: 100%; }',
isStyle: true,
},
],
},
],
},
{
id: 7,
type: 2,
tagName: 'body',
attributes: {},
childNodes: [
{
type: 2,
tagName: 'div',
attributes: {},
childNodes: [
{
type: 2,
tagName: 'ul',
attributes: {},
childNodes: [
{
type: 2,
tagName: 'li',
attributes: {},
childNodes: [
{
type: 2,
tagName: 'a',
attributes: {
href: 'https://localhost/page1',
},
childNodes: [
{
type: 3,
textContent: '\nGo to page 1\n',
id: 12,
},
],
id: 11,
},
],
id: 10,
},
{
type: 2,
tagName: 'li',
attributes: {},
childNodes: [
{
type: 2,
tagName: 'a',
attributes: {
href: 'https://localhost/page2',
},
childNodes: [
{
type: 3,
textContent: '\nGo to page 2\n',
id: 15,
},
],
id: 14,
},
],
id: 13,
},
],
id: 9,
},
],
id: 8,
},
],
},
],
},
],
},
initialOffset: {
left: 0,
top: 0,
},
},
timestamp: now + 100,
},
// mutation that adds all of the new parent/child elements
{
type: EventType.IncrementalSnapshot,
data: {
source: IncrementalSource.Mutation,
texts: [],
attributes: [],
removes: [],
adds: [
{
parentId: 8,
nextId: null,
node: {
type: 2,
tagName: 'main',
attributes: {
'data-v-7231068e': '',
},
childNodes: [],
id: 16,
},
},
{
parentId: 16,
nextId: null,
node: {
type: 2,
tagName: 'div',
attributes: {
'data-v-7231068e': '',
class: 'container',
},
childNodes: [],
id: 17,
},
},
{
parentId: 17,
nextId: null,
node: {
type: 2,
tagName: 'div',
attributes: {
'data-v-7231068e': '',
class: 'card',
},
childNodes: [],
id: 18,
},
},
{
parentId: 18,
nextId: null,
node: {
type: 2,
tagName: 'button',
attributes: {
'data-v-7231068e': '',
},
childNodes: [],
id: 19,
},
},
{
parentId: 19,
nextId: null,
node: {
type: 3,
textContent: '1',
id: 20,
},
},
{
parentId: 17,
nextId: 18,
node: {
type: 2,
tagName: 'div',
attributes: {
'data-v-7231068e': '',
class: 'card',
},
childNodes: [],
id: 21,
},
},
{
parentId: 21,
nextId: null,
node: {
type: 2,
tagName: 'button',
attributes: {
'data-v-7231068e': '',
},
childNodes: [],
id: 22,
},
},
{
parentId: 22,
nextId: null,
node: {
type: 3,
textContent: '2',
id: 23,
},
},
{
parentId: 17,
nextId: 21,
node: {
type: 2,
tagName: 'div',
attributes: {
'data-v-7231068e': '',
class: 'card',
},
childNodes: [],
id: 24,
},
},
{
parentId: 24,
nextId: null,
node: {
type: 2,
tagName: 'button',
attributes: {
'data-v-7231068e': '',
},
childNodes: [],
id: 25,
},
},
{
parentId: 25,
nextId: null,
node: {
type: 3,
textContent: '3',
id: 26,
},
},
],
},
timestamp: now + 500,
},
// scroll event on the '.container' div
{
type: EventType.IncrementalSnapshot,
data: {
source: IncrementalSource.Scroll,
id: 17,
x: 0,
y: 800,
},
timestamp: now + 1000,
},
{
type: EventType.IncrementalSnapshot,
data: {
source: IncrementalSource.Mutation,
texts: [],
attributes: [],
removes: [{ parentId: 16, id: 17 }],
adds: [],
},
timestamp: now + 2000,
},
];

export default events;

0 comments on commit e607e83

Please sign in to comment.