Repository navigation
fix(replay): fill screenshot masks to the full rectangle - #969
Merged
Merged
Conversation
Masks were painted as 10pt rounded rects, so the pixels in each mask's four corners stayed visible in the uploaded screenshot. Fill the whole rect instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
PostHog Review · Standard · reviewed d3bea8c: 1 Should fix.
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.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… (reverted next) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
posthog-ios Compliance ReportDate: 2026-10-27T16:48:59.613969+00:00
|
| Test | Status | Duration |
|---|---|---|
| Format Validation.Event Has Required Fields | ✅ | 3104ms |
| Format Validation.Event Has Uuid | ✅ | 2934ms |
| Format Validation.Event Has Lib Properties | ✅ | 2900ms |
| Format Validation.Distinct Id Is String | ✅ | 2972ms |
| Format Validation.Token Is Present | ✅ | 2904ms |
| Format Validation.Custom Properties Preserved | ✅ | 3003ms |
| Format Validation.Event Has Timestamp | ✅ | 2848ms |
| Format Validation.Non Utc Event Timestamp Is Converted To Utc | ✅ | 2947ms |
| Retry Behavior.Retries On 503 | ✅ | 11980ms |
| Retry Behavior.Does Not Retry On 400 | ✅ | 4800ms |
| Retry Behavior.Does Not Retry On 401 | ✅ | 4903ms |
| Retry Behavior.Respects Retry After Header | ✅ | 7825ms |
| Retry Behavior.Implements Backoff | ✅ | 21417ms |
| Retry Behavior.Retries On 500 | ✅ | 8830ms |
| Retry Behavior.Retries On 502 | ✅ | 5375ms |
| Retry Behavior.Retries On 504 | ✅ | 9257ms |
| Retry Behavior.Max Retries Respected | ❌ | 19164ms |
| Deduplication.Generates Unique Uuids | ✅ | 2992ms |
| Deduplication.Preserves Uuid On Retry | ✅ | 5412ms |
| Deduplication.Preserves Uuid And Timestamp On Retry | ✅ | 16836ms |
| Deduplication.Preserves Uuid And Timestamp On Batch Retry | ✅ | 7789ms |
| Deduplication.No Duplicate Events In Batch | ✅ | 3169ms |
| Deduplication.Different Events Have Different Uuids | ✅ | 3067ms |
| Compression.Sends Gzip When Enabled | ✅ | 3140ms |
| Batch Format.Uses Proper Batch Structure | ✅ | 2855ms |
| Batch Format.Flush With No Events Sends Nothing | ✅ | 446ms |
| Batch Format.Multiple Events Batched Together | ✅ | 3163ms |
| Error Handling.Does Not Retry On 403 | ✅ | 4957ms |
| Error Handling.Does Not Retry On 413 | ✅ | 5004ms |
| Error Handling.Retries On 408 | ✅ | 9442ms |
Failures
retry_behavior.max_retries_respected
Expected 4 requests, got 6
Feature_Flags Tests
✅ 17/17 tests passed
View Details
| Test | Status | Duration |
|---|---|---|
| Request Payload.Request With Person Properties Device Id | ✅ | 2957ms |
| Request Payload.Flags Request Uses V2 Query Param | ✅ | 3044ms |
| Request Payload.Flags Request Hits Flags Path Not Decide | ✅ | 2997ms |
| Request Payload.Flags Request Omits Authorization Header | ✅ | 3008ms |
| Request Payload.Token In Flags Body Matches Init | ✅ | 3001ms |
| Request Payload.Groups Round Trip | ✅ | 3005ms |
| Request Payload.Groups Default To Empty Object | ✅ | 2997ms |
| Request Payload.Disable Geoip False Propagates As Geoip Disable False | ✅ | 3035ms |
| Request Payload.Disable Geoip Omitted Defaults To False | ✅ | 2972ms |
| Request Payload.Flag Keys To Evaluate Contains Only Requested Key | ✅ | 2996ms |
| Request Lifecycle.No Flags Request On Init Alone | ✅ | 137ms |
| Request Lifecycle.No Flags Request On Normal Capture | ✅ | 2900ms |
| Request Lifecycle.Two Flag Calls Produce Two Remote Requests | ✅ | 5802ms |
| Request Lifecycle.Mock Response Value Is Returned To Caller | ✅ | 2981ms |
| Retry Behavior.Retries Flags On 502 | ✅ | 3007ms |
| Retry Behavior.Retries Flags On 504 | ✅ | 3144ms |
| Side Effect Events.Get Feature Flag Captures Feature Flag Called Event | ✅ | 3157ms |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Rendered by CI on the pinned Xcode 26.3 / iOS 26.2 runtime, which isn't installed locally. Every changed pixel is inside a mask's corner. The 3 unchanged cases mask nothing. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
dustinbyrne
approved these changes
Oct 27, 2026
dustinbyrne
left a comment
Contributor
There was a problem hiding this comment.
The full-rectangle fill closes the screenshot-mask corner gap without changing discovery, encoding or public APIs. The focused corner/edge regression and refreshed pinned goldens cover the change; the earlier golden-image finding is resolved.
AI-assisted review.
ioannisj
approved these changes
Oct 27, 2026
This was referenced Oct 29, 2026
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
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.
💡 Motivation and Context
Screenshot replay painted each mask as a rounded rect (
UIBezierPath(roundedRect: rect, cornerRadius: 10)). UIKit draws that as a continuous corner about 13–14px across, so the pixels in the four corners of every masked region weren't covered and were uploaded. #873 (4.0) makes screenshot mode mandatory, so every masked region in every recording has this gap. This PR fills the whole rect (UIRectFill(rect)).Alternative, if we want to keep the rounded look: grow each mask a little and keep the 10pt rounded fill, so the corner arc falls outside the original rect:
A 10pt radius covers the original corner once each side grows by about 2.9pt (10 × (1 − 1/√2)). 4pt leaves room for UIKit's continuous corners. The cost is that every mask is 4pt bigger on each side, so nearby masks can merge and more of the surrounding UI gets covered. It would also need the goldens re-recorded again. This PR goes with the square fill because it's simpler and covers exactly the rect. Wireframe mode never had mask shapes (masked text became
***and sensitive images were dropped), so the rounded boxes were only ever a screenshot-mode look.This came out of the bot review thread on #873: #873 (comment). The fix lands on
mainand will merge forward into the v4 branch.Spec / parity:
session-replay-privacysays native screenshot replay "paints opaque masks over screenshots" for the computed mask rectangles. It doesn't specify a corner shape, so a full-rect fill is the conforming behavior.canvas.drawRect), so iOS now matches it.canvas.drawRoundRect(maskRect, 10f * scaleX, 10f * scaleY, paint)inPostHogReplayIntegration.kt, so it has the same corner gap. It isn't changed here.💚 How did you test it?
RRWireframeTest›maskImage, iOS): checks that all 4 corner pixels of a mask rect are filled, and that the pixel just outside each edge isn't.MASK_SNAPSHOT_XCODE/MASK_SNAPSHOT_OS), because that toolchain isn't installed locally.actualPNGs. A later commit reverts it.actualPNGs are written with the samewrite(composite)callrecordGoldens()uses.make lintpasses.RRWireframeTest,PostHogMaskFailClosedTest,PostHogReactNativeMaskingTest(39 tests).make buildSdkcouldn't run locally becausexcprettyisn't installed. The CI Build SDK jobs cover it.📝 Checklist
If releasing new changes
pnpm changesetto generate a changeset file🤖 Agent context
Autonomy: Human-driven (agent-assisted)
🤖 Generated with Claude Code