Skip to content
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.

Sign up for GitHub

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

Fix a broken input for the Korean alphabet in TextInput #32523

Closed
wants to merge 4 commits into from
Closed

Fix a broken input for the Korean alphabet in TextInput #32523

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
82d3519
fall back to bare text comparison when inputting korean
kmsbernard Nov 3, 2021
ea6b9d8
remove redundant overrides
kmsbernard Nov 3, 2021
c679dab
add fabric version
kmsbernard Nov 3, 2021
2dd1dc0
invalidate placeholder visibility
kmsbernard Nov 18, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 1 addition & 29 deletions Libraries/Text/TextInput/Multiline/RCTUITextView.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,21 +147,7 @@ - (void)setTextAlignment:(NSTextAlignment)textAlignment

- (void)setAttributedText:(NSAttributedString *)attributedText
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you explain why this is redundant and safe to remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Root cause of this issue is updating attributedText via -setAttributedText: of RCTBaseTextInputView.
In this removed block, it attempts to avoid calling the method by comparing two attributed text with bare strings. But it should be handled by -textOf:quals: which already handles some other special cases such as Chinese and Japanese.
I added detection for Korean to safely fallbacks to bare text comparison in the -textOf:quals:.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found out that it is required to invalidate placeholder visibility in this method. I added a required commit.

{
// Using `setAttributedString:` while user is typing breaks some internal mechanics
// when entering complex input languages such as Chinese, Korean or Japanese.
// see: https://github.com/facebook/react-native/issues/19339

// We try to avoid calling this method as much as we can.
// If the text has changed, there is nothing we can do.
if (![super.attributedText.string isEqualToString:attributedText.string]) {
[super setAttributedText:attributedText];
} else {
// But if the text is preserved, we just copying the attributes from the source string.
if (![super.attributedText isEqualToAttributedString:attributedText]) {
[self copyTextAttributesFrom:attributedText];
}
}

[super setAttributedText:attributedText];
[self textDidChange];
}

Expand Down Expand Up @@ -311,18 +297,4 @@ - (CGRect)caretRectForPosition:(UITextPosition *)position

#pragma mark - Utility Methods

- (void)copyTextAttributesFrom:(NSAttributedString *)sourceString
{
[self.textStorage beginEditing];

NSTextStorage *textStorage = self.textStorage;
[sourceString enumerateAttributesInRange:NSMakeRange(0, sourceString.length)
options:NSAttributedStringEnumerationReverse
usingBlock:^(NSDictionary<NSAttributedStringKey,id> * _Nonnull attrs, NSRange range, BOOL * _Nonnull stop) {
[textStorage setAttributes:attrs range:range];
}];

[self.textStorage endEditing];
}

@end
2 changes: 2 additions & 0 deletions Libraries/Text/TextInput/RCTBaseTextInputView.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ - (BOOL)textOf:(NSAttributedString*)newText equals:(NSAttributedString*)oldText{
// Similarly, when the user is in the middle of inputting some text in Japanese/Chinese, there will be styling on the
// text that we should disregard. See https://developer.apple.com/documentation/uikit/uitextinput/1614489-markedtextrange?language=objc
// for more info.
// Also, updating the attributed text while inputting Korean language will break input mechanism.
// If the user added an emoji, the system adds a font attribute for the emoji and stores the original font in NSOriginalFont.
// Lastly, when entering a password, etc., there will be additional styling on the field as the native text view
// handles showing the last character for a split second.
Expand All @@ -116,6 +117,7 @@ - (BOOL)textOf:(NSAttributedString*)newText equals:(NSAttributedString*)oldText{

BOOL shouldFallbackToBareTextComparison =
[self.backedTextInputView.textInputMode.primaryLanguage isEqualToString:@"dictation"] ||
[self.backedTextInputView.textInputMode.primaryLanguage isEqualToString:@"ko-KR"] ||
self.backedTextInputView.markedTextRange ||
self.backedTextInputView.isSecureTextEntry ||
fontHasBeenUpdatedBySystem;
Expand Down
6 changes: 4 additions & 2 deletions React/Fabric/Mounting/ComponentViews/TextInput/RCTTextInputComponentView.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -579,8 +579,9 @@ - (BOOL)_textOf:(NSAttributedString *)newText equals:(NSAttributedString *)oldTe
// the settings on a dictation.
// Similarly, when the user is in the middle of inputting some text in Japanese/Chinese, there will be styling on the
// text that we should disregard. See
// https://developer.apple.com/documentation/uikit/uitextinput/1614489-markedtextrange?language=objc for more info. If
// the user added an emoji, the system adds a font attribute for the emoji and stores the original font in
// https://developer.apple.com/documentation/uikit/uitextinput/1614489-markedtextrange?language=objc for more info.
// Also, updating the attributed text while inputting Korean language will break input mechanism.
// If the user added an emoji, the system adds a font attribute for the emoji and stores the original font in
// NSOriginalFont. Lastly, when entering a password, etc., there will be additional styling on the field as the native
// text view handles showing the last character for a split second.
__block BOOL fontHasBeenUpdatedBySystem = false;
Expand All @@ -595,6 +596,7 @@ - (BOOL)_textOf:(NSAttributedString *)newText equals:(NSAttributedString *)oldTe

BOOL shouldFallbackToBareTextComparison =
[_backedTextInputView.textInputMode.primaryLanguage isEqualToString:@"dictation"] ||
[_backedTextInputView.textInputMode.primaryLanguage isEqualToString:@"ko-KR"] ||
_backedTextInputView.markedTextRange || _backedTextInputView.isSecureTextEntry || fontHasBeenUpdatedBySystem;

if (shouldFallbackToBareTextComparison) {
Expand Down