fix(replay): force preserveDrawingBuffer before the recorder chunk loads - #4543
Draft
ksvat wants to merge 5 commits into
Draft
fix(replay): force preserveDrawingBuffer before the recorder chunk loads#4543ksvat wants to merge 5 commits into
ksvat wants to merge 5 commits into
Conversation
A WebGL context created with `preserveDrawingBuffer: false` lets the browser discard the drawn pixels as soon as the frame is composited, so canvas frames captured for replay come back blank. rrweb already forces the attribute on in its `getContext` patch, but that patch only lands once the lazily loaded recorder bundle has arrived. Context attributes are fixed at creation time, so a renderer that boots with the page has already created a context we can never capture. Run the same forcing synchronously during `posthog.init()` when canvas recording is already known to be on - from `session_recording.captureCanvas.recordCanvas`, or from a remote config persisted on an earlier page load. Generated-By: PostHog Desktop Task-Id: 3b084b6b-b9ca-40d2-b734-e95624600275
Contributor
Replay incident risk checkThis diff touches code involved in past incidents. This is a heads-up, not a verdict: read the matched sections of INCIDENTS.md and answer their review questions before merging. For a judgment on whether this diff has the same failure mode, run the |
…config Gating on `_isRecordingEnabled` meant the patch could not run until a remote config had been persisted by an earlier page load, so someone who declared `captureCanvas.recordCanvas` in their own config still lost their first load - exactly the load where a renderer that boots with the page creates the context we cannot capture. Gate on "not disabled and not opted out" instead, and cover the gating with tests. Generated-By: PostHog Desktop Task-Id: 3b084b6b-b9ca-40d2-b734-e95624600275
Contributor
posthog-js Compliance ReportDate: 2026-08-17 22:29:04 UTC ✅ All Tests Passed!26/26 tests passed Capture Tests✅ 26/26 tests passed View Details
|
Contributor
Contributor
|
Size Change: +10.4 kB (+0.05%) Total Size: 19.7 MB 📦 View Changed
ℹ️ View Unchanged
|
`_preserveCanvasDrawingBuffers` is mangled in production builds, so it has to be listed. Matches what the "Write mangled property names" check generates. Generated-By: PostHog Desktop Task-Id: 3b084b6b-b9ca-40d2-b734-e95624600275
The unit tests assert the mechanism - that `preserveDrawingBuffer: true` reaches `getContext` - but not the outcome, and jsdom has no WebGL to check it against. This drives the real ordering in a real browser: the canvas is created from posthog's `loaded` callback, after init() patches getContext but before remote config returns and the recorder chunk loads, and it draws once and never repaints. It leaves a transparent clear colour so rrweb's rescue hack wipes the canvas rather than happening to repaint it, then decodes the captured frame and asserts the drawn red survived. Verified against the unfixed code: without the patch no canvas frame is emitted at all, because every frame encodes as transparent and the worker's fingerprint dedup drops them. Generated-By: PostHog Desktop Task-Id: 3b084b6b-b9ca-40d2-b734-e95624600275
ksvat
marked this pull request as ready for review
August 17, 2026 20:50
ksvat
requested review from
TueHaulund,
arnohillen and
fasyy612
and removed request for
a team
August 17, 2026 20:50
ksvat
marked this pull request as draft
August 17, 2026 22:11
… fix The backward compatibility job runs newly built extensions against the published array.js, and the new test fails there for the reason the fix exists: forcing `preserveDrawingBuffer` has to happen in array.js during `posthog.init()`, before the recorder extension has loaded, so an older array.js cannot do it and the canvas it captures is blank. That is new behaviour gated on the core bundle, not a regression, so skip it below the version that ships it. Generated-By: PostHog Desktop Task-Id: 3b084b6b-b9ca-40d2-b734-e95624600275
ksvat
marked this pull request as ready for review
August 17, 2026 22:17
TueHaulund
reviewed
Aug 18, 2026
Comment on lines
+37
to
+38
| if (isObject(args[0])) { | ||
| args[0].preserveDrawingBuffer = true |
Contributor
There was a problem hiding this comment.
This changes an object the page gave us. If the page reuses that object for other canvases, we have changed it for them too. And if the object is frozen, this line throws, the catch hides it, and the canvas gets created without the fix.
Copying instead of editing avoids both:
args[0] = isObject(args[0]) ? { ...args[0], preserveDrawingBuffer: true } : { preserveDrawingBuffer: true }| * the remote config, and `onRemoteConfig` covers any canvas created after it arrives. | ||
| */ | ||
| private _preserveCanvasDrawingBuffers() { | ||
| if (!window || this._config.disable_session_recording || this._instance.consent.isOptedOut()) { |
Contributor
There was a problem hiding this comment.
This returns early when the visitor has not opted in yet, which is every load on a site with a cookie banner.
When they click accept, opt_in_capturing() starts recording but never calls this method again. It only replays remote config in the cookieless branch. So the patch never runs on that page, and canvas capture stays broken even though recording is now on. That is a common setup in the EU.
Calling _preserveCanvasDrawingBuffers() from startIfEnabledOrStop() would handle init, remote config, and opt-in in one place.
marandaneto
reviewed
Aug 18, 2026
| } | ||
|
|
||
| initialize() { | ||
| this._preserveCanvasDrawingBuffers() |
Member
There was a problem hiding this comment.
blocking: Deferred extension initialization misses the critical window
With __preview_deferred_init_extensions: true, this runs from a timer, but _loaded() and the customer's loaded callback run synchronously first. A WebGL context created there still precedes the patch, defeating the intended ordering fix. Please install this critical patch outside extension deferral and cover this ordering in a browser test.
| } | ||
|
|
||
| initialize() { | ||
| this._preserveCanvasDrawingBuffers() |
Member
There was a problem hiding this comment.
blocking: Dynamically starting recording does not install the patch
If initialization used disable_session_recording: true, this call is skipped. A later startSessionRecording() only reaches startIfEnabledOrStop() through set_config(), so a canvas created immediately after that public call remains unpatched before the recorder chunk loads. Please install the patch synchronously on the recording-start/config-change path too.
|
|
||
| const clientSide = this._config.session_recording?.captureCanvas?.recordCanvas | ||
| const serverSide = this._instance.get_property(SESSION_RECORDING_REMOTE_CONFIG)?.canvasRecording?.enabled | ||
| if (clientSide ?? serverSide) { |
Member
There was a problem hiding this comment.
blocking: Known-disabled projects still have WebGL behavior changed
When persisted session-recording config explicitly has enabled: false, a local captureCanvas.recordCanvas: true still wins here and patches WebGL even though the recorder cannot start. That imposes the preserveDrawingBuffer memory/performance cost on a known-disabled project. Please distinguish missing remote config from explicit disablement.
| if (patched || !canvasPrototype?.getContext) { | ||
| return | ||
| } | ||
| patched = true |
Member
There was a problem hiding this comment.
blocking: The global patch is never removed
patched only transitions to true, and the original getContext is not retained for restoration. After stop, remote disablement, opt-out, or shutdown(), newly created contexts therefore keep forcing preserveDrawingBuffer for the page lifetime. Please add multi-instance-safe, reference-counted cleanup that restores the original when no recorder requires it.
marandaneto
marked this pull request as draft
August 20, 2026 07:38
Member
|
draft until reviews are addressed so its clear from our review queue |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Canvas recording can be enabled and working, and a WebGL canvas still replays blank.
A WebGL context created with
preserveDrawingBuffer: false— the spec default, and whatmost renderers ask for — lets the browser discard the drawn pixels as soon as the frame has
been composited. Replay reads pixels back out of the canvas some time after the page drew
into them, so the captured frames come back empty.
rrweb already forces that attribute on inside its
getContextpatch, but the patch isinstalled by
CanvasManager, which only exists once the lazily loaded recorder bundle hasarrived. Context attributes are fixed at creation time and cannot be changed afterwards, so
any renderer that boots while the page is loading — WebGL editors, map and 3D canvases,
WASM-backed engines — has already created a context we can never capture. The existing
context.clear(COLOR_BUFFER_BIT)fallback does not reliably rescue those pixels, and for arenderer that only redraws on demand it can blank pixels that were still valid.
This came out of a support thread where an app embedding a third-party WebGL editor had
canvas capture switched on and still saw an empty canvas in every recording.
Changes
Run the same
preserveDrawingBufferforcing synchronously duringposthog.init(), so acontext created during page load is capture-ready before the recorder exists.
It is deliberately narrow — it forces the one attribute, only for WebGL context types, and
only when canvas recording is already known to be on: either declared client side via
session_recording.captureCanvas.recordCanvas, or read from a remote config persisted on anearlier page load. Projects that turn canvas recording on purely in project settings
therefore get the fix from their users' next page load onwards.
Note for anyone tracking this: the patch has to live in
array.js, not the recorder extension —the entire point is to run before the extension has loaded. So this reaches users when they upgrade
the core bundle; a cached older
array.jspicking up fresh extensions will not get it. That is whatthe backward-compatibility job flags, and why the new test is skipped below the shipping version.
Release info Sub-libraries affected
Libraries affected
Checklist
Created with PostHog Desktop