Skip to content

Commit e607e83

Browse files
fix: scrolling can be incorrect when fast-forwarding (rrweb-io#1352)
* 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>
1 parent cc6c687 commit e607e83

File tree

4 files changed

+387
-3
lines changed

4 files changed

+387
-3
lines changed

.changeset/spotty-bees-destroy.md

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'rrdom': patch
3+
---
4+
5+
fix: scrolling may not be applied when fast-forwarding

packages/rrdom/src/diff.ts

+10-3
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@ export function diff(
118118

119119
diffChildren(oldTree, newTree, replayer, rrnodeMirror);
120120

121-
diffAfterUpdatingChildren(oldTree, newTree, replayer, rrnodeMirror);
121+
diffAfterUpdatingChildren(oldTree, newTree, replayer);
122122
}
123123

124124
/**
@@ -194,6 +194,15 @@ function diffBeforeUpdatingChildren(
194194
rrnodeMirror,
195195
);
196196
}
197+
/**
198+
* Attributes and styles of the old element need to be updated before updating its children because of an edge case:
199+
* `applyScroll` may fail in `diffAfterUpdatingChildren` when the height of a node when `applyScroll` is called may be incorrect if
200+
* 1. its parent node contains styles that affects the targeted node's height
201+
* 2. the CSS selector is targeting an attribute of the parent node
202+
* by running `diffProps` on the parent node before `diffChildren` is called,
203+
* we can ensure that the correct attributes (and therefore styles) have applied to parent nodes
204+
*/
205+
diffProps(oldElement, newRRElement, rrnodeMirror);
197206
break;
198207
}
199208
}
@@ -207,7 +216,6 @@ function diffAfterUpdatingChildren(
207216
oldTree: Node,
208217
newTree: IRRNode,
209218
replayer: ReplayerHandler,
210-
rrnodeMirror: Mirror,
211219
) {
212220
switch (newTree.RRNodeType) {
213221
case RRNodeType.Document: {
@@ -218,7 +226,6 @@ function diffAfterUpdatingChildren(
218226
case RRNodeType.Element: {
219227
const oldElement = oldTree as HTMLElement;
220228
const newRRElement = newTree as RRElement;
221-
diffProps(oldElement, newRRElement, rrnodeMirror);
222229
newRRElement.scrollData &&
223230
replayer.applyScroll(newRRElement.scrollData, true);
224231
/**
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,326 @@
1+
import { EventType, IncrementalSource } from '@rrweb/types';
2+
import type { eventWithTime } from '@rrweb/types';
3+
4+
const now = Date.now();
5+
const events: eventWithTime[] = [
6+
{
7+
type: EventType.DomContentLoaded,
8+
data: {},
9+
timestamp: now,
10+
},
11+
{
12+
type: EventType.Load,
13+
data: {},
14+
timestamp: now + 100,
15+
},
16+
{
17+
type: EventType.Meta,
18+
data: {
19+
href: 'http://localhost',
20+
width: 1200,
21+
height: 500,
22+
},
23+
timestamp: now + 100,
24+
},
25+
// full snapshot:
26+
{
27+
type: EventType.FullSnapshot,
28+
data: {
29+
node: {
30+
id: 1,
31+
type: 0,
32+
childNodes: [
33+
{
34+
type: 1,
35+
name: 'html',
36+
publicId: '',
37+
systemId: '',
38+
id: 2,
39+
},
40+
{
41+
id: 3,
42+
type: 2,
43+
tagName: 'html',
44+
attributes: {
45+
lang: 'en',
46+
},
47+
childNodes: [
48+
{
49+
id: 4,
50+
type: 2,
51+
tagName: 'head',
52+
attributes: {},
53+
childNodes: [
54+
{
55+
id: 5,
56+
type: 2,
57+
tagName: 'style',
58+
attributes: {
59+
type: 'text/css',
60+
},
61+
childNodes: [
62+
{
63+
id: 6,
64+
type: 3,
65+
textContent:
66+
'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%; }',
67+
isStyle: true,
68+
},
69+
],
70+
},
71+
],
72+
},
73+
{
74+
id: 7,
75+
type: 2,
76+
tagName: 'body',
77+
attributes: {},
78+
childNodes: [
79+
{
80+
type: 2,
81+
tagName: 'div',
82+
attributes: {},
83+
childNodes: [
84+
{
85+
type: 2,
86+
tagName: 'ul',
87+
attributes: {},
88+
childNodes: [
89+
{
90+
type: 2,
91+
tagName: 'li',
92+
attributes: {},
93+
childNodes: [
94+
{
95+
type: 2,
96+
tagName: 'a',
97+
attributes: {
98+
href: 'https://localhost/page1',
99+
},
100+
childNodes: [
101+
{
102+
type: 3,
103+
textContent: '\nGo to page 1\n',
104+
id: 12,
105+
},
106+
],
107+
id: 11,
108+
},
109+
],
110+
id: 10,
111+
},
112+
{
113+
type: 2,
114+
tagName: 'li',
115+
attributes: {},
116+
childNodes: [
117+
{
118+
type: 2,
119+
tagName: 'a',
120+
attributes: {
121+
href: 'https://localhost/page2',
122+
},
123+
childNodes: [
124+
{
125+
type: 3,
126+
textContent: '\nGo to page 2\n',
127+
id: 15,
128+
},
129+
],
130+
id: 14,
131+
},
132+
],
133+
id: 13,
134+
},
135+
],
136+
id: 9,
137+
},
138+
],
139+
id: 8,
140+
},
141+
],
142+
},
143+
],
144+
},
145+
],
146+
},
147+
initialOffset: {
148+
left: 0,
149+
top: 0,
150+
},
151+
},
152+
timestamp: now + 100,
153+
},
154+
// mutation that adds all of the new parent/child elements
155+
{
156+
type: EventType.IncrementalSnapshot,
157+
data: {
158+
source: IncrementalSource.Mutation,
159+
texts: [],
160+
attributes: [],
161+
removes: [],
162+
adds: [
163+
{
164+
parentId: 8,
165+
nextId: null,
166+
node: {
167+
type: 2,
168+
tagName: 'main',
169+
attributes: {
170+
'data-v-7231068e': '',
171+
},
172+
childNodes: [],
173+
id: 16,
174+
},
175+
},
176+
{
177+
parentId: 16,
178+
nextId: null,
179+
node: {
180+
type: 2,
181+
tagName: 'div',
182+
attributes: {
183+
'data-v-7231068e': '',
184+
class: 'container',
185+
},
186+
childNodes: [],
187+
id: 17,
188+
},
189+
},
190+
{
191+
parentId: 17,
192+
nextId: null,
193+
node: {
194+
type: 2,
195+
tagName: 'div',
196+
attributes: {
197+
'data-v-7231068e': '',
198+
class: 'card',
199+
},
200+
childNodes: [],
201+
id: 18,
202+
},
203+
},
204+
{
205+
parentId: 18,
206+
nextId: null,
207+
node: {
208+
type: 2,
209+
tagName: 'button',
210+
attributes: {
211+
'data-v-7231068e': '',
212+
},
213+
childNodes: [],
214+
id: 19,
215+
},
216+
},
217+
{
218+
parentId: 19,
219+
nextId: null,
220+
node: {
221+
type: 3,
222+
textContent: '1',
223+
id: 20,
224+
},
225+
},
226+
{
227+
parentId: 17,
228+
nextId: 18,
229+
node: {
230+
type: 2,
231+
tagName: 'div',
232+
attributes: {
233+
'data-v-7231068e': '',
234+
class: 'card',
235+
},
236+
childNodes: [],
237+
id: 21,
238+
},
239+
},
240+
{
241+
parentId: 21,
242+
nextId: null,
243+
node: {
244+
type: 2,
245+
tagName: 'button',
246+
attributes: {
247+
'data-v-7231068e': '',
248+
},
249+
childNodes: [],
250+
id: 22,
251+
},
252+
},
253+
{
254+
parentId: 22,
255+
nextId: null,
256+
node: {
257+
type: 3,
258+
textContent: '2',
259+
id: 23,
260+
},
261+
},
262+
{
263+
parentId: 17,
264+
nextId: 21,
265+
node: {
266+
type: 2,
267+
tagName: 'div',
268+
attributes: {
269+
'data-v-7231068e': '',
270+
class: 'card',
271+
},
272+
childNodes: [],
273+
id: 24,
274+
},
275+
},
276+
{
277+
parentId: 24,
278+
nextId: null,
279+
node: {
280+
type: 2,
281+
tagName: 'button',
282+
attributes: {
283+
'data-v-7231068e': '',
284+
},
285+
childNodes: [],
286+
id: 25,
287+
},
288+
},
289+
{
290+
parentId: 25,
291+
nextId: null,
292+
node: {
293+
type: 3,
294+
textContent: '3',
295+
id: 26,
296+
},
297+
},
298+
],
299+
},
300+
timestamp: now + 500,
301+
},
302+
// scroll event on the '.container' div
303+
{
304+
type: EventType.IncrementalSnapshot,
305+
data: {
306+
source: IncrementalSource.Scroll,
307+
id: 17,
308+
x: 0,
309+
y: 800,
310+
},
311+
timestamp: now + 1000,
312+
},
313+
{
314+
type: EventType.IncrementalSnapshot,
315+
data: {
316+
source: IncrementalSource.Mutation,
317+
texts: [],
318+
attributes: [],
319+
removes: [{ parentId: 16, id: 17 }],
320+
adds: [],
321+
},
322+
timestamp: now + 2000,
323+
},
324+
];
325+
326+
export default events;

0 commit comments

Comments
 (0)