From 8ab4f4dbf200e5ceb9e2a912cb55073796d4b6f3 Mon Sep 17 00:00:00 2001 From: sylenien Date: Mon, 4 Jul 2022 17:35:07 +0200 Subject: [PATCH 1/4] fix(tracker): fix srcset tracking --- tracker/tracker/package.json | 2 +- tracker/tracker/src/main/modules/img.ts | 19 ++++++++++++++++--- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/tracker/tracker/package.json b/tracker/tracker/package.json index ebbcc4e6e..183006c05 100644 --- a/tracker/tracker/package.json +++ b/tracker/tracker/package.json @@ -1,7 +1,7 @@ { "name": "@openreplay/tracker", "description": "The OpenReplay tracker main package", - "version": "3.5.14", + "version": "3.5.15", "keywords": [ "logging", "replay" diff --git a/tracker/tracker/src/main/modules/img.ts b/tracker/tracker/src/main/modules/img.ts index e635eb741..b100bb4e3 100644 --- a/tracker/tracker/src/main/modules/img.ts +++ b/tracker/tracker/src/main/modules/img.ts @@ -3,7 +3,16 @@ import { timestamp, isURL } from "../utils.js"; import { ResourceTiming, SetNodeAttributeURLBased, SetNodeAttribute } from "../../common/messages.js"; import { hasTag } from "../app/guards.js"; - +function resolveImageUrl(url: string, location: Location) { + const cleanUrl = url.trim() + if (cleanUrl.startsWith('/')) { + return location.origin + cleanUrl + } else if (cleanUrl.startsWith('http') || cleanUrl.startsWith('data:')) { + return cleanUrl + } else { + return location.origin + location.pathname + cleanUrl + } +} const PLACEHOLDER_SRC = "https://static.openreplay.com/tracker/placeholder.jpeg"; @@ -35,8 +44,12 @@ export default function (app: App): void { } else if (src.length >= 1e5 || app.sanitizer.isMasked(id)) { sendPlaceholder(id, this) } else { - app.send(new SetNodeAttributeURLBased(id, 'src', src, app.getBaseHref())); - srcset && app.send(new SetNodeAttribute(id, 'srcset', srcset)); + const imgSrc = resolveImageUrl(src, document.location) + app.send(new SetNodeAttribute(id, 'src', imgSrc)); + if (srcset) { + const fixedSet = srcset.split(',').map(str => resolveImageUrl(str, document.location)).join(',') + app.send(new SetNodeAttribute(id, 'srcset', fixedSet)); + } } }); From 573bdc5657fa20fcbf0e8f947954b195be65b90c Mon Sep 17 00:00:00 2001 From: Alex Kaminskii Date: Mon, 4 Jul 2022 19:08:07 +0200 Subject: [PATCH 2/4] feat(player): dev-log on skipping message --- .../app/player/MessageDistributor/messages/MFileReader.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frontend/app/player/MessageDistributor/messages/MFileReader.ts b/frontend/app/player/MessageDistributor/messages/MFileReader.ts index a4df3e2f8..48f9bd3cf 100644 --- a/frontend/app/player/MessageDistributor/messages/MFileReader.ts +++ b/frontend/app/player/MessageDistributor/messages/MFileReader.ts @@ -45,9 +45,11 @@ export default class MFileReader extends RawMessageReader { } while (this.needSkipMessage()) { - if (!this.readRawMessage()) { + const skippedMessage = this.readRawMessage() + if (!skippedMessage) { return null } + logger.log("Skipping message: ", skippedMessage) } this.pLastMessageID = this.p From d83b28079da7033634b1540f088c529c32478e7e Mon Sep 17 00:00:00 2001 From: Alex Kaminskii Date: Mon, 4 Jul 2022 19:48:58 +0200 Subject: [PATCH 3/4] fix(tracker): removeNode mutation priority over attributes --- tracker/tracker/src/main/app/observer/observer.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tracker/tracker/src/main/app/observer/observer.ts b/tracker/tracker/src/main/app/observer/observer.ts index dfaaa3986..940a7eefb 100644 --- a/tracker/tracker/src/main/app/observer/observer.ts +++ b/tracker/tracker/src/main/app/observer/observer.ts @@ -184,7 +184,7 @@ export default abstract class Observer { const [ id, isNew ]= this.app.nodes.registerNode(node); if (isNew){ this.recents.set(id, RecentsType.New) - } else if (!this.recents.has(id)) { + } else if (this.recents.get(id) !== RecentsType.New) { // can we do just `else` here? this.recents.set(id, RecentsType.Removed) } } @@ -231,6 +231,8 @@ export default abstract class Observer { // TODO: Clean the logic (though now it workd fine) if (!hasTag(node, "HTML") || !this.isTopContext) { if (parent === null) { + // Sometimes one observation contains attribute mutations for the removimg node, which gets ignored here. + // That shouldn't affect the visual rendering ( should it? ) this.unbindNode(node); return false; } From e414829ee6a8a25baba6b9e1806ea8348da968ba Mon Sep 17 00:00:00 2001 From: Alex Kaminskii Date: Mon, 4 Jul 2022 20:09:58 +0200 Subject: [PATCH 4/4] fix(tracker): capture relative img timing;use startsWith instead of substr; codestyle fix --- tracker/tracker/src/main/modules/img.ts | 32 ++++++++++++++----------- tracker/tracker/src/main/utils.ts | 2 +- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/tracker/tracker/src/main/modules/img.ts b/tracker/tracker/src/main/modules/img.ts index b100bb4e3..b462f7705 100644 --- a/tracker/tracker/src/main/modules/img.ts +++ b/tracker/tracker/src/main/modules/img.ts @@ -3,14 +3,18 @@ import { timestamp, isURL } from "../utils.js"; import { ResourceTiming, SetNodeAttributeURLBased, SetNodeAttribute } from "../../common/messages.js"; import { hasTag } from "../app/guards.js"; -function resolveImageUrl(url: string, location: Location) { - const cleanUrl = url.trim() - if (cleanUrl.startsWith('/')) { - return location.origin + cleanUrl - } else if (cleanUrl.startsWith('http') || cleanUrl.startsWith('data:')) { - return cleanUrl +function resolveURL(url: string, location: Location = document.location) { + url = url.trim() + if (url.startsWith('/')) { + return location.origin + url + } else if ( + url.startsWith('http://') || + url.startsWith('https://') || + url.startsWith('data:') // any other possible value here? + ){ + return url } else { - return location.origin + location.pathname + cleanUrl + return location.origin + location.pathname + url } } @@ -37,18 +41,18 @@ export default function (app: App): void { if (!complete) { return; } + const resolvedSrc = resolveURL(src || '') // Src type is null sometimes. - is it true? if (naturalWidth === 0 && naturalHeight === 0) { - if (src != null && isURL(src)) { // TODO: How about relative urls ? Src type is null sometimes. - app.send(new ResourceTiming(timestamp(), 0, 0, 0, 0, 0, src, 'img')); + if (isURL(resolvedSrc)) { + app.send(new ResourceTiming(timestamp(), 0, 0, 0, 0, 0, resolvedSrc, 'img')); } - } else if (src.length >= 1e5 || app.sanitizer.isMasked(id)) { + } else if (resolvedSrc.length >= 1e5 || app.sanitizer.isMasked(id)) { sendPlaceholder(id, this) } else { - const imgSrc = resolveImageUrl(src, document.location) - app.send(new SetNodeAttribute(id, 'src', imgSrc)); + app.send(new SetNodeAttribute(id, 'src', resolvedSrc)) if (srcset) { - const fixedSet = srcset.split(',').map(str => resolveImageUrl(str, document.location)).join(',') - app.send(new SetNodeAttribute(id, 'srcset', fixedSet)); + const resolvedSrcset = srcset.split(',').map(str => resolveURL(str)).join(',') + app.send(new SetNodeAttribute(id, 'srcset', resolvedSrcset)) } } }); diff --git a/tracker/tracker/src/main/utils.ts b/tracker/tracker/src/main/utils.ts index 6124119dd..20dcb3a25 100644 --- a/tracker/tracker/src/main/utils.ts +++ b/tracker/tracker/src/main/utils.ts @@ -13,7 +13,7 @@ export function normSpaces(str: string): string { // isAbsoluteUrl regexp: /^([a-z][a-z\d\+\-\.]*:)?\/\//i.test(url) export function isURL(s: string): boolean { - return s.substr(0, 8) === 'https://' || s.substr(0, 7) === 'http://'; + return s.startsWith('https://')|| s.startsWith('http://'); } export const IN_BROWSER = !(typeof window === "undefined");