Skip to content

Commit 804bdb1

Browse files
NickGerlemanfacebook-github-bot
authored andcommitted
Fix cursor moving while typing quickly and autocorrection triggered in controlled single line TextInput on iOS (New Arch) (facebook#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, 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. Afaict, there is not a public way to queue our own mutation until after this happens. 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 facebook/react-native-website#4247 no longer repros in new arch. 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 (so, 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
1 parent b69a92e commit 804bdb1

File tree

4 files changed

+123
-7
lines changed

4 files changed

+123
-7
lines changed

packages/react-native/Libraries/Text/TextInput/RCTBackedTextInputViewProtocol.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ NS_ASSUME_NONNULL_BEGIN
3535
@property (nonatomic, assign, readonly) CGFloat zoomScale;
3636
@property (nonatomic, assign, readonly) CGPoint contentOffset;
3737
@property (nonatomic, assign, readonly) UIEdgeInsets contentInset;
38+
@property (nullable, nonatomic, copy) NSDictionary<NSAttributedStringKey, id> *typingAttributes;
3839

3940
// This protocol disallows direct access to `selectedTextRange` property because
4041
// unwise usage of it can break the `delegate` behavior. So, we always have to

packages/react-native/React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm

Lines changed: 100 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,13 @@ @implementation RCTTextInputComponentView {
6161
*/
6262
BOOL _comingFromJS;
6363
BOOL _didMoveToWindow;
64+
65+
/*
66+
* Newly initialized default typing attributes contain a no-op NSParagraphStyle and NSShadow. These cause inequality
67+
* between the AttributedString backing the input and those generated from state. We stote these attributes to make
68+
* later comparison insensitive to them.
69+
*/
70+
NSDictionary<NSAttributedStringKey, id> *_defaultTypingAttributes;
6471
}
6572

6673
#pragma mark - UIView overrides
@@ -76,6 +83,7 @@ - (instancetype)initWithFrame:(CGRect)frame
7683
_ignoreNextTextInputCall = NO;
7784
_comingFromJS = NO;
7885
_didMoveToWindow = NO;
86+
_defaultTypingAttributes = [_backedTextInputView.defaultTextAttributes copy];
7987

8088
[self addSubview:_backedTextInputView];
8189
[self initializeReturnKeyType];
@@ -84,6 +92,20 @@ - (instancetype)initWithFrame:(CGRect)frame
8492
return self;
8593
}
8694

95+
- (void)updateEventEmitter:(const EventEmitter::Shared &)eventEmitter
96+
{
97+
[super updateEventEmitter:eventEmitter];
98+
99+
NSMutableDictionary<NSAttributedStringKey, id> *defaultAttributes =
100+
[_backedTextInputView.defaultTextAttributes mutableCopy];
101+
102+
RCTWeakEventEmitterWrapper *eventEmitterWrapper = [RCTWeakEventEmitterWrapper new];
103+
eventEmitterWrapper.eventEmitter = _eventEmitter;
104+
defaultAttributes[RCTAttributedStringEventEmitterKey] = eventEmitterWrapper;
105+
106+
_backedTextInputView.defaultTextAttributes = defaultAttributes;
107+
}
108+
87109
- (void)didMoveToWindow
88110
{
89111
[super didMoveToWindow];
@@ -236,8 +258,11 @@ - (void)updateProps:(const Props::Shared &)props oldProps:(const Props::Shared &
236258
}
237259

238260
if (newTextInputProps.textAttributes != oldTextInputProps.textAttributes) {
239-
_backedTextInputView.defaultTextAttributes =
261+
NSMutableDictionary<NSAttributedStringKey, id> *defaultAttributes =
240262
RCTNSTextAttributesFromTextAttributes(newTextInputProps.getEffectiveTextAttributes(RCTFontSizeMultiplier()));
263+
defaultAttributes[RCTAttributedStringEventEmitterKey] =
264+
_backedTextInputView.defaultTextAttributes[RCTAttributedStringEventEmitterKey];
265+
_backedTextInputView.defaultTextAttributes = [defaultAttributes copy];
241266
}
242267

243268
if (newTextInputProps.selectionColor != oldTextInputProps.selectionColor) {
@@ -418,6 +443,7 @@ - (void)textInputDidChange
418443

419444
- (void)textInputDidChangeSelection
420445
{
446+
[self _updateTypingAttributes];
421447
if (_comingFromJS) {
422448
return;
423449
}
@@ -674,9 +700,26 @@ - (void)_setAttributedString:(NSAttributedString *)attributedString
674700
[_backedTextInputView scrollRangeToVisible:NSMakeRange(offsetStart, 0)];
675701
}
676702
[self _restoreTextSelection];
703+
[self _updateTypingAttributes];
677704
_lastStringStateWasUpdatedWith = attributedString;
678705
}
679706

707+
// Ensure that newly typed text will inherit any custom attributes. We follow the logic of RN Android, where attributes
708+
// 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
709+
// copy the attributes from text to the right. This allows consistency between backed input and new AttributedText
710+
// dhttps://github.com/facebook/react-native/blob/3102a58df38d96f3dacef0530e4dbb399037fcd2/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/views/text/internal/span/SetSpanOperation.kt#L30
711+
- (void)_updateTypingAttributes
712+
{
713+
if (_backedTextInputView.attributedText.length > 0) {
714+
NSUInteger offsetStart = [_backedTextInputView offsetFromPosition:_backedTextInputView.beginningOfDocument
715+
toPosition:_backedTextInputView.selectedTextRange.start];
716+
717+
NSUInteger samplePoint = offsetStart == 0 ? 0 : offsetStart - 1;
718+
_backedTextInputView.typingAttributes = [_backedTextInputView.attributedText attributesAtIndex:samplePoint
719+
effectiveRange:NULL];
720+
}
721+
}
722+
680723
- (void)_setMultiline:(BOOL)multiline
681724
{
682725
[_backedTextInputView removeFromSuperview];
@@ -706,6 +749,10 @@ - (void)_setShowSoftInputOnFocus:(BOOL)showSoftInputOnFocus
706749

707750
- (BOOL)_textOf:(NSAttributedString *)newText equals:(NSAttributedString *)oldText
708751
{
752+
if (![newText.string isEqualToString:oldText.string]) {
753+
return NO;
754+
}
755+
709756
// When the dictation is running we can't update the attributed text on the backed up text view
710757
// because setting the attributed string will kill the dictation. This means that we can't impose
711758
// the settings on a dictation.
@@ -732,10 +779,59 @@ - (BOOL)_textOf:(NSAttributedString *)newText equals:(NSAttributedString *)oldTe
732779
_backedTextInputView.markedTextRange || _backedTextInputView.isSecureTextEntry || fontHasBeenUpdatedBySystem;
733780

734781
if (shouldFallbackToBareTextComparison) {
735-
return ([newText.string isEqualToString:oldText.string]);
782+
return YES;
736783
} else {
737-
return ([newText isEqualToAttributedString:oldText]);
738-
}
784+
return [self _areAttributesEffectivelyEqual:oldText newText:newText];
785+
}
786+
}
787+
788+
- (BOOL)_areAttributesEffectivelyEqual:(NSAttributedString *)oldText newText:(NSAttributedString *)newText
789+
{
790+
// We check that for every fragment in the old string
791+
// 1. A fragment of the same range exists in the new string
792+
// 2. The attributes of each matching fragment are the same, ignoring those which match the always set default typing
793+
// attributes
794+
__block BOOL areAttriubtesEqual = YES;
795+
[oldText enumerateAttributesInRange:NSMakeRange(0, oldText.length)
796+
options:0
797+
usingBlock:^(NSDictionary<NSAttributedStringKey, id> *attrs, NSRange range, BOOL *stop) {
798+
[oldText enumerateAttributesInRange:range
799+
options:0
800+
usingBlock:^(
801+
NSDictionary<NSAttributedStringKey, id> *innerAttrs,
802+
NSRange innerRange,
803+
BOOL *innerStop) {
804+
if (!NSEqualRanges(range, innerRange)) {
805+
areAttriubtesEqual = NO;
806+
*innerStop = YES;
807+
*stop = YES;
808+
return;
809+
}
810+
811+
NSMutableDictionary<NSAttributedStringKey, id> *normAttrs =
812+
[attrs mutableCopy];
813+
NSMutableDictionary<NSAttributedStringKey, id> *normInnerAttrs =
814+
[innerAttrs mutableCopy];
815+
816+
for (NSAttributedStringKey key in _defaultTypingAttributes) {
817+
id defaultAttr = _defaultTypingAttributes[key];
818+
if ([normAttrs[key] isEqual:defaultAttr]) {
819+
[normAttrs removeObjectForKey:key];
820+
}
821+
if ([normInnerAttrs[key] isEqual:normInnerAttrs]) {
822+
[normInnerAttrs removeObjectForKey:key];
823+
}
824+
}
825+
826+
if (![normAttrs isEqualToDictionary:normInnerAttrs]) {
827+
areAttriubtesEqual = NO;
828+
*innerStop = YES;
829+
*stop = YES;
830+
}
831+
}];
832+
}];
833+
834+
return areAttriubtesEqual;
739835
}
740836

741837
- (SubmitBehavior)getSubmitBehavior

packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTAttributedTextUtils.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ NSString *const RCTTextAttributesAccessibilityRoleAttributeName = @"Accessibilit
2222
/*
2323
* Creates `NSTextAttributes` from given `facebook::react::TextAttributes`
2424
*/
25-
NSDictionary<NSAttributedStringKey, id> *RCTNSTextAttributesFromTextAttributes(
25+
NSMutableDictionary<NSAttributedStringKey, id> *RCTNSTextAttributesFromTextAttributes(
2626
const facebook::react::TextAttributes &textAttributes);
2727

2828
/*

packages/react-native/ReactCommon/react/renderer/textlayoutmanager/platform/ios/react/renderer/textlayoutmanager/RCTAttributedTextUtils.mm

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,24 @@ - (void)dealloc
3535
_weakEventEmitter.reset();
3636
}
3737

38+
- (BOOL)isEqual:(id)object
39+
{
40+
// We consider the underlying EventEmitter as the identity
41+
if (![object isKindOfClass:[self class]]) {
42+
return NO;
43+
}
44+
45+
auto thisEventEmitter = [self eventEmitter];
46+
auto otherEventEmitter = [((RCTWeakEventEmitterWrapper *)object) eventEmitter];
47+
return thisEventEmitter == otherEventEmitter;
48+
}
49+
50+
- (NSUInteger)hash
51+
{
52+
// We consider the underlying EventEmitter as the identity
53+
return (NSUInteger)_weakEventEmitter.lock().get();
54+
}
55+
3856
@end
3957

4058
inline static UIFontWeight RCTUIFontWeightFromInteger(NSInteger fontWeight)
@@ -178,7 +196,8 @@ inline static CGFloat RCTEffectiveFontSizeMultiplierFromTextAttributes(const Tex
178196
return effectiveBackgroundColor ?: [UIColor clearColor];
179197
}
180198

181-
NSDictionary<NSAttributedStringKey, id> *RCTNSTextAttributesFromTextAttributes(const TextAttributes &textAttributes)
199+
NSMutableDictionary<NSAttributedStringKey, id> *RCTNSTextAttributesFromTextAttributes(
200+
const TextAttributes &textAttributes)
182201
{
183202
NSMutableDictionary<NSAttributedStringKey, id> *attributes = [NSMutableDictionary dictionaryWithCapacity:10];
184203

@@ -302,7 +321,7 @@ inline static CGFloat RCTEffectiveFontSizeMultiplierFromTextAttributes(const Tex
302321
attributes[RCTTextAttributesAccessibilityRoleAttributeName] = [NSString stringWithUTF8String:roleStr.c_str()];
303322
}
304323

305-
return [attributes copy];
324+
return attributes;
306325
}
307326

308327
void RCTApplyBaselineOffset(NSMutableAttributedString *attributedText)

0 commit comments

Comments
 (0)