fix(web): forward maskAllTexts/maskAllImages to posthog-js maskCanvas on Flutter web#454
fix(web): forward maskAllTexts/maskAllImages to posthog-js maskCanvas on Flutter web#454tsushanth wants to merge 1 commit into
Conversation
… on Flutter web
On Flutter web with the CanvasKit renderer, session replay is handled by
posthog-js recording the raw <canvas> element. The Dart widget-tree masking
pipeline (RenderParagraph / RenderImage rectangles) is never reached because
sendMetaEvent and sendFullSnapshot are no-ops on web, so maskAllTexts and
maskAllImages were silently ignored and PII painted to the canvas remained
visible in replays.
Fix: during setup() on web, when sessionReplay is enabled and either
masking flag is true, call posthog.set_config({ session_recording: { maskCanvas:
true } }). rrweb then replaces the entire canvas content with a solid colour
before encoding each snapshot frame, matching the intent of the masking flags.
The tradeoff — whole-canvas masking instead of per-widget bounds — is
fundamental to how CanvasKit works (pixels, no DOM text nodes) and is
documented on PostHogSessionReplayConfig with guidance to configure
session_recording.maskCanvas directly in posthog.init() for more control.
Adds four browser-only tests covering the four flag combinations.
Fixes PostHog/posthog#66291
|
Reviews (1): Last reviewed commit: "fix(web): forward maskAllTexts/maskAllIm..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| test('calls set_config maskCanvas when maskAllTexts is true', () async { | ||
| final config = PostHogConfig('test-token') | ||
| ..sessionReplay = true | ||
| ..sessionReplayConfig = (PostHogSessionReplayConfig() | ||
| ..maskAllTexts = true | ||
| ..maskAllImages = false); | ||
|
|
||
| await PosthogFlutterWeb().setup(config); | ||
|
|
||
| expect(setConfigCalled, isTrue); | ||
| final cfg = capturedConfig!.dartify()! as Map<Object?, Object?>; | ||
| final recording = cfg['session_recording'] as Map<Object?, Object?>; | ||
| expect(recording['maskCanvas'], isTrue); | ||
| }); | ||
|
|
||
| test('calls set_config maskCanvas when maskAllImages is true', () async { | ||
| final config = PostHogConfig('test-token') | ||
| ..sessionReplay = true | ||
| ..sessionReplayConfig = (PostHogSessionReplayConfig() | ||
| ..maskAllTexts = false | ||
| ..maskAllImages = true); | ||
|
|
||
| await PosthogFlutterWeb().setup(config); | ||
|
|
||
| expect(setConfigCalled, isTrue); | ||
| final cfg = capturedConfig!.dartify()! as Map<Object?, Object?>; | ||
| final recording = cfg['session_recording'] as Map<Object?, Object?>; | ||
| expect(recording['maskCanvas'], isTrue); | ||
| }); | ||
|
|
||
| test('does not call set_config maskCanvas when both masking flags are false', | ||
| () async { | ||
| final config = PostHogConfig('test-token') | ||
| ..sessionReplay = true | ||
| ..sessionReplayConfig = (PostHogSessionReplayConfig() | ||
| ..maskAllTexts = false | ||
| ..maskAllImages = false); | ||
|
|
||
| await PosthogFlutterWeb().setup(config); | ||
|
|
||
| expect(setConfigCalled, isFalse); | ||
| }); | ||
|
|
||
| test('does not call set_config maskCanvas when sessionReplay is disabled', | ||
| () async { | ||
| final config = PostHogConfig('test-token') | ||
| ..sessionReplay = false | ||
| ..sessionReplayConfig = (PostHogSessionReplayConfig() | ||
| ..maskAllTexts = true | ||
| ..maskAllImages = true); | ||
|
|
||
| await PosthogFlutterWeb().setup(config); | ||
|
|
||
| expect(setConfigCalled, isFalse); | ||
| }); |
There was a problem hiding this comment.
The four new tests are structurally identical — each creates a PostHogConfig with a specific pair of (maskAllTexts, maskAllImages, sessionReplay) values and asserts whether set_config was called. Per the project's coding conventions, this pattern should be expressed as a single parameterised test (a for loop over a table of cases) rather than four near-duplicate test bodies. The "both flags true + sessionReplay enabled" combination is also absent from the positive-call cases, which a table format would make obvious to add.
Context Used: Do not attempt to comment on incorrect alphabetica... (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
|
@tsushanth IIRC Could you please validate against an actual recorded replay, since the current tests only assert the SDK calls its own mock? A short public video of a replay with the canvas masked attached to the PR would be super helpful for confirming it end to end. 🙏 |
| ### Bug Fixes | ||
|
|
||
| - Flutter web (CanvasKit): `maskAllTexts` and `maskAllImages` now take effect on web. Previously these flags were no-ops on Flutter web because the Dart widget-tree masking pipeline is bypassed when posthog-js records the raw `<canvas>` element. The SDK now translates either flag to posthog-js `session_recording.maskCanvas: true` during `setup()`, which instructs rrweb to replace the entire canvas with a solid colour in the recorded stream. This closes the PII gap reported for `Text` widgets rendered with the CanvasKit renderer (fixes PostHog/posthog#66291). |
There was a problem hiding this comment.
Let's revert this and add it as a changeset item
| try { | ||
| ph.set_config( | ||
| mapToJSAny({ | ||
| 'session_recording': {'maskCanvas': true}, |
There was a problem hiding this comment.
Can we validate that this gets used correctly by web sdk? Looking at the code here it seems that maskCanvas will be silently dropped
There was a problem hiding this comment.
Also should this config be merged with any possible config passed js init side? (since we'll be replacing the whole config this way)
3 participants
Problem
On Flutter web with the CanvasKit renderer,
maskAllTexts = trueandmaskAllImages = trueinPostHogSessionReplayConfigwere silently ignored. Session replay there is handled entirely by posthog-js recording the raw<canvas>element — the Dart widget-tree masking pipeline (painting black rectangles overRenderParagraph/RenderImagenodes before screenshot) is never reached becausesendMetaEventandsendFullSnapshotare no-ops on web. As a result, PII painted to the canvas by Text widgets remained visible in recorded replays regardless of the masking config.Reported in PostHog/posthog#66291.
Fix
During
setup()on web, whensessionReplayis enabled and either masking flag istrue, the SDK now calls:rrweb then replaces the entire canvas content with a solid colour before encoding each snapshot frame — which is the closest equivalent to per-widget masking that the CanvasKit architecture allows, since all content exists as opaque pixels rather than DOM text nodes.
The tradeoff (whole-canvas replacement vs per-widget bounds) is intrinsic to how CanvasKit works. The
PostHogSessionReplayConfigdocstring now explains the platform difference and points users toposthog.init()session_recording.maskCanvasfor direct control if they need to decouple the flags.Changes
posthog_flutter_web_handler.dart— exposeset_configon thePostHogJS interop extensionposthog_flutter_web.dart— callset_configwithmaskCanvas: trueduringsetup()when either masking flag is enabledposthog_config.dart— document per-platform masking behaviour onPostHogSessionReplayConfig,maskAllTexts, andmaskAllImagesposthog_flutter_web_handler_test.dart— four browser-only tests covering all flag combinations (maskAllTexts only, maskAllImages only, both false, sessionReplay disabled)Testing
All four new tests plus the existing suite pass.