diff --git a/CHANGELOG.md b/CHANGELOG.md index 2076795a4db..ff253e3b527 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ ### Fixes - Session replay touch tracking race condition (#4548) +- Use `options.reportAccessibilityIdentifier` for Breadcrumbs and UIEvents (#4569) - Session replay transformed view masking (#4529) - Load integration from same binary (#4541) diff --git a/Samples/iOS-Swift/iOS-Swift/Tools/SentryExposure.h b/Samples/iOS-Swift/iOS-Swift/Tools/SentryExposure.h index dc681526390..ee873d0f89b 100644 --- a/Samples/iOS-Swift/iOS-Swift/Tools/SentryExposure.h +++ b/Samples/iOS-Swift/iOS-Swift/Tools/SentryExposure.h @@ -3,6 +3,7 @@ @interface SentryBreadcrumbTracker : NSObject -+ (NSDictionary *)extractDataFromView:(UIView *)view; ++ (NSDictionary *)extractDataFromView:(UIView *)view + withAccessibilityIdentifier:(BOOL)includeIdentifier; @end diff --git a/Samples/iOS-Swift/iOS-Swift/ViewControllers/InfoForBreadcrumbController.swift b/Samples/iOS-Swift/iOS-Swift/ViewControllers/InfoForBreadcrumbController.swift index 43452a9dd04..849a1c0aee9 100644 --- a/Samples/iOS-Swift/iOS-Swift/ViewControllers/InfoForBreadcrumbController.swift +++ b/Samples/iOS-Swift/iOS-Swift/ViewControllers/InfoForBreadcrumbController.swift @@ -14,8 +14,8 @@ class InfoForBreadcrumbController: UIViewController { @IBAction func buttonPressed(_ sender: Any) { guard let view = self.view, - let viewInfo = SentryBreadcrumbTracker.extractData(from: view), - let buttonInfo = SentryBreadcrumbTracker.extractData(from: button) + let viewInfo = SentryBreadcrumbTracker.extractData(from: view, withAccessibilityIdentifier: true), + let buttonInfo = SentryBreadcrumbTracker.extractData(from: button, withAccessibilityIdentifier: true) else { label?.text = "ERROR" return diff --git a/Sources/Sentry/SentryAutoBreadcrumbTrackingIntegration.m b/Sources/Sentry/SentryAutoBreadcrumbTrackingIntegration.m index d04f1e185a7..31e73154326 100644 --- a/Sources/Sentry/SentryAutoBreadcrumbTrackingIntegration.m +++ b/Sources/Sentry/SentryAutoBreadcrumbTrackingIntegration.m @@ -27,16 +27,20 @@ - (BOOL)installWithOptions:(SentryOptions *)options return NO; } - [self installWithOptions:options - breadcrumbTracker:[[SentryBreadcrumbTracker alloc] init] #if TARGET_OS_IOS && SENTRY_HAS_UIKIT + [self installWithOptions:options + breadcrumbTracker:[[SentryBreadcrumbTracker alloc] initReportAccessibilityIdentifier: + options.reportAccessibilityIdentifier] systemEventBreadcrumbs: [[SentrySystemEventBreadcrumbs alloc] initWithFileManager:[SentryDependencyContainer sharedInstance].fileManager andNotificationCenterWrapper:[SentryDependencyContainer sharedInstance] - .notificationCenterWrapper] + .notificationCenterWrapper]]; +#else + [self installWithOptions:options + breadcrumbTracker:[[SentryBreadcrumbTracker alloc] + initReportAccessibilityIdentifier:false]]; #endif // TARGET_OS_IOS && SENTRY_HAS_UIKIT - ]; return YES; } diff --git a/Sources/Sentry/SentryBreadcrumbTracker.m b/Sources/Sentry/SentryBreadcrumbTracker.m index eae691cb2d3..455e32bd808 100644 --- a/Sources/Sentry/SentryBreadcrumbTracker.m +++ b/Sources/Sentry/SentryBreadcrumbTracker.m @@ -34,7 +34,9 @@ @interface SentryBreadcrumbTracker () @end -@implementation SentryBreadcrumbTracker +@implementation SentryBreadcrumbTracker { + BOOL _reportAccessibilityIdentifier; +} #if SENTRY_HAS_REACHABILITY - (void)dealloc @@ -43,6 +45,14 @@ - (void)dealloc } #endif // !TARGET_OS_WATCH +- (instancetype)initReportAccessibilityIdentifier:(BOOL)report +{ + if (self = [super init]) { + _reportAccessibilityIdentifier = report; + } + return self; +} + - (void)startWithDelegate:(id)delegate { _delegate = delegate; @@ -211,7 +221,9 @@ - (void)swizzleSendAction NSDictionary *data = nil; for (UITouch *touch in event.allTouches) { if (touch.phase == UITouchPhaseCancelled || touch.phase == UITouchPhaseEnded) { - data = [SentryBreadcrumbTracker extractDataFromView:touch.view]; + data = [SentryBreadcrumbTracker + extractDataFromView:touch.view + withAccessibilityIdentifier:self->_reportAccessibilityIdentifier]; } } @@ -264,6 +276,7 @@ - (void)swizzleViewDidAppear } + (NSDictionary *)extractDataFromView:(UIView *)view + withAccessibilityIdentifier:(BOOL)includeIdentifier { NSMutableDictionary *result = @{ @"view" : [NSString stringWithFormat:@"%@", view] }.mutableCopy; @@ -272,7 +285,8 @@ + (NSDictionary *)extractDataFromView:(UIView *)view [result setValue:[NSNumber numberWithInteger:view.tag] forKey:@"tag"]; } - if (view.accessibilityIdentifier && ![view.accessibilityIdentifier isEqualToString:@""]) { + if (includeIdentifier && view.accessibilityIdentifier + && ![view.accessibilityIdentifier isEqualToString:@""]) { [result setValue:view.accessibilityIdentifier forKey:@"accessibilityIdentifier"]; } diff --git a/Sources/Sentry/SentryUIEventTracker.m b/Sources/Sentry/SentryUIEventTracker.m index 2eb0fa780ab..4218250132c 100644 --- a/Sources/Sentry/SentryUIEventTracker.m +++ b/Sources/Sentry/SentryUIEventTracker.m @@ -19,12 +19,16 @@ @interface SentryUIEventTracker () @end -@implementation SentryUIEventTracker +@implementation SentryUIEventTracker { + BOOL _reportAccessibilityIdentifier; +} - (instancetype)initWithMode:(id)mode + reportAccessibilityIdentifier:(BOOL)report { if (self = [super init]) { self.uiEventTrackerMode = mode; + _reportAccessibilityIdentifier = report; } return self; } @@ -73,7 +77,7 @@ - (void)sendActionCallback:(NSString *)action NSString *operation = [self getOperation:sender]; NSString *accessibilityIdentifier = nil; - if ([[sender class] isSubclassOfClass:[UIView class]]) { + if (_reportAccessibilityIdentifier && [[sender class] isSubclassOfClass:[UIView class]]) { UIView *view = sender; accessibilityIdentifier = view.accessibilityIdentifier; } diff --git a/Sources/Sentry/SentryUIEventTrackerTransactionMode.m b/Sources/Sentry/SentryUIEventTrackerTransactionMode.m index d57091374a5..f96838f8918 100644 --- a/Sources/Sentry/SentryUIEventTrackerTransactionMode.m +++ b/Sources/Sentry/SentryUIEventTrackerTransactionMode.m @@ -36,7 +36,7 @@ - (instancetype)initWithIdleTimeout:(NSTimeInterval)idleTimeout - (void)handleUIEvent:(NSString *)action operation:(NSString *)operation - accessibilityIdentifier:(NSString *)accessibilityIdentifier + accessibilityIdentifier:(nullable NSString *)accessibilityIdentifier { // There might be more active transactions stored, but only the last one might still be diff --git a/Sources/Sentry/SentryUIEventTrackingIntegration.m b/Sources/Sentry/SentryUIEventTrackingIntegration.m index e2e6e1e076c..0ce4a88b07e 100644 --- a/Sources/Sentry/SentryUIEventTrackingIntegration.m +++ b/Sources/Sentry/SentryUIEventTrackingIntegration.m @@ -26,7 +26,9 @@ - (BOOL)installWithOptions:(SentryOptions *)options SentryUIEventTrackerTransactionMode *mode = [[SentryUIEventTrackerTransactionMode alloc] initWithIdleTimeout:options.idleTimeout]; - self.uiEventTracker = [[SentryUIEventTracker alloc] initWithMode:mode]; + self.uiEventTracker = + [[SentryUIEventTracker alloc] initWithMode:mode + reportAccessibilityIdentifier:options.reportAccessibilityIdentifier]; [self.uiEventTracker start]; diff --git a/Sources/Sentry/include/SentryBreadcrumbTracker.h b/Sources/Sentry/include/SentryBreadcrumbTracker.h index 907e57ca677..6b10dea36a4 100644 --- a/Sources/Sentry/include/SentryBreadcrumbTracker.h +++ b/Sources/Sentry/include/SentryBreadcrumbTracker.h @@ -6,6 +6,10 @@ NS_ASSUME_NONNULL_BEGIN @interface SentryBreadcrumbTracker : NSObject +SENTRY_NO_INIT + +- (instancetype)initReportAccessibilityIdentifier:(BOOL)report; + - (void)startWithDelegate:(id)delegate; #if SENTRY_HAS_UIKIT - (void)startSwizzle; diff --git a/Sources/Sentry/include/SentryUIEventTracker.h b/Sources/Sentry/include/SentryUIEventTracker.h index 456315c9228..38970c1b713 100644 --- a/Sources/Sentry/include/SentryUIEventTracker.h +++ b/Sources/Sentry/include/SentryUIEventTracker.h @@ -10,7 +10,8 @@ NS_ASSUME_NONNULL_BEGIN @interface SentryUIEventTracker : NSObject SENTRY_NO_INIT -- (instancetype)initWithMode:(id)mode; +- (instancetype)initWithMode:(id)mode + reportAccessibilityIdentifier:(BOOL)report; - (void)start; - (void)stop; diff --git a/Sources/Sentry/include/SentryUIEventTrackerMode.h b/Sources/Sentry/include/SentryUIEventTrackerMode.h index 7c0f4f9c29c..637311fecf4 100644 --- a/Sources/Sentry/include/SentryUIEventTrackerMode.h +++ b/Sources/Sentry/include/SentryUIEventTrackerMode.h @@ -8,7 +8,7 @@ NS_ASSUME_NONNULL_BEGIN - (void)handleUIEvent:(NSString *)action operation:(NSString *)operation - accessibilityIdentifier:(NSString *)accessibilityIdentifier; + accessibilityIdentifier:(nullable NSString *)accessibilityIdentifier; @end diff --git a/Tests/SentryTests/Integrations/Breadcrumbs/SentryBreadcrumbTrackerTests.swift b/Tests/SentryTests/Integrations/Breadcrumbs/SentryBreadcrumbTrackerTests.swift index 369cb0ce133..4e166c4f7d1 100644 --- a/Tests/SentryTests/Integrations/Breadcrumbs/SentryBreadcrumbTrackerTests.swift +++ b/Tests/SentryTests/Integrations/Breadcrumbs/SentryBreadcrumbTrackerTests.swift @@ -2,6 +2,12 @@ import SentryTestUtils import XCTest +extension SentryBreadcrumbTracker { + override convenience init() { + self.init(reportAccessibilityIdentifier: true) + } +} + class SentryBreadcrumbTrackerTests: XCTestCase { private var delegate: SentryBreadcrumbTestDelegate! @@ -21,15 +27,15 @@ class SentryBreadcrumbTrackerTests: XCTestCase { func testStopRemovesSwizzleSendAction() { let sut = SentryBreadcrumbTracker() - + sut.start(with: delegate) sut.startSwizzle() sut.stop() - + let dict = Dynamic(SentryDependencyContainer.sharedInstance().swizzleWrapper).swizzleSendActionCallbacks().asDictionary XCTAssertEqual(0, dict?.count) } - + func testNetworkConnectivityChangeBreadcrumbs() throws { let testReachability = TestSentryReachability() @@ -38,11 +44,11 @@ class SentryBreadcrumbTrackerTests: XCTestCase { let sut = SentryBreadcrumbTracker() sut.start(with: delegate) let states = [SentryConnectivityCellular, - SentryConnectivityWiFi, - SentryConnectivityNone, - SentryConnectivityWiFi, - SentryConnectivityCellular, - SentryConnectivityWiFi + SentryConnectivityWiFi, + SentryConnectivityNone, + SentryConnectivityWiFi, + SentryConnectivityCellular, + SentryConnectivityWiFi ] states.forEach { testReachability.setReachabilityState(state: $0) @@ -64,7 +70,7 @@ class SentryBreadcrumbTrackerTests: XCTestCase { sut.start(with: delegate) testReachability.setReachabilityState(state: SentryConnectivityCellular) sut.stop() - + guard let breadcrumb = delegate.addCrumbInvocations.invocations.dropFirst().first else { XCTFail("No connectivity breadcrumb") return @@ -80,7 +86,7 @@ class SentryBreadcrumbTrackerTests: XCTestCase { XCTAssertEqual(payload["category"] as? String, "device.connectivity") XCTAssertEqual(payloadData["state"] as? String, "cellular") } - + func testSwizzlingStarted_ViewControllerAppears_AddsUILifeCycleBreadcrumb() throws { let testReachability = TestSentryReachability() @@ -99,7 +105,7 @@ class SentryBreadcrumbTrackerTests: XCTestCase { let sut = SentryBreadcrumbTracker() sut.start(with: delegate) sut.startSwizzle() - + // Using UINavigationController as a parent doesn't work on tvOS 17.0 // for an unknown reason. Therefore, we manually set the parent. class ParentUIViewController: UIViewController { @@ -113,15 +119,15 @@ class SentryBreadcrumbTrackerTests: XCTestCase { print("delegate: \(String(describing: delegate))") print("tracker: \(sut); SentryBreadcrumbTracker.delegate: \(String(describing: Dynamic(sut).delegate.asObject))") viewController.viewDidAppear(false) - + let crumbs = delegate.addCrumbInvocations.invocations - + // one breadcrumb for starting the tracker, one for the first reachability breadcrumb and one final one for the swizzled viewDidAppear guard crumbs.count == 2 else { XCTFail("Expected exactly 2 breadcrumbs, got: \(crumbs)") return } - + let lifeCycleCrumb = try XCTUnwrap(crumbs.element(at: 1)) XCTAssertEqual("navigation", lifeCycleCrumb.type) XCTAssertEqual("ui.lifecycle", lifeCycleCrumb.category) @@ -221,7 +227,7 @@ class SentryBreadcrumbTrackerTests: XCTestCase { XCTFail("No touch breadcrumb") return } - + let result = try XCTUnwrap(sut.convert(from: crumb) as? SentryRRWebBreadcrumbEvent) let crumbData = try XCTUnwrap(result.data) let payload = try XCTUnwrap(crumbData["payload"] as? [String: Any]) @@ -230,6 +236,78 @@ class SentryBreadcrumbTrackerTests: XCTestCase { XCTAssertEqual(payload["message"] as? String, "methodPressed:") } + func testTouchBreadcrumb_DontReportAccessibilityIdentifier() throws { + let scope = Scope() + let client = TestClient(options: Options()) + let hub = TestHub(client: client, andScope: scope) + SentrySDK.setCurrentHub(hub) + + let swizzlingWrapper = TestSentrySwizzleWrapper() + SentryDependencyContainer.sharedInstance().swizzleWrapper = swizzlingWrapper + + let tracker = SentryBreadcrumbTracker(reportAccessibilityIdentifier: false) + tracker.start(with: delegate) + tracker.startSwizzle() + + let button = UIButton() + button.accessibilityIdentifier = "TestAccessibilityIdentifier" + + swizzlingWrapper.execute(action: "methodPressed:", target: self, sender: button, event: TestEvent(touchedView: button)) + + guard let crumb = delegate.addCrumbInvocations.invocations.first(where: { $0.category == "touch" }) else { + XCTFail("No touch breadcrumb") + return + } + + let crumbData = try XCTUnwrap(crumb.data) + + XCTAssertNil(crumbData["accessibilityIdentifier"]) + } + + func testTouchBreadcrumb_ReportAccessibilityIdentifier() throws { + let scope = Scope() + let client = TestClient(options: Options()) + let hub = TestHub(client: client, andScope: scope) + SentrySDK.setCurrentHub(hub) + + let swizzlingWrapper = TestSentrySwizzleWrapper() + SentryDependencyContainer.sharedInstance().swizzleWrapper = swizzlingWrapper + + let tracker = SentryBreadcrumbTracker(reportAccessibilityIdentifier: true) + tracker.start(with: delegate) + tracker.startSwizzle() + + let button = UIButton() + button.accessibilityIdentifier = "TestAccessibilityIdentifier" + + swizzlingWrapper.execute(action: "methodPressed:", target: self, sender: button, event: TestEvent(touchedView: button)) + + let crumb = try XCTUnwrap(delegate.addCrumbInvocations.invocations.first(where: { $0.category == "touch" })) + let crumbData = try XCTUnwrap(crumb.data) + + XCTAssertEqual(crumbData["accessibilityIdentifier"] as? String, "TestAccessibilityIdentifier") + } + + private class TestEvent: UIEvent { + let touchedView: UIView? + class TestEndTouch: UITouch { + let touchedView: UIView? + init(touchedView: UIView?) { + self.touchedView = touchedView + } + override var phase: UITouch.Phase { .ended } + override var view: UIView? { touchedView } + } + + init(touchedView: UIView?) { + self.touchedView = touchedView + } + + override var allTouches: Set { [ + TestEndTouch(touchedView: touchedView) + ] } + } + #endif - + } diff --git a/Tests/SentryTests/Integrations/UIEvents/SentryUIEventTrackerTests.swift b/Tests/SentryTests/Integrations/UIEvents/SentryUIEventTrackerTests.swift index 24cb5b2f156..baaf1b74d3d 100644 --- a/Tests/SentryTests/Integrations/UIEvents/SentryUIEventTrackerTests.swift +++ b/Tests/SentryTests/Integrations/UIEvents/SentryUIEventTrackerTests.swift @@ -20,8 +20,8 @@ class SentryUIEventTrackerTests: XCTestCase { uiEventTrackerMode = SentryUIEventTrackerTransactionMode(idleTimeout: 3.0) } - func getSut() -> SentryUIEventTracker { - return SentryUIEventTracker(mode: uiEventTrackerMode) + func getSut(reportAccessibilityIdentifier: Bool = true) -> SentryUIEventTracker { + return SentryUIEventTracker(mode: uiEventTrackerMode, reportAccessibilityIdentifier: reportAccessibilityIdentifier) } } @@ -115,6 +115,23 @@ class SentryUIEventTrackerTests: XCTestCase { }) } + func test_UIViewWithAccessibilityIdentifier_DontReportAccessibilityIdentifier() throws { + fixture.swizzleWrapper.removeAllCallbacks() + + let button = fixture.button + button.accessibilityIdentifier = accessibilityIdentifier + + let sut = fixture.getSut(reportAccessibilityIdentifier: false) + sut.start() + + callExecuteAction(action: action, target: fixture.target, sender: button, event: TestUIEvent()) + + let span = try! XCTUnwrap(SentrySDK.span as? SentryTracer) + XCTAssertFalse(span.tags.contains { + $0.key == "accessibilityIdentifier" + }) + } + func test_SubclassOfUIButton_CreatesTransaction() { callExecuteAction(action: action, target: fixture.target, sender: TestUIButton(), event: TestUIEvent())