From cb9e1fe8bee99f1213a9e8ed790510f3cae414d6 Mon Sep 17 00:00:00 2001 From: Nick Gerleman Date: Fri, 11 Oct 2024 05:11:06 -0700 Subject: [PATCH] Fix cursor moving while typing quickly and autocorrection triggered in controlled single line TextInput on iOS (New Arch) (#46970) Summary: This one is a bit of a doozy... During auto-correct in UITextField (used for single line TextInput) iOS will mutate the buffer in two parts, non-atomically. After the first part, after iOS triggers `textFieldDidChange`, selection is in the wrong position. If we set full new AttributedText at this point, we propagate the incorrect cursor position, and it is never restored. In the common case, where we are not mutating text in the controlled component, we shouldn't need to be setting AttributedString in the first place, and we do have an equality comparison there currently. But it is defeated because attributes are not identical. There are a few sources of that: 1. NSParagraphStyle is present in backing input, but not the AttributedString we are setting. 2. Backing text has an NSShadow with no color (does not render) not in the AttributedText 3. Event emitter attributes change on each update, and new text does not inherit the attributes. The first two are part of the backing input `typingAttributes`, even if we set a dictionary without them. To solve for them, we make attribute comparison insensitive to the attribute values in a default initialized control. There is code around here fully falling back to attribute insensitive comparison, which we would ideally fix to instead role into this "effective" attribute comparison. The event emitter attributes being misaligned is a real problem. We fix in a couple ways. 1. We treat the attribute values as equal if the backing event emitter is the same 2. We preserve attributes that we already set and want to expand as part of typingAttributes 3. We set paragraph level event emitter as a default attribute so the first typed character receives it After these fixes, scenario in https://github.com/facebook/react-native-website/pull/4247 no longer repros in new arch. Typing in debug build also subjectively seems faster? (we are not doing second invalidation of the control on every keypress). Changes which do mutate content may be susceptible to the same style of issue, though on web/`react-dom` in Chrome, this seems to not try to preserve selection at all if the selection is uncontrolled, so this seems like less of an issue. I haven't yet looked at old arch, but my guess is we have similar issues there, and could be fixed in similar ways (though, we've been trying to avoid changing it as much as possible, and 0.76+ has new arch as default, so not sure if worth fixing in old impl as well if this is very long running issue). Changelog: [iOS][Fixed] - Fix cursor moving in iOS controlled single line TextInput on Autocorrection (New Arch) Differential Revision: D64121570 --- .../RCTBackedTextInputViewProtocol.h | 1 + .../TextInput/RCTTextInputComponentView.mm | 152 +++++++++++++++++- .../RCTAttributedTextUtils.h | 2 +- .../RCTAttributedTextUtils.mm | 32 +++- .../RCTTextPrimitivesConversions.h | 12 +- 5 files changed, 180 insertions(+), 19 deletions(-) diff --git a/packages/react-native/Libraries/Text/TextInput/RCTBackedTextInputViewProtocol.h b/packages/react-native/Libraries/Text/TextInput/RCTBackedTextInputViewProtocol.h index 7aa5bcd54e7b36..cc510133cc614f 100644 --- a/packages/react-native/Libraries/Text/TextInput/RCTBackedTextInputViewProtocol.h +++ b/packages/react-native/Libraries/Text/TextInput/RCTBackedTextInputViewProtocol.h @@ -35,6 +35,7 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, assign, readonly) CGFloat zoomScale; @property (nonatomic, assign, readonly) CGPoint contentOffset; @property (nonatomic, assign, readonly) UIEdgeInsets contentInset; +@property (nullable, nonatomic, copy) NSDictionary *typingAttributes; // This protocol disallows direct access to `selectedTextRange` property because // unwise usage of it can break the `delegate` behavior. So, we always have to diff --git a/packages/react-native/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm b/packages/react-native/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm index 8c532d85502bc1..05757cd5304ad0 100644 --- a/packages/react-native/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm @@ -9,6 +9,7 @@ #import #import +#import #import #import @@ -61,6 +62,13 @@ @implementation RCTTextInputComponentView { */ BOOL _comingFromJS; BOOL _didMoveToWindow; + + /* + * Newly initialized default typing attributes contain a no-op NSParagraphStyle and NSShadow. These cause inequality + * between the AttributedString backing the input and those generated from state. We store these attributes to make + * later comparison insensitive to them. + */ + NSDictionary *_defaultTypingAttributes; } #pragma mark - UIView overrides @@ -79,11 +87,27 @@ - (instancetype)initWithFrame:(CGRect)frame [self addSubview:_backedTextInputView]; [self initializeReturnKeyType]; + + _defaultTypingAttributes = [_backedTextInputView.typingAttributes copy]; } return self; } +- (void)updateEventEmitter:(const EventEmitter::Shared &)eventEmitter +{ + [super updateEventEmitter:eventEmitter]; + + NSMutableDictionary *defaultAttributes = + [_backedTextInputView.defaultTextAttributes mutableCopy]; + + RCTWeakEventEmitterWrapper *eventEmitterWrapper = [RCTWeakEventEmitterWrapper new]; + eventEmitterWrapper.eventEmitter = _eventEmitter; + defaultAttributes[RCTAttributedStringEventEmitterKey] = eventEmitterWrapper; + + _backedTextInputView.defaultTextAttributes = defaultAttributes; +} + - (void)didMoveToWindow { [super didMoveToWindow]; @@ -236,8 +260,11 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared & } if (newTextInputProps.textAttributes != oldTextInputProps.textAttributes) { - _backedTextInputView.defaultTextAttributes = + NSMutableDictionary *defaultAttributes = RCTNSTextAttributesFromTextAttributes(newTextInputProps.getEffectiveTextAttributes(RCTFontSizeMultiplier())); + defaultAttributes[RCTAttributedStringEventEmitterKey] = + _backedTextInputView.defaultTextAttributes[RCTAttributedStringEventEmitterKey]; + _backedTextInputView.defaultTextAttributes = [defaultAttributes copy]; } if (newTextInputProps.selectionColor != oldTextInputProps.selectionColor) { @@ -418,6 +445,7 @@ - (void)textInputDidChange - (void)textInputDidChangeSelection { + [self _updateTypingAttributes]; if (_comingFromJS) { return; } @@ -674,9 +702,26 @@ - (void)_setAttributedString:(NSAttributedString *)attributedString [_backedTextInputView scrollRangeToVisible:NSMakeRange(offsetStart, 0)]; } [self _restoreTextSelection]; + [self _updateTypingAttributes]; _lastStringStateWasUpdatedWith = attributedString; } +// Ensure that newly typed text will inherit any custom attributes. We follow the logic of RN Android, where attributes +// to the left of the cursor are copied into new text, unless we are at the start of the field, in which case we will +// copy the attributes from text to the right. This allows consistency between backed input and new AttributedText +// https://github.com/facebook/react-native/blob/3102a58df38d96f3dacef0530e4dbb399037fcd2/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/internal/span/SetSpanOperation.kt#L30 +- (void)_updateTypingAttributes +{ + if (_backedTextInputView.attributedText.length > 0) { + NSUInteger offsetStart = [_backedTextInputView offsetFromPosition:_backedTextInputView.beginningOfDocument + toPosition:_backedTextInputView.selectedTextRange.start]; + + NSUInteger samplePoint = offsetStart == 0 ? 0 : offsetStart - 1; + _backedTextInputView.typingAttributes = [_backedTextInputView.attributedText attributesAtIndex:samplePoint + effectiveRange:NULL]; + } +} + - (void)_setMultiline:(BOOL)multiline { [_backedTextInputView removeFromSuperview]; @@ -706,6 +751,10 @@ - (void)_setShowSoftInputOnFocus:(BOOL)showSoftInputOnFocus - (BOOL)_textOf:(NSAttributedString *)newText equals:(NSAttributedString *)oldText { + if (![newText.string isEqualToString:oldText.string]) { + return NO; + } + // When the dictation is running we can't update the attributed text on the backed up text view // because setting the attributed string will kill the dictation. This means that we can't impose // the settings on a dictation. @@ -732,10 +781,107 @@ - (BOOL)_textOf:(NSAttributedString *)newText equals:(NSAttributedString *)oldTe _backedTextInputView.markedTextRange || _backedTextInputView.isSecureTextEntry || fontHasBeenUpdatedBySystem; if (shouldFallbackToBareTextComparison) { - return ([newText.string isEqualToString:oldText.string]); + return YES; } else { - return ([newText isEqualToAttributedString:oldText]); + return [self _areAttributesEffectivelyEqual:oldText newText:newText]; + } +} + +- (BOOL)_areAttributesEffectivelyEqual:(NSAttributedString *)oldText newText:(NSAttributedString *)newText +{ + // We check that for every fragment in the old string + // 1. A fragment of the same range exists in the new string + // 2. The attributes of each matching fragment are the same, ignoring those which match the always set default typing + // attributes + __block BOOL areAttriubtesEqual = YES; + [oldText enumerateAttributesInRange:NSMakeRange(0, oldText.length) + options:0 + usingBlock:^(NSDictionary *attrs, NSRange range, BOOL *stop) { + [newText + enumerateAttributesInRange:range + options:0 + usingBlock:^( + NSDictionary *innerAttrs, + NSRange innerRange, + BOOL *innerStop) { + if (!NSEqualRanges(range, innerRange)) { + areAttriubtesEqual = NO; + *innerStop = YES; + *stop = YES; + return; + } + + NSMutableDictionary *normAttrs = + [attrs mutableCopy]; + NSMutableDictionary *normInnerAttrs = + [innerAttrs mutableCopy]; + + __unused NSDictionary *currentTypingAttributes = + _backedTextInputView.typingAttributes; + __unused NSDictionary *defaultAttributes = + _backedTextInputView.defaultTextAttributes; + + for (NSAttributedStringKey key in _defaultTypingAttributes) { + id defaultAttr = _defaultTypingAttributes[key]; + if ([normAttrs[key] isEqual:defaultAttr] || + (key == NSParagraphStyleAttributeName && + [self _areParagraphStylesEffectivelyEqual:normAttrs[key] + other:defaultAttr])) { + [normAttrs removeObjectForKey:key]; + } + if ([normInnerAttrs[key] isEqual:defaultAttr] || + (key == NSParagraphStyleAttributeName && + [self _areParagraphStylesEffectivelyEqual:normInnerAttrs[key] + other:defaultAttr])) { + [normInnerAttrs removeObjectForKey:key]; + } + } + + if (![normAttrs isEqualToDictionary:normInnerAttrs]) { + areAttriubtesEqual = NO; + *innerStop = YES; + *stop = YES; + } + }]; + }]; + + return areAttriubtesEqual; +} + +// The default NSParagraphStyle included as part of typingAttributes will eventually resolve "natural" directions to +// physical direction, so we should compare resolved directions +- (BOOL)_areParagraphStylesEffectivelyEqual:(NSParagraphStyle *)style1 other:(NSParagraphStyle *)style2 +{ + NSMutableParagraphStyle *mutableStyle1 = [style1 mutableCopy]; + NSMutableParagraphStyle *mutableStyle2 = [style2 mutableCopy]; + + const auto &textAttributes = static_cast(*_props).textAttributes; + + auto layoutDirection = textAttributes.layoutDirection.value_or(LayoutDirection::LeftToRight); + if (mutableStyle1.alignment == NSTextAlignmentNatural) { + mutableStyle1.alignment = + layoutDirection == LayoutDirection::LeftToRight ? NSTextAlignmentLeft : NSTextAlignmentRight; + } + if (mutableStyle2.alignment == NSTextAlignmentNatural) { + mutableStyle2.alignment = + layoutDirection == LayoutDirection::LeftToRight ? NSTextAlignmentLeft : NSTextAlignmentRight; + } + + auto baseWritingDirection = [&]() { + if (textAttributes.baseWritingDirection.has_value()) { + return RCTNSWritingDirectionFromWritingDirection(textAttributes.baseWritingDirection.value()); + } else { + return [NSParagraphStyle defaultWritingDirectionForLanguage:nil]; + } + }(); + if (mutableStyle1.baseWritingDirection == NSWritingDirectionNatural) { + mutableStyle1.baseWritingDirection = baseWritingDirection; } + if (mutableStyle2.baseWritingDirection == NSWritingDirectionNatural) { + mutableStyle2.baseWritingDirection = baseWritingDirection; + } + + return [mutableStyle1 isEqual:mutableStyle2]; } - (SubmitBehavior)getSubmitBehavior diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTAttributedTextUtils.h b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTAttributedTextUtils.h index c0158f3df61e7a..d39093382cb28f 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTAttributedTextUtils.h +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTAttributedTextUtils.h @@ -22,7 +22,7 @@ NSString *const RCTTextAttributesAccessibilityRoleAttributeName = @"Accessibilit /* * Creates `NSTextAttributes` from given `facebook::react::TextAttributes` */ -NSDictionary *RCTNSTextAttributesFromTextAttributes( +NSMutableDictionary *RCTNSTextAttributesFromTextAttributes( const facebook::react::TextAttributes &textAttributes); /* diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTAttributedTextUtils.mm b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTAttributedTextUtils.mm index 2b2cf02fa11604..9b01980fce46bf 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTAttributedTextUtils.mm +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTAttributedTextUtils.mm @@ -35,6 +35,24 @@ - (void)dealloc _weakEventEmitter.reset(); } +- (BOOL)isEqual:(id)object +{ + // We consider the underlying EventEmitter as the identity + if (![object isKindOfClass:[self class]]) { + return NO; + } + + auto thisEventEmitter = [self eventEmitter]; + auto otherEventEmitter = [((RCTWeakEventEmitterWrapper *)object) eventEmitter]; + return thisEventEmitter == otherEventEmitter; +} + +- (NSUInteger)hash +{ + // We consider the underlying EventEmitter as the identity + return (NSUInteger)_weakEventEmitter.lock().get(); +} + @end inline static UIFontWeight RCTUIFontWeightFromInteger(NSInteger fontWeight) @@ -156,7 +174,8 @@ inline static CGFloat RCTEffectiveFontSizeMultiplierFromTextAttributes(const Tex inline static UIColor *RCTEffectiveForegroundColorFromTextAttributes(const TextAttributes &textAttributes) { - UIColor *effectiveForegroundColor = RCTUIColorFromSharedColor(textAttributes.foregroundColor) ?: [UIColor blackColor]; + UIColor *effectiveForegroundColor = + RCTPlatformColorFromColor(*textAttributes.foregroundColor) ?: [UIColor blackColor]; if (!isnan(textAttributes.opacity)) { effectiveForegroundColor = [effectiveForegroundColor @@ -168,7 +187,7 @@ inline static CGFloat RCTEffectiveFontSizeMultiplierFromTextAttributes(const Tex inline static UIColor *RCTEffectiveBackgroundColorFromTextAttributes(const TextAttributes &textAttributes) { - UIColor *effectiveBackgroundColor = RCTUIColorFromSharedColor(textAttributes.backgroundColor); + UIColor *effectiveBackgroundColor = RCTPlatformColorFromColor(*textAttributes.backgroundColor); if (effectiveBackgroundColor && !isnan(textAttributes.opacity)) { effectiveBackgroundColor = [effectiveBackgroundColor @@ -178,7 +197,8 @@ inline static CGFloat RCTEffectiveFontSizeMultiplierFromTextAttributes(const Tex return effectiveBackgroundColor ?: [UIColor clearColor]; } -NSDictionary *RCTNSTextAttributesFromTextAttributes(const TextAttributes &textAttributes) +NSMutableDictionary *RCTNSTextAttributesFromTextAttributes( + const TextAttributes &textAttributes) { NSMutableDictionary *attributes = [NSMutableDictionary dictionaryWithCapacity:10]; @@ -256,7 +276,7 @@ inline static CGFloat RCTEffectiveFontSizeMultiplierFromTextAttributes(const Tex NSUnderlineStyle style = RCTNSUnderlineStyleFromTextDecorationStyle( textAttributes.textDecorationStyle.value_or(TextDecorationStyle::Solid)); - UIColor *textDecorationColor = RCTUIColorFromSharedColor(textAttributes.textDecorationColor); + UIColor *textDecorationColor = RCTPlatformColorFromColor(*textAttributes.textDecorationColor); // Underline if (textDecorationLineType == TextDecorationLineType::Underline || @@ -285,7 +305,7 @@ inline static CGFloat RCTEffectiveFontSizeMultiplierFromTextAttributes(const Tex NSShadow *shadow = [NSShadow new]; shadow.shadowOffset = CGSize{textShadowOffset.width, textShadowOffset.height}; shadow.shadowBlurRadius = textAttributes.textShadowRadius; - shadow.shadowColor = RCTUIColorFromSharedColor(textAttributes.textShadowColor); + shadow.shadowColor = RCTPlatformColorFromColor(*textAttributes.textShadowColor); attributes[NSShadowAttributeName] = shadow; } @@ -302,7 +322,7 @@ inline static CGFloat RCTEffectiveFontSizeMultiplierFromTextAttributes(const Tex attributes[RCTTextAttributesAccessibilityRoleAttributeName] = [NSString stringWithUTF8String:roleStr.c_str()]; } - return [attributes copy]; + return attributes; } void RCTApplyBaselineOffset(NSMutableAttributedString *attributedText) diff --git a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextPrimitivesConversions.h b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextPrimitivesConversions.h index 6a9624bff9f9de..707fd0072f64f8 100644 --- a/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextPrimitivesConversions.h +++ b/packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTTextPrimitivesConversions.h @@ -7,9 +7,9 @@ #import -#include -#include -#include +#import +#import +#import inline static NSTextAlignment RCTNSTextAlignmentFromTextAlignment(facebook::react::TextAlignment textAlignment) { @@ -112,9 +112,3 @@ inline static NSUnderlineStyle RCTNSUnderlineStyleFromTextDecorationStyle( return NSUnderlinePatternDot | NSUnderlineStyleSingle; } } - -// TODO: this file has some duplicates method, we can remove it -inline static UIColor *_Nullable RCTUIColorFromSharedColor(const facebook::react::SharedColor &sharedColor) -{ - return RCTPlatformColorFromColor(*sharedColor); -}