Repository navigation
chore!: remove wireframe session replay capture - #908
Conversation
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 697f9c0: 1 Should fix.
This is the only automatic review while this pull request is a draft. Comment @posthog review to review new changes sooner. Otherwise, a review of the new changes runs when you mark it ready for review.
See this review and its findings in PostHog Review. Comment @posthog review --deep for a Deep review. A Deep review takes longer and looks deeper than a Standard review.
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
PR overviewAll previously flagged issues have been addressed. No open security concerns remain on this pull request. Security reviewNo open security issues remain on this pull request. Fixed/addressed: 3 · PR risk: 0/10 |
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 37df5a2: 1 Must fix.
This is review round 3 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.
dustinbyrne
left a comment
dustinbyrne
left a comment
There was a problem hiding this comment.
The animated-view and idle surface-scan fixes are verified. Two default screenshot privacy gaps remain: overflowing descendants of masked containers and Compose images without descriptions. Details are inline and in the existing Compose-image discussion.
AI-assisted review.
Session replay always records masked screenshots, the mode `screenshot = true` already used. Remove `PostHogSessionReplayConfig.screenshot`, and `drawableConverter` / `PostHogDrawableConverter`, which only the wireframe builder read. Co-Authored-By: Claude Opus 5.5 <[email protected]>
With wireframe capture gone, each window sends a single screenshot node, so the tree fields, RRStyle and the child-flattening diff are unused. Screenshot frames no longer carry an empty `style`, matching iOS 4.0. Co-Authored-By: Claude Opus 5.5 <[email protected]>
37df5a2 to
9856753
Compare
# Conflicts: # posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt # posthog-android/src/main/java/com/posthog/android/replay/PostHogSessionReplayConfig.kt # posthog-android/src/test/java/com/posthog/android/replay/PostHogReplayIntegrationTest.kt
Co-Authored-By: Claude Opus 5.5 <[email protected]>
There was a problem hiding this comment.
PostHog Review
Found 3 must fix.
This pull request is large, so the review ran in 4 parts with less depth than usual.
This is review round 4 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.
Findings on the changed lines
posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt:789
P1 · Mask clipped text inputs before making screenshots the default
With default replay, an EditText partially clipped by a scrolling parent now uploads visible characters. addTextAreaGlobalVisibleRect() adds compound padding to the already-clipped visible rect, leaving rendered text outside the mask; wireframe capture previously serialized masked text. Compute text bounds before clipping, or mask the full visible rect.
posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt:789
P1 · Cover the entire sensitive rectangle before capturing screenshots by default
With default replay settings, masked images now expose corner pixels: paintScreenshotMasks() uses drawRoundRect with an inward radius, leaving pixels inside the sensitive rectangle untouched. Wireframe capture previously omitted those images entirely. Small images and text near view corners therefore leak into uploaded screenshots; paint opaque rectangular masks.
posthog-android/src/main/java/com/posthog/android/replay/PostHogReplayIntegration.kt:1122
P1 · Traverse unclipped containers whose own bounds are offscreen
When a stable container has offscreen bounds and clipChildren = false, its descendants can still draw onscreen. isVisible() rejects the container, while isRenderedButUnplaceable() returns false, so the walk skips its sensitive descendants. Default screenshot capture now uploads pixels wireframes omitted; traverse unclipped descendants despite the parent's invisible rect.
… HEAD # Conflicts: # .changeset/android-v4-major.md
There was a problem hiding this comment.
PostHog Review · Standard · reviewed fc6bbaa: 1 Must fix.
This is review round 5 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.
Merged semantics promote a child's postHogUnmask to its clickable parent, which skipped masking for every sibling. Read the unmerged tree instead.
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 885380c: 2 Must fix.
This is review round 6 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.
Text with TextOverflow.Visible draws past its layout box, and clearAndSetSemantics removes the Text metadata the walk relies on. Both left text unmasked in screenshots. Co-Authored-By: Claude Opus 5.5 <[email protected]>
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 92916ae: 2 Must fix, 1 Should fix.
This is review round 7 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.
… shadows Overflow bounds ignored graphicsLayer transforms, and text shadows could paint readable copies outside the mask.
Text(String) rebuilds its paragraph on every GetTextLayoutResult call, which the verification and draw-sample walks repeated on the main thread.
There was a problem hiding this comment.
PostHog Review · Standard · reviewed 788b3a8: 2 Must fix, 1 Should fix. This pull request is large, so the review ran in 4 parts with less depth than usual.
This is review round 8 on this pull request. When findings keep coming back in the same code, the approach is usually the problem. Before the next fix, check whether a different design would remove the whole class of findings.
…er walk A text style change is invisible until the layout is re-read, so any draw now drops the cached bounds. A walk over several ComposeViews also evicted earlier roots by clearing the shared cache for each one. Co-Authored-By: Claude Opus 5.5 <[email protected]>
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Cleared semantics wipe the text semantics on the same layout node, so its overflow was masked only to the layout box. Read them from the text's modifier and reuse the painted-bounds measurement. Discard the frame when they can't be read, including on legacy walks.
…0 guide Co-Authored-By: Claude Opus 5.5 <[email protected]>
💡 Motivation and Context
Part of the 4.0 stack, based on #871 (
chore/v4-remove-deprecated). Ioannis asked to drop wireframe on Android too, matching posthog-ios 4.0 (PostHog/posthog-ios#907, then PostHog/posthog-ios#973 removedscreenshotModeoutright).Android session replay has two capture modes. Wireframe is the default (
screenshot = false). It serializes the View tree to JSON. Screenshot captures masked images. This PR removes wireframe, so replay always captures masked screenshots.Why:
ph-no-capturecontainers. sdk-specscompliance/posthog-android.mdn26 marks this:toWireframe()never checksisNoCapture()for a tagged container, so its children's text is still serialized in the default mode. The screenshot path masks a tagged view as one rect.ComposeViewwas one box with a background color. The screenshot path masks Compose through semantics.What changes
childWireframes,style, text masked with*)screenshotwireframe with maskedbase64, the same payloadscreenshot = truealready sentPostHogSessionReplayConfig.screenshotdrawableConverter/PostHogDrawableConverter$sdk_debug_replay_capture_modewireframe/screenshotscreenshotThe full snapshot and meta events are the same as what screenshot mode sends today, and the meta
href(window title) is unchanged. The player already handles both formats, so the server doesn't need any change.Public API: removing the option, not deprecating it as a no-op, follows PostHog/posthog-ios#973. An app that set
screenshot = falseto keep pixels out of recordings now gets a compile error instead of silently uploading screenshots, so it has to choose: mask what's sensitive (ph-no-capture,Modifier.postHogMask(),maskAllImages/maskAllTextInputs) or turn replay off.Core
RRWireframetrimmed (16126b22): it keeps onlyid,x,y,width,height,typeandbase64, matching iOS 4.0, andRRStyleis deleted. Each window sends one screenshot node, so the child-flattening tree diff becomes a single-node compare (update when the image or size changes). Screenshot frames no longer send an emptystyle: {}, the same shape iOS 4.0 sends. posthog-flutter'sSnapshotSenderpassedstyle = RRStyle(), so it needs PostHog/posthog-flutter#684.Deleted as orphaned:
toWireframe()and its helpers (Drawable.base64/toRGBColor/toBitmap/copy,String.mask(), the theme background lookup) and theforceScreenshotparameter ongenerateSnapshot.For standard widgets the masking decision is unchanged: both modes used the same
shouldMask*/isTextInputSensitivehelpers. What changes is everything else:onDraw, maps, video, charts). Screenshots render the pixels, and masking only covers recognized widgets plusph-no-capture/Modifier.postHogMask().Downstream PRs
SessionRecordingConfig.screenshot(stacked on chore!: adopt posthog-android 4.0, core 7.0 and posthog-ios 4.0 posthog-kmp#108). It no longer compiles against this branch without that change.sessionReplayConfig.screenshot = true(stacked on chore(react-native-plugin)!: require posthog-android 4.0 posthog-js#5476).SnapshotSender.ktusedRRStyleandRRWireframe(style = …), which this PR deletes. chore: stop passing RRStyle to replay screenshot frames posthog-flutter#684 removes them on fix(replay): keep frames on screens with continuous surface rendering (e.g. Rive) #649's branch, and the same commit is now on feat!: capture v1 on every platform, with per-event options and captureAi posthog-flutter#673 and the PRs stacked on it. Flutter doesn't setscreenshot, and its Android replay already reportsscreenshot. The Gradle plugin doesn't touch replay.Follow-ups (not in this PR)
posthog-react-native-session-replayrepo also setsscreenshot = true, but it's still on posthog-android 3.x.💚 How did you test it?
:posthog-android:testDebugUnitTestandtestWithoutCompose: pass.PostHogReplayIntegrationTestis 133/133.screenshot = truenow get the same thing from the default config.debugPropertiestests are now one test, which checks that the default config reportsscreenshot.batch-request.jsonevent-shape snapshot changes only on that one property.spotlessCheck,apiCheckand the sample app build pass.posthog7.0.0 /posthog-android4.0.0 from this branch to a local Maven repo and built KMP's downstream PR against it: config tests pass, the sample builds.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file (added a bullet to the existing.changeset/android-v4-major.md)🤖 Agent context
Autonomy: Human-driven (agent-assisted)
drawableConverter, which was never deprecated, because without wireframe it does nothing. Easy to put back as a deprecated no-op if we'd rather.🤖 Generated with Claude Code