Skip to content

Conversation

@philprime
Copy link
Member

@philprime philprime commented Sep 26, 2025

📜 Description

This PR fixes several session replay masking issues and adds comprehensive test coverage for the redaction builder to prevent regressions.

💡 Motivation and Context

Fixed Masking Issues

SwiftUI.List Background Clipping Issue (#6292)
One screen in the dogfooding app Flinky had significant masking problems where the entire top half of a screen with SwiftUI.List was unmasked. Analysis revealed that _UICollectionViewListLayoutSectionBackgroundColorDecorationView has an extremely large frame (-20 -1135.33; 442 2336) that extends far beyond the visible list bounds, causing incorrect clipping calculations.

Implementation Improvements

String-Based Class Comparison

  • Introduced ExtendedClassIdentifier to compare view classes by string description instead of direct class references
  • Prevents triggering Objective-C +initialize on private UIKit classes when running off the main thread
  • Supports optional layer class filtering for multi-purpose views (e.g., SwiftUI._UIGraphicsView)

Rendering & Layer Improvements

  • Fixed sublayer rendering order by sorting layers by zPosition (with insertion order as tie-breaker)
  • Switched from view-based to layer-based traversal for accurate presentation state
  • Properly handle presentation layers during animations vs model layers

Edge Case Handling

  • UISwitch subtree ignoring to prevent internal UIImageView from being incorrectly redacted
  • UITextField placeholder handling via UITextFieldLabel detection
  • SwiftUI.List background decoration view treated as special case (direct redact, no clip-out)
  • Axis-aligned transform detection to optimize opaque view clipping
  • Custom anchor point support in transform calculations

Test Coverage

Added 49 new tests to systematically verify:

  • ExtendedClassIdentifier matching with layer filtering
  • UIImageView boundary conditions (nil, 9x9, 10x10, 11x11 pixels)
  • Hidden views and zero opacity handling
  • Transform calculations with custom anchor points
  • Region ordering and SwiftUI region prioritization
  • Nested clipping regions
  • Class hierarchy inheritance patterns
  • iOS version-specific behavior (iOS 26 UISlider visual element)

Note: SwiftUI-specific integration tests (Text, Image, List, Label, Button) will be added in a follow-up PR.

💚 How did you test it?

Manual Testing

  • Verified fix in Flinky app - SwiftUI.List screens now mask correctly
  • Tested across iOS 15.5, 16.4, 17.5, 18.6, and 26.0 simulators

Automated Testing

  • 49 new test cases added
  • All 118+ tests passing (0 failures)
  • Tests follow Arrange-Act-Assert pattern
  • Standardized snapshot naming (masked/unmasked)

Files Changed

  • Sources/Swift/Core/Tools/ViewCapture/SentryUIRedactBuilder.swift - Core fixes
  • Tests/SentryTests/ViewCapture/SentryUIRedactBuilderTests+EdgeCases.swift - New (32 tests)
  • Tests/SentryTests/ViewCapture/SentryUIRedactBuilderTests+Common.swift - Enhanced (8 new tests)
  • Tests/SentryTests/ViewCapture/SentryUIRedactBuilderTests+UIKit.swift - Enhanced (5 new tests, removed 2 duplicates)
  • Tests/SentryTests/ViewCapture/SentryUIRedactBuilderTests+SpecialViews.swift - Updated naming

📝 Checklist

You have to check all boxes before merging:

  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

@philprime philprime self-assigned this Sep 26, 2025
@codecov
Copy link

codecov bot commented Sep 26, 2025

Codecov Report

❌ Patch coverage is 89.78495% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.398%. Comparing base (24a1eec) to head (352a044).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...Core/Tools/ViewCapture/SentryUIRedactBuilder.swift 89.673% 19 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #6292       +/-   ##
=============================================
- Coverage   85.458%   85.398%   -0.061%     
=============================================
  Files          451       451               
  Lines        27328     27428      +100     
  Branches     11917     11936       +19     
=============================================
+ Hits         23354     23423       +69     
+ Misses        3929      3721      -208     
- Partials        45       284      +239     
Files with missing lines Coverage Δ
...ft/Core/Tools/ViewCapture/SentryRedactRegion.swift 100.000% <ø> (ø)
...ore/Tools/ViewCapture/SentryViewPhotographer.swift 88.888% <100.000%> (+0.653%) ⬆️
...Core/Tools/ViewCapture/SentryUIRedactBuilder.swift 87.662% <89.673%> (-9.957%) ⬇️

... and 44 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 24a1eec...352a044. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 26, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1217.75 ms 1258.41 ms 40.66 ms
Size 23.75 KiB 1.01 MiB 1016.21 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
9264ee8 1233.15 ms 1250.02 ms 16.87 ms
07d7e83 1211.71 ms 1240.08 ms 28.37 ms
119ab1c 1226.79 ms 1254.55 ms 27.76 ms
66147d5 1234.45 ms 1268.45 ms 34.00 ms
fa9a4bb 1217.91 ms 1248.72 ms 30.81 ms
be882e4 1199.35 ms 1231.20 ms 31.86 ms
d7461dc 1233.69 ms 1255.29 ms 21.60 ms
f97a070 1218.88 ms 1253.12 ms 34.24 ms
339539a 1219.58 ms 1254.63 ms 35.05 ms
26f7b17 1218.47 ms 1253.82 ms 35.35 ms

App size

Revision Plain With Sentry Diff
9264ee8 23.75 KiB 913.09 KiB 889.34 KiB
07d7e83 23.75 KiB 913.27 KiB 889.52 KiB
119ab1c 23.75 KiB 993.70 KiB 969.95 KiB
66147d5 23.74 KiB 1.01 MiB 1008.77 KiB
fa9a4bb 23.75 KiB 986.91 KiB 963.16 KiB
be882e4 23.75 KiB 946.69 KiB 922.94 KiB
d7461dc 23.75 KiB 874.45 KiB 850.70 KiB
f97a070 23.75 KiB 858.68 KiB 834.93 KiB
339539a 23.75 KiB 968.24 KiB 944.50 KiB
26f7b17 23.75 KiB 960.93 KiB 937.19 KiB

Previous results on branch: philprime/fix-masking

Startup times

Revision Plain With Sentry Diff
bb49b7a 1229.51 ms 1260.31 ms 30.80 ms
9919db5 1230.88 ms 1262.00 ms 31.12 ms
0f2e562 1225.90 ms 1257.69 ms 31.80 ms
d70d220 1238.80 ms 1262.08 ms 23.29 ms
a126606 1218.71 ms 1248.26 ms 29.55 ms
8d1f930 1241.27 ms 1263.71 ms 22.44 ms
5a94e38 1190.59 ms 1244.60 ms 54.01 ms
5a4767b 1206.00 ms 1238.21 ms 32.21 ms
8d12b30 1222.56 ms 1251.98 ms 29.42 ms

App size

Revision Plain With Sentry Diff
bb49b7a 23.75 KiB 1014.06 KiB 990.32 KiB
9919db5 23.75 KiB 1.02 MiB 1017.54 KiB
0f2e562 23.75 KiB 1.02 MiB 1018.58 KiB
d70d220 23.75 KiB 994.81 KiB 971.06 KiB
a126606 23.75 KiB 1.02 MiB 1018.04 KiB
8d1f930 23.75 KiB 981.78 KiB 958.03 KiB
5a94e38 23.75 KiB 1.00 MiB 1006.18 KiB
5a4767b 23.75 KiB 1.01 MiB 1006.46 KiB
8d12b30 23.75 KiB 1.02 MiB 1019.75 KiB

@philprime philprime changed the title fix(session-replay): Ignore list background decoration view in redaction fix(session-replay): Extend masking and focus masking on sensitive information Oct 2, 2025
@philprime philprime marked this pull request as ready for review October 8, 2025 12:26
@philprime philprime marked this pull request as draft October 8, 2025 12:27
cursor[bot]

This comment was marked as outdated.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks way better than before. I gave the tests just a quick pass; they look good, but I found a couple of minor things to improve. Thanks @philprime for cleaning this up.

@cursor
Copy link

cursor bot commented Oct 23, 2025

Bug: Incorrect Transform Reference in Safari View Controller

In the assertSFSafariViewControllerRegions function for iOS 15/16, the assertion for placeholderRegion.transform incorrectly references toolbarRegion.transform.

Fix in Cursor Fix in Web

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@philprime philprime enabled auto-merge (squash) October 24, 2025 10:19
@philprime
Copy link
Member Author

FYI @philipphofmann @itaybre @noahsmartin this PR is done, but the CI is failing because snapshot tests of standard components are flaky, especially as we are using a Beta of Xcode 26.1

@philipphofmann
Copy link
Member

FYI @philipphofmann @itaybre @noahsmartin this PR is done, but the CI is failing because snapshot tests of standard components are flaky, especially as we are using a Beta of Xcode 26.1

@philprime what do you suggest to solve this problem?

@philprime
Copy link
Member Author

I am going to split this up into a PR stack with a PR per test-file so we can merge them individually instead of keeping this open forever

@philprime philprime force-pushed the philprime/fix-masking branch 2 times, most recently from fbdd1cb to 496198d Compare October 28, 2025 15:20
cursor[bot]

This comment was marked as outdated.

@philprime philprime force-pushed the philprime/fix-masking branch from 496198d to 6a5c478 Compare October 28, 2025 15:28
@philprime philprime merged commit 005f255 into main Oct 28, 2025
172 of 178 checks passed
@philprime philprime deleted the philprime/fix-masking branch October 28, 2025 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants