From 7b4223ceec96696595aea3cabb81d8f07521bedb Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Wed, 3 Aug 2022 16:24:59 +0200 Subject: [PATCH 1/5] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20round=20scroll=20value?= =?UTF-8?q?s=20everywhere?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/rum/src/domain/record/observer.ts | 4 ++-- packages/rum/src/domain/record/viewports.ts | 24 ++++++++++++--------- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/packages/rum/src/domain/record/observer.ts b/packages/rum/src/domain/record/observer.ts index 35484e0f50..423406b681 100644 --- a/packages/rum/src/domain/record/observer.ts +++ b/packages/rum/src/domain/record/observer.ts @@ -229,8 +229,8 @@ function initScrollObserver(cb: ScrollCallback, defaultPrivacyLevel: DefaultPriv } else { cb({ id, - x: (target as HTMLElement).scrollLeft, - y: (target as HTMLElement).scrollTop, + x: Math.round((target as HTMLElement).scrollLeft), + y: Math.round((target as HTMLElement).scrollTop), }) } }), diff --git a/packages/rum/src/domain/record/viewports.ts b/packages/rum/src/domain/record/viewports.ts index 4feb00b736..d016e77961 100644 --- a/packages/rum/src/domain/record/viewports.ts +++ b/packages/rum/src/domain/record/viewports.ts @@ -72,23 +72,27 @@ export const getVisualViewport = (): VisualViewportRecord['data'] => { } export function getScrollX() { + let scrollX const visual = window.visualViewport if (visual) { - return visual.pageLeft - visual.offsetLeft - } - if (window.scrollX !== undefined) { - return window.scrollX + scrollX = visual.pageLeft - visual.offsetLeft + } else if (window.scrollX !== undefined) { + scrollX = window.scrollX + } else { + scrollX = window.pageXOffset || 0 } - return window.pageXOffset || 0 + return Math.round(scrollX) } export function getScrollY() { + let scrollY const visual = window.visualViewport if (visual) { - return visual.pageTop - visual.offsetTop - } - if (window.scrollY !== undefined) { - return window.scrollY + scrollY = visual.pageTop - visual.offsetTop + } else if (window.scrollY !== undefined) { + scrollY = window.scrollY + } else { + scrollY = window.pageYOffset || 0 } - return window.pageYOffset || 0 + return Math.round(scrollY) } From b77e10ba9f0320a734ca88415e8ee908331215dc Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Wed, 3 Aug 2022 16:29:04 +0200 Subject: [PATCH 2/5] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20rename=20observer=20to?= =?UTF-8?q?=20observers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/rum/src/domain/record/mutationObserver.spec.ts | 2 +- packages/rum/src/domain/record/mutationObserver.ts | 2 +- .../src/domain/record/{observer.spec.ts => observers.spec.ts} | 4 ++-- packages/rum/src/domain/record/{observer.ts => observers.ts} | 0 packages/rum/src/domain/record/record.ts | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) rename packages/rum/src/domain/record/{observer.spec.ts => observers.spec.ts} (99%) rename packages/rum/src/domain/record/{observer.ts => observers.ts} (100%) diff --git a/packages/rum/src/domain/record/mutationObserver.spec.ts b/packages/rum/src/domain/record/mutationObserver.spec.ts index f7cb80d00c..fa77150e43 100644 --- a/packages/rum/src/domain/record/mutationObserver.spec.ts +++ b/packages/rum/src/domain/record/mutationObserver.spec.ts @@ -11,7 +11,7 @@ import type { AttributeMutation, Attributes } from '../../types' import { NodeType } from '../../types' import { serializeDocument } from './serialize' import { sortAddedAndMovedNodes, startMutationObserver, MutationController } from './mutationObserver' -import type { MutationCallBack } from './observer' +import type { MutationCallBack } from './observers' describe('startMutationCollection', () => { let sandbox: HTMLElement diff --git a/packages/rum/src/domain/record/mutationObserver.ts b/packages/rum/src/domain/record/mutationObserver.ts index d3ff3acda3..2f95f5bbf1 100644 --- a/packages/rum/src/domain/record/mutationObserver.ts +++ b/packages/rum/src/domain/record/mutationObserver.ts @@ -14,7 +14,7 @@ import { import { serializeNodeWithId, serializeAttribute } from './serialize' import { forEach } from './utils' import { createMutationBatch } from './mutationBatch' -import type { MutationCallBack } from './observer' +import type { MutationCallBack } from './observers' type WithSerializedTarget = T & { target: NodeWithSerializedNode } diff --git a/packages/rum/src/domain/record/observer.spec.ts b/packages/rum/src/domain/record/observers.spec.ts similarity index 99% rename from packages/rum/src/domain/record/observer.spec.ts rename to packages/rum/src/domain/record/observers.spec.ts index 2d4f421000..a5b9fec3a7 100644 --- a/packages/rum/src/domain/record/observer.spec.ts +++ b/packages/rum/src/domain/record/observers.spec.ts @@ -5,8 +5,8 @@ import type { RawRumEventCollectedData } from 'packages/rum-core/src/domain/life import { createNewEvent } from '../../../../core/test/specHelper' import { NodePrivacyLevel, PRIVACY_ATTR_NAME, PRIVACY_ATTR_VALUE_MASK_USER_INPUT } from '../../constants' import { RecordType } from '../../types' -import type { FrustrationCallback, InputCallback } from './observer' -import { initFrustrationObserver, initInputObserver } from './observer' +import type { FrustrationCallback, InputCallback } from './observers' +import { initFrustrationObserver, initInputObserver } from './observers' import { serializeDocument } from './serialize' describe('initInputObserver', () => { diff --git a/packages/rum/src/domain/record/observer.ts b/packages/rum/src/domain/record/observers.ts similarity index 100% rename from packages/rum/src/domain/record/observer.ts rename to packages/rum/src/domain/record/observers.ts diff --git a/packages/rum/src/domain/record/record.ts b/packages/rum/src/domain/record/record.ts index 8cd290c9cb..ef7730135c 100644 --- a/packages/rum/src/domain/record/record.ts +++ b/packages/rum/src/domain/record/record.ts @@ -14,7 +14,7 @@ import type { } from '../../types' import { RecordType, IncrementalSource } from '../../types' import { serializeDocument } from './serialize' -import { initObservers } from './observer' +import { initObservers } from './observers' import { MutationController } from './mutationObserver' import { getVisualViewport, getScrollX, getScrollY } from './viewports' From f81ed76178177e1e66d2c38e9078d444858e25af Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Wed, 3 Aug 2022 17:16:37 +0200 Subject: [PATCH 3/5] =?UTF-8?q?=E2=AC=86=EF=B8=8F=EF=B8=8F=20bump=20safari?= =?UTF-8?q?=20to=20have=20consistent=20behavior=20across=20browsers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit on safari 12, scroll attributes do not seem to be on html node... --- .../view/trackViewMetrics.spec.ts | 15 ++++++++++++--- test/browsers.conf.js | 4 ++-- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts index 996818b80e..668cdabdc6 100644 --- a/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts +++ b/packages/rum-core/src/domain/rumEventsCollection/view/trackViewMetrics.spec.ts @@ -336,6 +336,8 @@ describe('rum track view metrics', () => { describe('cumulativeLayoutShift', () => { let isLayoutShiftSupported: boolean + let originalSupportedEntryTypes: PropertyDescriptor | undefined + function newLayoutShift(lifeCycle: LifeCycle, { value = 0.1, hadRecentInput = false }) { lifeCycle.notify(LifeCycleEventType.PERFORMANCE_ENTRIES_COLLECTED, [ { @@ -351,10 +353,17 @@ describe('rum track view metrics', () => { if (!('PerformanceObserver' in window) || !('supportedEntryTypes' in PerformanceObserver)) { pending('No PerformanceObserver support') } + originalSupportedEntryTypes = Object.getOwnPropertyDescriptor(PerformanceObserver, 'supportedEntryTypes') isLayoutShiftSupported = true - spyOnProperty(PerformanceObserver, 'supportedEntryTypes', 'get').and.callFake(() => - isLayoutShiftSupported ? ['layout-shift'] : [] - ) + Object.defineProperty(PerformanceObserver, 'supportedEntryTypes', { + get: () => (isLayoutShiftSupported ? ['layout-shift'] : []), + }) + }) + + afterEach(() => { + if (originalSupportedEntryTypes) { + Object.defineProperty(PerformanceObserver, 'supportedEntryTypes', originalSupportedEntryTypes) + } }) it('should be initialized to 0', () => { diff --git a/test/browsers.conf.js b/test/browsers.conf.js index 6fcc6c9fbd..a6d4783d73 100644 --- a/test/browsers.conf.js +++ b/test/browsers.conf.js @@ -18,9 +18,9 @@ module.exports = [ { sessionName: 'Safari desktop', name: 'Safari', - version: '12.0', + version: '13.1', os: 'OS X', - osVersion: 'Mojave', + osVersion: 'Catalina', }, { sessionName: 'Chrome mobile', From b4106cff57344d1469336cfd72c96a710245e59f Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Tue, 2 Aug 2022 16:00:43 +0200 Subject: [PATCH 4/5] =?UTF-8?q?=E2=9C=85=20add=20scroll=20positions=20scen?= =?UTF-8?q?ario?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../scenario/recorder/recorder.scenario.ts | 105 +++++++++++++++++- 1 file changed, 103 insertions(+), 2 deletions(-) diff --git a/test/e2e/scenario/recorder/recorder.scenario.ts b/test/e2e/scenario/recorder/recorder.scenario.ts index 0fdae3ba91..f73763bac9 100644 --- a/test/e2e/scenario/recorder/recorder.scenario.ts +++ b/test/e2e/scenario/recorder/recorder.scenario.ts @@ -1,4 +1,10 @@ -import type { InputData, StyleSheetRuleData, CreationReason, BrowserSegment } from '@datadog/browser-rum/src/types' +import type { + InputData, + StyleSheetRuleData, + CreationReason, + BrowserSegment, + ScrollData, +} from '@datadog/browser-rum/src/types' import { NodeType, IncrementalSource, RecordType, MouseInteractionType } from '@datadog/browser-rum/src/types' import type { RumInitConfiguration } from '@datadog/browser-rum-core' @@ -16,11 +22,12 @@ import { createMutationPayloadValidatorFromSegment, findAllFrustrationRecords, findMouseInteractionRecords, + findElementWithTagName, } from '@datadog/browser-rum/test/utils' import { renewSession } from '../../lib/helpers/session' import type { EventRegistry } from '../../lib/framework' import { flushEvents, createTest, bundleSetup, html } from '../../lib/framework' -import { browserExecute } from '../../lib/helpers/browser' +import { browserExecute, browserExecuteAsync } from '../../lib/helpers/browser' const INTEGER_RE = /^\d+$/ const TIMESTAMP_RE = /^\d{13}$/ @@ -721,6 +728,100 @@ describe('recorder', () => { }) }) }) + + describe('scroll positions', () => { + createTest('should be recorded across navigation') + .withRum() + .withSetup(bundleSetup) + .withBody( + html` + +
+
I'm bigger than the container
+
+
+ ` + ) + .run(async ({ serverEvents }) => { + async function scroll({ windowY, containerX }: { windowY: number; containerX: number }) { + return browserExecuteAsync( + (windowY, containerX, done) => { + let scrollCount = 0 + + document.addEventListener( + 'scroll', + () => { + scrollCount++ + if (scrollCount === 2) { + // ensure to bypass observer throttling + setTimeout(done, 100) + } + }, + { capture: true, passive: true } + ) + + window.scrollTo(0, windowY) + document.getElementById('container')!.scrollTo(containerX, 0) + }, + windowY, + containerX + ) + } + + // initial scroll positions + await scroll({ windowY: 100, containerX: 10 }) + + await browserExecute(() => { + window.DD_RUM!.startSessionReplayRecording() + }) + + // wait for recorder to be properly started + await browser.pause(200) + + // update scroll positions + await scroll({ windowY: 150, containerX: 20 }) + + // trigger new full snapshot + await browserExecute(() => { + window.DD_RUM!.startView() + }) + + await flushEvents() + + expect(serverEvents.sessionReplay.length).toBe(2) + const firstSegment = getFirstSegment(serverEvents) + + const firstFullSnapshot = findFullSnapshot(firstSegment)! + let htmlElement = findElementWithTagName(firstFullSnapshot.data.node, 'html')! + expect(htmlElement.attributes.rr_scrollTop).toBe(100) + let containerElement = findElementWithIdAttribute(firstFullSnapshot.data.node, 'container')! + expect(containerElement.attributes.rr_scrollLeft).toBe(10) + + const scrollRecords = findAllIncrementalSnapshots(firstSegment, IncrementalSource.Scroll) + expect(scrollRecords.length).toBe(2) + const [windowScrollData, containerScrollData] = scrollRecords.map((record) => record.data as ScrollData) + expect(windowScrollData.y).toEqual(150) + expect(containerScrollData.x).toEqual(20) + + const secondFullSnapshot = findFullSnapshot(getLastSegment(serverEvents))! + htmlElement = findElementWithTagName(secondFullSnapshot.data.node, 'html')! + expect(htmlElement.attributes.rr_scrollTop).toBe(150) + containerElement = findElementWithIdAttribute(secondFullSnapshot.data.node, 'container')! + expect(containerElement.attributes.rr_scrollLeft).toBe(20) + }) + }) }) function getFirstSegment(events: EventRegistry) { From 3f701c4e7639ab703aa73d30319554001272edc2 Mon Sep 17 00:00:00 2001 From: Bastien Caudan Date: Wed, 3 Aug 2022 16:33:52 +0200 Subject: [PATCH 5/5] =?UTF-8?q?=E2=9A=A1=20serialize=20scroll=20positions?= =?UTF-8?q?=20only=20for=20full=20snapshots?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../rum/src/domain/record/mutationObserver.ts | 1 + .../rum/src/domain/record/serialize.spec.ts | 51 +++++++++++++------ packages/rum/src/domain/record/serialize.ts | 23 ++++++--- packages/rum/test/htmlAst.ts | 1 + 4 files changed, 53 insertions(+), 23 deletions(-) diff --git a/packages/rum/src/domain/record/mutationObserver.ts b/packages/rum/src/domain/record/mutationObserver.ts index 2f95f5bbf1..987059be39 100644 --- a/packages/rum/src/domain/record/mutationObserver.ts +++ b/packages/rum/src/domain/record/mutationObserver.ts @@ -211,6 +211,7 @@ function processChildListMutations( document, serializedNodeIds, parentNodePrivacyLevel, + serializationContext: 'mutation', }) if (!serializedNode) { continue diff --git a/packages/rum/src/domain/record/serialize.spec.ts b/packages/rum/src/domain/record/serialize.spec.ts index 1c230c19cd..c5f80554ce 100644 --- a/packages/rum/src/domain/record/serialize.spec.ts +++ b/packages/rum/src/domain/record/serialize.spec.ts @@ -18,7 +18,7 @@ import { import type { ElementNode, SerializedNodeWithId, TextNode } from '../../types' import { NodeType } from '../../types' import { hasSerializedNode } from './serializationUtils' -import type { SerializeOptions } from './serialize' +import type { SerializeOptions, SerializationContext } from './serialize' import { serializeDocument, serializeNodeWithId, @@ -31,6 +31,7 @@ import { MAX_ATTRIBUTE_VALUE_CHAR_LENGTH } from './privacy' const DEFAULT_OPTIONS: SerializeOptions = { document, parentNodePrivacyLevel: NodePrivacyLevel.ALLOW, + serializationContext: 'full-snapshot', } describe('serializeNodeWithId', () => { @@ -114,22 +115,41 @@ describe('serializeNodeWithId', () => { style: 'width: 10px;', }) }) - - it('serializes scroll position', () => { - const element = document.createElement('div') - Object.assign(element.style, { width: '100px', height: '100px', overflow: 'scroll' }) - const inner = document.createElement('div') - Object.assign(inner.style, { width: '200px', height: '200px' }) - element.appendChild(inner) - sandbox.appendChild(element) - element.scrollBy(10, 20) - - expect((serializeNodeWithId(element, DEFAULT_OPTIONS)! as ElementNode).attributes).toEqual( - jasmine.objectContaining({ + ;[ + { + description: 'serializes scroll position during full snapshot', + serializationContext: 'full-snapshot' as SerializationContext, + shouldSerializeScroll: true, + }, + { + description: 'does not serialize scroll position during mutation', + serializationContext: 'mutation' as SerializationContext, + shouldSerializeScroll: false, + }, + ].forEach(({ description, serializationContext, shouldSerializeScroll }) => { + it(description, () => { + const element = document.createElement('div') + Object.assign(element.style, { width: '100px', height: '100px', overflow: 'scroll' }) + const inner = document.createElement('div') + Object.assign(inner.style, { width: '200px', height: '200px' }) + element.appendChild(inner) + sandbox.appendChild(element) + element.scrollBy(10, 20) + + const serializedAttributes = ( + serializeNodeWithId(element, { ...DEFAULT_OPTIONS, serializationContext })! as ElementNode + ).attributes + const attributesWithScrollPositions = jasmine.objectContaining({ rr_scrollTop: 20, rr_scrollLeft: 10, }) - ) + + if (shouldSerializeScroll) { + expect(serializedAttributes).toEqual(attributesWithScrollPositions) + } else { + expect(serializedAttributes).not.toEqual(attributesWithScrollPositions) + } + }) }) it('ignores white space in ', () => { @@ -491,9 +511,10 @@ describe('serializeDocumentNode handles', function testAllowDomTree() { }) it('a masked DOM Document itself is still serialized ', () => { - const serializeOptionsMask = { + const serializeOptionsMask: SerializeOptions = { document, parentNodePrivacyLevel: NodePrivacyLevel.MASK, + serializationContext: 'full-snapshot', } expect(serializeDocumentNode(document, serializeOptionsMask)).toEqual({ type: NodeType.Document, diff --git a/packages/rum/src/domain/record/serialize.ts b/packages/rum/src/domain/record/serialize.ts index daa4caf257..587239988e 100644 --- a/packages/rum/src/domain/record/serialize.ts +++ b/packages/rum/src/domain/record/serialize.ts @@ -34,11 +34,14 @@ type ParentNodePrivacyLevel = | typeof NodePrivacyLevel.MASK | typeof NodePrivacyLevel.MASK_USER_INPUT +export type SerializationContext = 'full-snapshot' | 'mutation' + export interface SerializeOptions { document: Document serializedNodeIds?: Set ignoreWhiteSpace?: boolean parentNodePrivacyLevel: ParentNodePrivacyLevel + serializationContext: SerializationContext } export function serializeDocument( @@ -49,6 +52,7 @@ export function serializeDocument( return serializeNodeWithId(document, { document, parentNodePrivacyLevel: defaultPrivacyLevel, + serializationContext: 'full-snapshot', })! } @@ -145,7 +149,7 @@ export function serializeElementNode(element: Element, options: SerializeOptions return } - const attributes = getAttributesForPrivacyLevel(element, nodePrivacyLevel) + const attributes = getAttributesForPrivacyLevel(element, nodePrivacyLevel, options.serializationContext) let childNodes: SerializedNodeWithId[] = [] if (element.childNodes.length) { @@ -304,7 +308,8 @@ function isSVGElement(el: Element): boolean { function getAttributesForPrivacyLevel( element: Element, - nodePrivacyLevel: NodePrivacyLevel + nodePrivacyLevel: NodePrivacyLevel, + serializationContext: SerializationContext ): Record { if (nodePrivacyLevel === NodePrivacyLevel.HIDDEN) { return {} @@ -394,13 +399,15 @@ function getAttributesForPrivacyLevel( } /** - * Serialize the scroll state for each element + * Serialize the scroll state for each element only for full snapshot */ - if (element.scrollLeft) { - safeAttrs.rr_scrollLeft = Math.round(element.scrollLeft) - } - if (element.scrollTop) { - safeAttrs.rr_scrollTop = Math.round(element.scrollTop) + if (serializationContext === 'full-snapshot') { + if (element.scrollLeft) { + safeAttrs.rr_scrollLeft = Math.round(element.scrollLeft) + } + if (element.scrollTop) { + safeAttrs.rr_scrollTop = Math.round(element.scrollTop) + } } return safeAttrs diff --git a/packages/rum/test/htmlAst.ts b/packages/rum/test/htmlAst.ts index d3ca55a65d..5bc7bdf941 100644 --- a/packages/rum/test/htmlAst.ts +++ b/packages/rum/test/htmlAst.ts @@ -32,6 +32,7 @@ export const generateLeanSerializedDoc = (htmlContent: string, privacyTag: strin serializeNodeWithId(newDoc, { document: newDoc, parentNodePrivacyLevel: NodePrivacyLevel.ALLOW, + serializationContext: 'full-snapshot', })! as unknown as Record ) as unknown as SerializedNodeWithId return serializedDoc