From c7072220b502d56e6c6df722c282f7c55dc1ba62 Mon Sep 17 00:00:00 2001 From: Alex Kaminskii Date: Mon, 12 Sep 2022 16:18:44 +0200 Subject: [PATCH] fix(tracker): iframe scrolls --- tracker/tracker/src/main/app/guards.ts | 5 ++ tracker/tracker/src/main/app/nodes.ts | 8 +-- .../src/main/app/observer/iframe_observer.ts | 1 + .../src/main/app/observer/top_observer.ts | 68 ++++++++++--------- tracker/tracker/src/main/modules/img.ts | 4 +- tracker/tracker/src/main/modules/scroll.ts | 40 +++++++---- tracker/tracker/src/webworker/BatchWriter.ts | 2 +- 7 files changed, 73 insertions(+), 55 deletions(-) diff --git a/tracker/tracker/src/main/app/guards.ts b/tracker/tracker/src/main/app/guards.ts index f88abc293..e4cfa8116 100644 --- a/tracker/tracker/src/main/app/guards.ts +++ b/tracker/tracker/src/main/app/guards.ts @@ -1,3 +1,8 @@ +//@ts-ignore +export function isNode(sth: any): sth is Node { + return !!sth && sth.nodeType != null +} + export function isSVGElement(node: Element): node is SVGElement { return node.namespaceURI === 'http://www.w3.org/2000/svg' } diff --git a/tracker/tracker/src/main/app/nodes.ts b/tracker/tracker/src/main/app/nodes.ts index b6c3e998a..785875636 100644 --- a/tracker/tracker/src/main/app/nodes.ts +++ b/tracker/tracker/src/main/app/nodes.ts @@ -12,20 +12,18 @@ export default class Nodes { attachNodeCallback(nodeCallback: NodeCallback): void { this.nodeCallbacks.push(nodeCallback) } - // TODO: what is the difference with app.attachEventListener. can we use only one of those? - attachElementListener(type: string, node: Element, elementListener: EventListener): void { + attachNodeListener(type: string, node: Node, listener: EventListener): void { const id = this.getID(node) if (id === undefined) { return } - node.addEventListener(type, elementListener) + node.addEventListener(type, listener) let listeners = this.elementListeners.get(id) if (listeners === undefined) { listeners = [] this.elementListeners.set(id, listeners) - return } - listeners.push([type, elementListener]) + listeners.push([type, listener]) } registerNode(node: Node): [/*id:*/ number, /*isNew:*/ boolean] { diff --git a/tracker/tracker/src/main/app/observer/iframe_observer.ts b/tracker/tracker/src/main/app/observer/iframe_observer.ts index b4dd78016..05df1fe54 100644 --- a/tracker/tracker/src/main/app/observer/iframe_observer.ts +++ b/tracker/tracker/src/main/app/observer/iframe_observer.ts @@ -10,6 +10,7 @@ export default class IFrameObserver extends Observer { } //log TODO common app.logger // Have to observe document, because the inner might be changed this.observeRoot(doc, (docID) => { + //MBTODO: do not send if empty (send on load? it might be in-place iframe, like our replayer, which does not get loaded) if (docID === undefined) { console.log('OpenReplay: Iframe document not bound') return diff --git a/tracker/tracker/src/main/app/observer/top_observer.ts b/tracker/tracker/src/main/app/observer/top_observer.ts index 5e4899692..d9f2d7f9d 100644 --- a/tracker/tracker/src/main/app/observer/top_observer.ts +++ b/tracker/tracker/src/main/app/observer/top_observer.ts @@ -78,41 +78,45 @@ export default class TopObserver extends Observer { private handleIframe(iframe: HTMLIFrameElement): void { let doc: Document | null = null let win: Window | null = null - const handle = this.app.safe(() => { - const id = this.app.nodes.getID(iframe) - if (id === undefined) { - //log - return - } - const currentWin = iframe.contentWindow - const currentDoc = iframe.contentDocument - if (currentDoc && currentDoc !== doc) { - const observer = new IFrameObserver(this.app) - this.iframeObservers.push(observer) - observer.observe(iframe) - doc = currentDoc + // setTimeout is required. Otherwise some event listeners (scroll, mousemove) applied in modules + // do not work on the iframe document when it 've been loaded dynamically ((why?)) + const handle = this.app.safe(() => + setTimeout(() => { + const id = this.app.nodes.getID(iframe) + if (id === undefined) { + //log + return + } + const currentWin = iframe.contentWindow + const currentDoc = iframe.contentDocument + if (currentDoc && currentDoc !== doc) { + const observer = new IFrameObserver(this.app) + this.iframeObservers.push(observer) + observer.observe(iframe) // TODO: call unregisterNode for the previous doc if present (incapsulate: one iframe - one observer) + doc = currentDoc - // Le truc - ;(doc as PatchedDocument).__openreplay__getOffset = () => { - const { top, left } = this.getDocumentOffset(iframe.ownerDocument) - return { - top: iframe.offsetTop + top, - left: iframe.offsetLeft + left, + // Le truc + ;(doc as PatchedDocument).__openreplay__getOffset = () => { + const { top, left } = this.getDocumentOffset(iframe.ownerDocument) + return { + top: iframe.offsetTop + top, + left: iframe.offsetLeft + left, + } } } - } - if ( - currentWin && - // Sometimes currentWin.window is null (not in specification). Such window object is not functional - currentWin === currentWin.window && - !this.contextsSet.has(currentWin) // for each context callbacks called once per Tracker (TopObserver) instance - ) { - this.contextsSet.add(currentWin) - //@ts-ignore https://github.com/microsoft/TypeScript/issues/41684 - this.contextCallbacks.forEach((cb) => cb(currentWin)) - win = currentWin - } - }) + if ( + currentWin && + // Sometimes currentWin.window is null (not in specification). Such window object is not functional + currentWin === currentWin.window && + !this.contextsSet.has(currentWin) // for each context callbacks called once per Tracker (TopObserver) instance + ) { + this.contextsSet.add(currentWin) + //@ts-ignore https://github.com/microsoft/TypeScript/issues/41684 + this.contextCallbacks.forEach((cb) => cb(currentWin)) + win = currentWin + } + }, 0), + ) iframe.addEventListener('load', handle) // why app.attachEventListener not working? handle() } diff --git a/tracker/tracker/src/main/modules/img.ts b/tracker/tracker/src/main/modules/img.ts index 724fa0583..4837573ee 100644 --- a/tracker/tracker/src/main/modules/img.ts +++ b/tracker/tracker/src/main/modules/img.ts @@ -97,8 +97,8 @@ export default function (app: App): void { if (!hasTag(node, 'IMG')) { return } - app.nodes.attachElementListener('error', node, sendImgAttrs.bind(node)) - app.nodes.attachElementListener('load', node, sendImgAttrs.bind(node)) + app.nodes.attachNodeListener('error', node, sendImgAttrs.bind(node)) + app.nodes.attachNodeListener('load', node, sendImgAttrs.bind(node)) sendImgAttrs.call(node) observer.observe(node, { attributes: true, attributeFilter: ['src', 'srcset'] }) }) diff --git a/tracker/tracker/src/main/modules/scroll.ts b/tracker/tracker/src/main/modules/scroll.ts index 5f428fe11..78d12797f 100644 --- a/tracker/tracker/src/main/modules/scroll.ts +++ b/tracker/tracker/src/main/modules/scroll.ts @@ -1,30 +1,39 @@ import type App from '../app/index.js' import { SetViewportScroll, SetNodeScroll } from '../app/messages.gen.js' -import { isElementNode, isRootNode } from '../app/guards.js' +import { isNode, isElementNode, isRootNode, isDocument } from '../app/guards.js' + +function getDocumentScroll(doc: Document): [number, number] { + const win = doc.defaultView + return [ + (win && win.pageXOffset) || + (doc.documentElement && doc.documentElement.scrollLeft) || + (doc.body && doc.body.scrollLeft) || + 0, + (win && win.pageYOffset) || + (doc.documentElement && doc.documentElement.scrollTop) || + (doc.body && doc.body.scrollTop) || + 0, + ] +} export default function (app: App): void { let documentScroll = false const nodeScroll: Map = new Map() function setNodeScroll(target: EventTarget | null) { - if (target instanceof Element) { + if (!isNode(target)) { + return + } + if (isElementNode(target)) { nodeScroll.set(target, [target.scrollLeft, target.scrollTop]) } + if (isDocument(target)) { + nodeScroll.set(target, getDocumentScroll(target)) + } } const sendSetViewportScroll = app.safe((): void => - app.send( - SetViewportScroll( - window.pageXOffset || - (document.documentElement && document.documentElement.scrollLeft) || - (document.body && document.body.scrollLeft) || - 0, - window.pageYOffset || - (document.documentElement && document.documentElement.scrollTop) || - (document.body && document.body.scrollTop) || - 0, - ), - ), + app.send(SetViewportScroll(...getDocumentScroll(document))), ) const sendSetNodeScroll = app.safe((s: [number, number], node: Node): void => { @@ -42,11 +51,12 @@ export default function (app: App): void { }) app.nodes.attachNodeCallback((node, isStart) => { + // MBTODO: iterate over all the nodes on start instead of using isStart hack if (isStart && isElementNode(node) && node.scrollLeft + node.scrollTop > 0) { nodeScroll.set(node, [node.scrollLeft, node.scrollTop]) } else if (isRootNode(node)) { // scroll is not-composed event (https://javascript.info/shadow-dom-events) - app.attachEventListener(node, 'scroll', (e: Event): void => { + app.nodes.attachNodeListener('scroll', node, (e: Event): void => { setNodeScroll(e.target) }) } diff --git a/tracker/tracker/src/webworker/BatchWriter.ts b/tracker/tracker/src/webworker/BatchWriter.ts index 252b07e5d..47ef6560c 100644 --- a/tracker/tracker/src/webworker/BatchWriter.ts +++ b/tracker/tracker/src/webworker/BatchWriter.ts @@ -40,7 +40,7 @@ export default class BatchWriter { return } - // MBTODO: move service-messages creation to webworker + // MBTODO: move service-messages creation methods to webworker const batchMetadata: Messages.BatchMetadata = [ Messages.Type.BatchMetadata, 1,