fix: scrolling can be incorrect when fast-forwarding (#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>

.changeset/spotty-bees-destroy.md

@@ -0,0 +1,5 @@
+---
+'rrdom': patch
+---
+
+fix: scrolling may not be applied when fast-forwarding

packages/rrdom/src/diff.ts

@@ -118,7 +118,7 @@ export function diff(
 
   diffChildren(oldTree, newTree, replayer, rrnodeMirror);
 
-  diffAfterUpdatingChildren(oldTree, newTree, replayer, rrnodeMirror);
+  diffAfterUpdatingChildren(oldTree, newTree, replayer);
 }
 
 /**
@@ -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;
     }
   }
@@ -207,7 +216,6 @@ function diffAfterUpdatingChildren(
   oldTree: Node,
   newTree: IRRNode,
   replayer: ReplayerHandler,
-  rrnodeMirror: Mirror,
 ) {
   switch (newTree.RRNodeType) {
     case RRNodeType.Document: {
@@ -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);
       /**

packages/rrweb/test/events/scroll-with-parent-styles.ts

@@ -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;

packages/rrweb/test/replayer.test.ts

@@ -12,6 +12,7 @@ import {
 import styleSheetRuleEvents from './events/style-sheet-rule-events';
 import orderingEvents from './events/ordering';
 import scrollEvents from './events/scroll';
+import scrollWithParentStylesEvents from './events/scroll-with-parent-styles';
 import inputEvents from './events/input';
 import iframeEvents from './events/iframe';
 import selectionEvents from './events/selection';
@@ -411,6 +412,51 @@ describe('replayer', function () {
     ).toEqual(0);
   });
 
+  it('can fast forward scroll events w/ a parent node that affects a child nodes height', async () => {
+    await page.evaluate(`
+      events = ${JSON.stringify(scrollWithParentStylesEvents)};
+      const { Replayer } = rrweb;
+      var replayer = new Replayer(events,{showDebug:true});
+      replayer.pause(550);
+    `);
+    // add the ".container" element at 500
+    const iframe = await page.$('iframe');
+    const contentDocument = await iframe!.contentFrame()!;
+    expect(await contentDocument!.$('.container')).not.toBeNull();
+    expect(
+      await contentDocument!.$eval(
+        '.container',
+        (element: Element) => element.scrollTop,
+      ),
+    ).toEqual(0);
+
+    // restart the replayer
+    await page.evaluate('replayer.play(0);');
+    await waitForRAF(page);
+
+    await page.evaluate('replayer.pause(1050);');
+    // scroll the ".container" div' at 1000
+    expect(
+      await contentDocument!.$eval(
+        '.container',
+        (element: Element) => element.scrollTop,
+      ),
+    ).toEqual(800);
+
+    await page.evaluate('replayer.play(0);');
+    await waitForRAF(page);
+    await page.evaluate('replayer.pause(2050);');
+    // remove the ".container" element at 2000
+    expect(await contentDocument!.$('.container')).toBeNull();
+    expect(
+      await page.$eval(
+        'iframe',
+        (element: Element) =>
+          (element as HTMLIFrameElement)!.contentWindow!.scrollY,
+      ),
+    ).toEqual(0);
+  });
+
   it('can fast forward input events', async () => {
     await page.evaluate(`
       events = ${JSON.stringify(inputEvents)};