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 96.23656% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.320%. Comparing base (707c222) to head (5a92a94).
✅ All tests successful. No failed tests found.

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

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #6292       +/-   ##
=============================================
+ Coverage   85.899%   86.320%   +0.420%     
=============================================
  Files          451       451               
  Lines        27482     27588      +106     
  Branches     11970     11991       +21     
=============================================
+ Hits         23607     23814      +207     
+ Misses        3826      3730       -96     
+ Partials        49        44        -5     
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 95.454% <96.195%> (-0.260%) ⬇️

... and 55 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 707c222...5a92a94. Read the comment docs.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 26, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1214.09 ms 1243.47 ms 29.38 ms
Size 23.75 KiB 1.02 MiB 1018.58 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
afaa522 1234.71 ms 1256.19 ms 21.48 ms
670b474 1225.33 ms 1259.59 ms 34.26 ms
162cd7f 1230.59 ms 1256.76 ms 26.16 ms
9add417 1224.33 ms 1243.06 ms 18.73 ms
4c719e2 1206.92 ms 1237.45 ms 30.53 ms
ac4739e 1236.55 ms 1258.89 ms 22.34 ms
3133d0e 1237.86 ms 1262.87 ms 25.01 ms
51f74d7 1236.31 ms 1247.43 ms 11.12 ms
85a741b 1217.02 ms 1239.27 ms 22.25 ms
2c59847 1216.15 ms 1253.80 ms 37.65 ms

App size

Revision Plain With Sentry Diff
afaa522 23.74 KiB 996.91 KiB 973.17 KiB
670b474 23.75 KiB 974.89 KiB 951.14 KiB
162cd7f 23.75 KiB 908.39 KiB 884.64 KiB
9add417 23.75 KiB 908.40 KiB 884.65 KiB
4c719e2 23.75 KiB 912.77 KiB 889.02 KiB
ac4739e 23.75 KiB 872.67 KiB 848.92 KiB
3133d0e 23.74 KiB 976.79 KiB 953.04 KiB
51f74d7 23.74 KiB 874.08 KiB 850.34 KiB
85a741b 23.75 KiB 959.44 KiB 935.69 KiB
2c59847 23.75 KiB 1.00 MiB 1005.08 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.

@philprime philprime marked this pull request as ready for review October 21, 2025 09:12
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

… and add new snapshot tests

- Introduced a mechanism to manage fake views requiring manual disposal to prevent crashes during deallocation.
- Updated `tearDown` method to remove fake views from superviews while retaining references to avoid unsafe deallocations.
- Added multiple snapshot tests to validate redaction behavior for various view types, including UIWebView and AVPlayerViewController, ensuring proper handling of sensitive content.
- Included documentation to clarify the purpose and usage of the new fake view management approach.
cursor[bot]

This comment was marked as outdated.

- Updated `containsIgnoreClass` method to accept a string representation of class identifiers instead of `AnyClass`.
- Adjusted related logic in `shouldIgnore` and other methods to maintain functionality with the new string-based approach.
- Modified tests to reflect changes in the method signature, ensuring proper validation of ignored classes.
…g O(1) lookups for both constrained and unconstrained classes.

- Updated the `SentryUIRedactBuilder` to initialize class identifiers using `AnyClass` for both class and layer, enhancing type safety.
- Modified methods to utilize the new class identifier structure, ensuring consistent behavior across redaction logic.
- Updated tests to reflect changes in method signatures and improve validation of class handling.
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

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.

3 participants