From e3fb1ea4e578c203ce6d2f71746a2a0689878967 Mon Sep 17 00:00:00 2001 From: Julie Cheng Date: Thu, 9 Nov 2023 11:33:37 -0500 Subject: [PATCH 1/3] fix: scrolling can be incorrect when fast-forwarding --- packages/rrdom/src/diff.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/rrdom/src/diff.ts b/packages/rrdom/src/diff.ts index f37f298106..10ae193a3f 100644 --- a/packages/rrdom/src/diff.ts +++ b/packages/rrdom/src/diff.ts @@ -194,6 +194,15 @@ function diffBeforeUpdatingChildren( rrnodeMirror, ); } + /** + * Edge case where `applyScroll` may fail in `diffAfterUpdatingChildren`: + * 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; } } From c1a7581d3d03c0e2ab7af88f777160bac82603ae Mon Sep 17 00:00:00 2001 From: Julie Cheng Date: Fri, 17 Nov 2023 13:29:49 -0500 Subject: [PATCH 2/3] add test case --- .../test/events/scroll-with-parent-styles.ts | 326 ++++++++++++++++++ packages/rrweb/test/replayer.test.ts | 46 +++ 2 files changed, 372 insertions(+) create mode 100644 packages/rrweb/test/events/scroll-with-parent-styles.ts diff --git a/packages/rrweb/test/events/scroll-with-parent-styles.ts b/packages/rrweb/test/events/scroll-with-parent-styles.ts new file mode 100644 index 0000000000..da9f1c01ef --- /dev/null +++ b/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; diff --git a/packages/rrweb/test/replayer.test.ts b/packages/rrweb/test/replayer.test.ts index 183d2417fb..da322e8749 100644 --- a/packages/rrweb/test/replayer.test.ts +++ b/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'; @@ -410,6 +411,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)}; From 0647734957d28d8dd1ceaabb27d020db46612a53 Mon Sep 17 00:00:00 2001 From: Yun Feng Date: Mon, 12 Feb 2024 13:17:26 +0800 Subject: [PATCH 3/3] add changeset and remove duplicate diffProps process --- .changeset/spotty-bees-destroy.md | 5 +++++ packages/rrdom/src/diff.ts | 8 +++----- 2 files changed, 8 insertions(+), 5 deletions(-) create mode 100644 .changeset/spotty-bees-destroy.md diff --git a/.changeset/spotty-bees-destroy.md b/.changeset/spotty-bees-destroy.md new file mode 100644 index 0000000000..08e6f737a7 --- /dev/null +++ b/.changeset/spotty-bees-destroy.md @@ -0,0 +1,5 @@ +--- +'rrdom': patch +--- + +fix: scrolling may not be applied when fast-forwarding diff --git a/packages/rrdom/src/diff.ts b/packages/rrdom/src/diff.ts index 10ae193a3f..05dc0f2218 100644 --- a/packages/rrdom/src/diff.ts +++ b/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); } /** @@ -195,8 +195,8 @@ function diffBeforeUpdatingChildren( ); } /** - * Edge case where `applyScroll` may fail in `diffAfterUpdatingChildren`: - * the height of a node when `applyScroll` is called may be incorrect if + * 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, @@ -216,7 +216,6 @@ function diffAfterUpdatingChildren( oldTree: Node, newTree: IRRNode, replayer: ReplayerHandler, - rrnodeMirror: Mirror, ) { switch (newTree.RRNodeType) { case RRNodeType.Document: { @@ -227,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); /**