-
Notifications
You must be signed in to change notification settings - Fork 54
[CHORE][iOS] Fix Session Replay Text Extraction on RN 0.84 #1112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| /* | ||
| * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. | ||
| * This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| * Copyright 2016-Present Datadog, Inc. | ||
| */ | ||
| #import <Foundation/Foundation.h> | ||
| #import "RCTTextPropertiesWrapper.h" | ||
|
|
||
| @class RCTUIManager; | ||
|
|
||
| @interface RCTTextExtractor : NSObject | ||
|
|
||
| - (nullable RCTTextPropertiesWrapper*)tryToExtractTextPropertiesFromView:(UIView* _Nonnull)view | ||
| withUIManager:(RCTUIManager* _Nonnull)uiManager; | ||
|
|
||
| - (BOOL)isRCTTextView:(UIView* _Nonnull)view; | ||
|
|
||
| @end |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,127 @@ | ||
| /* | ||
| * Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. | ||
| * This product includes software developed at Datadog (https://www.datadoghq.com/). | ||
| * Copyright 2016-Present Datadog, Inc. | ||
| */ | ||
|
|
||
| #import "RCTTextExtractor.h" | ||
|
|
||
| #if !RCT_NEW_ARCH_ENABLED | ||
| #import <React/RCTBridge.h> | ||
| #import <React/RCTUIManager.h> | ||
| #import <React/RCTTextView.h> | ||
| #import <React/RCTTextShadowView.h> | ||
| #import <React/RCTRawTextShadowView.h> | ||
| #import <React/RCTVirtualTextShadowView.h> | ||
| #endif | ||
|
|
||
| @implementation RCTTextExtractor | ||
|
|
||
| /** | ||
| * Extracts the text properties from the given UIView when using the old Paper architecture. | ||
| * Returns nil when using new architecture or if the view is not a RCTTextView. | ||
| */ | ||
| - (nullable RCTTextPropertiesWrapper*)tryToExtractTextPropertiesFromView:(UIView *)view | ||
| withUIManager:(RCTUIManager *)uiManager { | ||
| #if !RCT_NEW_ARCH_ENABLED | ||
| if (![view isKindOfClass:[RCTTextView class]]) { | ||
| return nil; | ||
| } | ||
|
|
||
| RCTTextView* textView = (RCTTextView*)view; | ||
| NSNumber* tag = textView.reactTag; | ||
|
|
||
| __block RCTTextShadowView* shadowView = nil; | ||
| NSTimeInterval timeout = 0.2; | ||
| dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); | ||
|
|
||
| // We need to access the shadow view from the UIManager queue, but we're currently on the main thread. | ||
| // Calling `.sync` from the main thread to the UIManager queue is unsafe, because the UIManager queue | ||
| // may already be executing a layout operation that in turn requires the main thread (e.g. measuring a native view). | ||
| // That would create a circular dependency and deadlock the app. | ||
| // To avoid this, we dispatch the work asynchronously to the UIManager queue and wait with a timeout. | ||
| // This ensures we block only if absolutely necessary, and can fail gracefully if the queue is busy. | ||
| dispatch_async(uiManager.methodQueue, ^{ | ||
| shadowView = (RCTTextShadowView*)[uiManager shadowViewForReactTag:tag]; | ||
| dispatch_semaphore_signal(semaphore); | ||
| }); | ||
|
|
||
| dispatch_time_t waitTimeout = dispatch_time(DISPATCH_TIME_NOW, (int64_t)(timeout * NSEC_PER_SEC)); | ||
| long waitResult = dispatch_semaphore_wait(semaphore, waitTimeout); | ||
|
|
||
| if (waitResult != 0) { // timeout | ||
| return nil; | ||
| } | ||
|
|
||
| if (shadowView == nil || ![shadowView isKindOfClass:[RCTTextShadowView class]]) { | ||
| return nil; | ||
| } | ||
|
|
||
| RCTTextPropertiesWrapper* textProperties = [[RCTTextPropertiesWrapper alloc] init]; | ||
|
|
||
| // Extract text from subviews | ||
| NSString* text = [self tryToExtractTextFromSubViews:shadowView.reactSubviews]; | ||
| if (text != nil) { | ||
| textProperties.text = text; | ||
| } | ||
|
|
||
| // Extract text attributes | ||
| if (shadowView.textAttributes.foregroundColor != nil) { | ||
| textProperties.foregroundColor = shadowView.textAttributes.foregroundColor; | ||
| } | ||
|
|
||
| textProperties.alignment = shadowView.textAttributes.alignment; | ||
| textProperties.fontSize = shadowView.textAttributes.fontSize; | ||
| textProperties.contentRect = shadowView.layoutMetrics.contentFrame; | ||
|
|
||
| return textProperties; | ||
| #else | ||
| return nil; | ||
| #endif | ||
| } | ||
|
|
||
| #if !RCT_NEW_ARCH_ENABLED | ||
| - (nullable NSString*)tryToExtractTextFromSubViews:(NSArray<RCTShadowView*>*)subviews { | ||
| if (subviews == nil) { | ||
| return nil; | ||
| } | ||
|
|
||
| NSMutableArray<NSString*>* textParts = [NSMutableArray array]; | ||
|
|
||
| for (RCTShadowView* subview in subviews) { | ||
| if ([subview isKindOfClass:[RCTRawTextShadowView class]]) { | ||
| RCTRawTextShadowView* rawTextView = (RCTRawTextShadowView*)subview; | ||
| if (rawTextView.text != nil) { | ||
| [textParts addObject:rawTextView.text]; | ||
| } | ||
| } else if ([subview isKindOfClass:[RCTVirtualTextShadowView class]]) { | ||
| // We recursively get all subviews for nested Text components | ||
| RCTVirtualTextShadowView* virtualTextView = (RCTVirtualTextShadowView*)subview; | ||
| NSString* nestedText = [self tryToExtractTextFromSubViews:virtualTextView.reactSubviews]; | ||
| if (nestedText != nil) { | ||
| [textParts addObject:nestedText]; | ||
| } | ||
| } | ||
| } | ||
|
|
||
| if (textParts.count == 0) { | ||
| return nil; | ||
| } | ||
|
|
||
| return [textParts componentsJoinedByString:@""]; | ||
| } | ||
| #endif | ||
|
|
||
| /** | ||
| * Checks if the given view is an RCTTextView. | ||
| * Returns NO when using new architecture or if the view is not a RCTTextView. | ||
| */ | ||
| - (BOOL)isRCTTextView:(UIView *)view { | ||
| #if !RCT_NEW_ARCH_ENABLED | ||
| return [view isKindOfClass:[RCTTextView class]]; | ||
| #else | ||
| return NO; | ||
| #endif | ||
| } | ||
|
|
||
| @end | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,10 +20,12 @@ internal class RCTTextViewRecorder: SessionReplayNodeRecorder { | |
|
|
||
| internal let uiManager: RCTUIManager | ||
| internal let fabricWrapper: RCTFabricWrapper | ||
| private let textExtractor: RCTTextExtractor | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We don't need it at the moment, but this could be easily extracted to be initialized by a new paramter passed to |
||
|
|
||
| internal init(uiManager: RCTUIManager, fabricWrapper: RCTFabricWrapper) { | ||
| self.uiManager = uiManager | ||
| self.fabricWrapper = fabricWrapper | ||
| self.textExtractor = RCTTextExtractor() | ||
| } | ||
|
|
||
| public func semantics( | ||
|
|
@@ -33,9 +35,11 @@ internal class RCTTextViewRecorder: SessionReplayNodeRecorder { | |
| ) -> SessionReplayNodeSemantics? { | ||
| guard | ||
| let textProperties = fabricWrapper.tryToExtractTextProperties(from: view) | ||
| ?? tryToExtractTextProperties(view: view) | ||
| ?? textExtractor.tryToExtractTextProperties(from: view, with: uiManager) | ||
| else { | ||
| return view is RCTTextView ? SessionReplayInvisibleElement.constant : nil | ||
| // Check if this is an RCTTextView that we couldn't extract text from | ||
| // This check is done in Objective-C to avoid compile-time dependency on RCTTextView | ||
| return textExtractor.isRCTTextView(view) ? SessionReplayInvisibleElement.constant : nil | ||
| } | ||
|
|
||
| let builder = RCTTextViewWireframesBuilder( | ||
|
|
@@ -56,73 +60,6 @@ internal class RCTTextViewRecorder: SessionReplayNodeRecorder { | |
| ]) | ||
| } | ||
|
|
||
| internal func tryToExtractTextFromSubViews( | ||
| subviews: [RCTShadowView]? | ||
| ) -> String? { | ||
| guard let subviews = subviews else { | ||
| return nil | ||
| } | ||
|
|
||
| return subviews.compactMap { subview in | ||
| if let sub = subview as? RCTRawTextShadowView { | ||
| return sub.text | ||
| } | ||
| if let sub = subview as? RCTVirtualTextShadowView { | ||
| // We recursively get all subviews for nested Text components | ||
| return tryToExtractTextFromSubViews(subviews: sub.reactSubviews()) | ||
| } | ||
| return nil | ||
| }.joined() | ||
| } | ||
|
|
||
| private func tryToExtractTextProperties(view: UIView) -> RCTTextPropertiesWrapper? { | ||
| guard let textView = view as? RCTTextView else { | ||
| return nil | ||
| } | ||
|
|
||
| var shadowView: RCTTextShadowView? = nil | ||
| let tag = textView.reactTag | ||
|
|
||
| let timeout: TimeInterval = 0.2 | ||
| let semaphore = DispatchSemaphore(value: 0) | ||
|
|
||
| // We need to access the shadow view from the UIManager queue, but we're currently on the main thread. | ||
| // Calling `.sync` from the main thread to the UIManager queue is unsafe, because the UIManager queue | ||
| // may already be executing a layout operation that in turn requires the main thread (e.g. measuring a native view). | ||
| // That would create a circular dependency and deadlock the app. | ||
| // To avoid this, we dispatch the work asynchronously to the UIManager queue and wait with a timeout. | ||
| // This ensures we block only if absolutely necessary, and can fail gracefully if the queue is busy. | ||
| RCTGetUIManagerQueue().async { | ||
| shadowView = self.uiManager.shadowView(forReactTag: tag) as? RCTTextShadowView | ||
| semaphore.signal() | ||
| } | ||
|
|
||
| let waitResult = semaphore.wait(timeout: .now() + timeout) | ||
| if waitResult == .timedOut { | ||
| return nil | ||
| } | ||
|
|
||
| guard let shadow = shadowView else { | ||
| return nil | ||
| } | ||
|
|
||
| let textProperties = RCTTextPropertiesWrapper() | ||
|
|
||
| // TODO: RUM-2173 check performance is ok | ||
| if let text = tryToExtractTextFromSubViews(subviews: shadow.reactSubviews()) { | ||
| textProperties.text = text | ||
| } | ||
|
|
||
| if let foregroundColor = shadow.textAttributes.foregroundColor { | ||
| textProperties.foregroundColor = foregroundColor | ||
| } | ||
|
|
||
| textProperties.alignment = shadow.textAttributes.alignment | ||
| textProperties.fontSize = shadow.textAttributes.fontSize | ||
| textProperties.contentRect = shadow.contentFrame | ||
|
|
||
| return textProperties | ||
| } | ||
|
Comment on lines
-59
to
-125
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not against extracting this logic outside of this class but I don't think we should remove the swift code, would this not work ? then in swfit:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would work to expose isNewArchEnabled (I think), but the main problem here are the specific .h imports, which as far as I know are not possible to include on Swift conditionally (we can only do that with full modules, not with specific .h files). I've edited the PR description to also mention this. |
||
| } | ||
|
|
||
| internal struct RCTTextViewWireframesBuilder: SessionReplayNodeWireframesBuilder { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we need this check to decide if we return an invisible view or not, and this relies on RCTTextView we need to have this inside an obj C as well.