⚡[RUMF-1344] Serialize scroll positions only for full snapshots (#1670)

* ♻️ round scroll values everywhere

* ♻️ rename observer to observers

* ⬆️️ bump safari to have consistent behavior across browsers

on safari 12, scroll attributes do not seem to be on html node...

* ✅ add scroll positions scenario

* ⚡ serialize scroll positions only for full snapshots

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', () => {

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

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> = T & { target: NodeWithSerializedNode }
 
@@ -211,6 +211,7 @@ function processChildListMutations(
       document,
       serializedNodeIds,
       parentNodePrivacyLevel,
+      serializationContext: 'mutation',
     })
     if (!serializedNode) {
       continue

packages/rum/src/domain/record/observer.spec.ts → 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', () => {

packages/rum/src/domain/record/observer.ts → packages/rum/src/domain/record/observers.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),
         })
       }
     }),

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'

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 <head>', () => {
@@ -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,

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<number>
   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<string, string | number | boolean> {
   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

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)
 }

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<string, unknown>
   ) as unknown as SerializedNodeWithId
   return serializedDoc

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',