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.

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

[IOS] TextInput with emojis capitalizes text and causes other issues #21243

Closed
3 tasks done
superandrew213 opened this issue Sep 21, 2018 · 9 comments
Closed
3 tasks done
Labels
Bug Component: TextInput Related to the TextInput component. Platform: iOS iOS applications. Resolution: Locked This issue was locked by the bot.

Comments

@superandrew213
Copy link

superandrew213 commented Sep 21, 2018

Environment

Environment:
OS: macOS High Sierra 10.13.2
Node: 8.9.1
Yarn: 1.9.4
npm: 5.8.0
Watchman: 4.7.0
Xcode: Xcode 9.2 Build version 9C40b
Android Studio: 3.1 AI-173.4907809

Packages: (wanted => installed)
react: 16.5.0 => 16.5.0
react-native: 0.57.0 => 0.57.0

Description

On IOS, adding emojis to TextInput causes the following:

  • Text goes capital. Caps must be turned off every time you press a letter.
  • Auto correct suggestions don't work.
  • When adding newlines and emoji moves out of view, cursor gets scrolled to top of TextInput
  • Removing emojis fixes it.

Works in RN0.53.

Started breaking in RN0.54 and still breaks in RN0.57.

RN0.57 has a bug that breaks the RN build. To test on RN0.57, apply this manual fix when the build fails the first time: #21071 (comment)

Reproducible Demo

See snack here:
https://snack.expo.io/@superandrew213/textinput-emoji-bug

Test on device and not on appetize/browser.

screenrecording_09-21-2018 19-44-21

Related Issues

#19389, #20908, wix-incubator/react-native-autogrow-textinput#47

@react-native-bot react-native-bot added Platform: iOS iOS applications. Component: TextInput Related to the TextInput component. labels Sep 21, 2018
@react-native-bot
Copy link
Collaborator

It looks like you are using an older version of React Native. Please update to the latest release, v0.57 and verify if the issue still exists.

The ":rewind:Old Version" label will be removed automatically once you edit your original post with the results of running react-native info on a project using the latest release.

@superandrew213
Copy link
Author

@shergin could you have a look at this? I can see that you did the reimplementation of TextInput in RN0.54.

@superandrew213
Copy link
Author

All emoji characters are automatically given an AppleColorEmoji NSFont attribute and the original font is moved to NSOriginalFont attribute.

This causes an issue when we call isEqualToAttributedString when we set the attributedText, since it will always be false if an emoji is present.

See here: https://stackoverflow.com/a/39456258

I guess we might need to add an exception for emojis here too and compare strings only and leave out attributes:

https://github.com/facebook/react-native/blob/master/Libraries/Text/TextInput/RCTBaseTextInputView.m#L110-L113

Or remove the attributes that are automatically added before we do the comparison.

@dchersey
Copy link
Contributor

This may be related to #20908 ; still happening in 57.1

@superandrew213
Copy link
Author

This is my patch. @dchersey feel free to create a PR if you are keen.

Patch is based on RN56. In RN57 method textOf was added. You just need to modify a small part of it to account for emojis.

patch-package
--- a/node_modules/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.m
+++ b/node_modules/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.m
@@ -98,10 +98,40 @@ - (NSAttributedString *)attributedText
   return self.backedTextInputView.attributedText;
 }
 
+- (BOOL)textOf:(NSAttributedString*)newText equals:(NSAttributedString*)oldText{
+  // When the dictation is running we can't update the attibuted 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.
+  // 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 if user has added an emoji, the sytem 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;
+  [oldText enumerateAttribute:@"NSOriginalFont" inRange:NSMakeRange(0, oldText.length) options:0 usingBlock:^(id value, NSRange range, BOOL *stop) {
+    if (value){
+      fontHasBeenUpdatedBySystem = true;
+    }
+  }];
+  
+  BOOL shouldFallbackToBareTextComparison =
+  [self.backedTextInputView.textInputMode.primaryLanguage isEqualToString:@"dictation"] ||
+  self.backedTextInputView.markedTextRange ||
+  self.backedTextInputView.isSecureTextEntry ||
+  fontHasBeenUpdatedBySystem;
+  
+  if (shouldFallbackToBareTextComparison) {
+    return ([newText.string isEqualToString:oldText.string]);
+  } else {
+    return ([newText isEqualToAttributedString:oldText]);
+  }
+}
+
 - (void)setAttributedText:(NSAttributedString *)attributedText
 {
   NSInteger eventLag = _nativeEventCount - _mostRecentEventCount;
-
+  BOOL textNeedsUpdate = NO;
   // Remove tag attribute to ensure correct attributed string comparison.
   NSMutableAttributedString *const backedTextInputViewTextCopy = [self.backedTextInputView.attributedText mutableCopy];
   NSMutableAttributedString *const attributedTextCopy = [attributedText mutableCopy];
@@ -112,7 +142,9 @@ - (void)setAttributedText:(NSAttributedString *)attributedText
   [attributedTextCopy removeAttribute:RCTTextAttributesTagAttributeName
                                 range:NSMakeRange(0, attributedTextCopy.length)];
 
-  if (eventLag == 0 && ![attributedTextCopy isEqualToAttributedString:backedTextInputViewTextCopy]) {
+  textNeedsUpdate = ([self textOf:attributedTextCopy equals:backedTextInputViewTextCopy] == NO);
+  
+  if (eventLag == 0 && textNeedsUpdate) {
     UITextRange *selection = self.backedTextInputView.selectedTextRange;
     NSInteger oldTextLength = self.backedTextInputView.attributedText.string.length;
 

@dchersey
Copy link
Contributor

dchersey commented Oct 24, 2018

Thanks, I will test it out tomorrow morning! To clarify, is this expected to address #20908 as well as this issue?

@superandrew213
Copy link
Author

@dchersey both. The issue is the same.

dchersey added a commit to Leap-Forward/react-native that referenced this issue Oct 25, 2018
dchersey added a commit to Leap-Forward/react-native that referenced this issue Oct 25, 2018
…t after the text being edited.

# Conflicts:
#	Libraries/Text/TextInput/RCTBaseTextInputView.m
dchersey added a commit to Leap-Forward/react-native that referenced this issue Oct 25, 2018
…t after the text being edited.

# Conflicts:
#	Libraries/Text/TextInput/RCTBaseTextInputView.m
@dchersey
Copy link
Contributor

That did the trick! PR will follow shortly.

dchersey added a commit to Leap-Forward/react-native that referenced this issue Oct 25, 2018
…t after the text being edited.

# Conflicts:
#	Libraries/Text/TextInput/RCTBaseTextInputView.m
@dchersey
Copy link
Contributor

dchersey commented Nov 1, 2018

Here's the tested patch updated for 0.57.x in case anyone needs it ... use patch_package to apply it during npm or yarn install.

patch-package
--- a/node_modules/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.m
+++ b/node_modules/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.m
@@ -105,12 +105,22 @@ - (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 if user has added an emoji, the sytem 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;
+  [oldText enumerateAttribute:@"NSOriginalFont" inRange:NSMakeRange(0, oldText.length) options:0 usingBlock:^(id value, NSRange range, BOOL *stop) {
+    if (value){
+      fontHasBeenUpdatedBySystem = true;
+    }
+  }];
+
   BOOL shouldFallbackToBareTextComparison =
     [self.backedTextInputView.textInputMode.primaryLanguage isEqualToString:@"dictation"] ||
     self.backedTextInputView.markedTextRange ||
-    self.backedTextInputView.isSecureTextEntry;
+    self.backedTextInputView.isSecureTextEntry ||
+    fontHasBeenUpdatedBySystem;
+
   if (shouldFallbackToBareTextComparison) {
     return ([newText.string isEqualToString:oldText.string]);
   } else {

@hramos hramos removed the Bug Report label Feb 6, 2019
matt-oakes pushed a commit to matt-oakes/react-native that referenced this issue Feb 7, 2019
…dited. (facebook#21951)

Summary:
Fixes facebook#21243.
Fixes facebook#20908.

Credit goes to superandrew213 who provided the patch based on 0.56; this commit merges and resolved the conflict introduced in 0.57.
Pull Request resolved: facebook#21951

Differential Revision: D13980799

Pulled By: cpojer

fbshipit-source-id: 6b9f1a1ae54ad9dba043005d683d6a221472c729
grabbou pushed a commit that referenced this issue Feb 18, 2019
…dited. (#21951)

Summary:
Fixes #21243.
Fixes #20908.

Credit goes to superandrew213 who provided the patch based on 0.56; this commit merges and resolved the conflict introduced in 0.57.
Pull Request resolved: #21951

Differential Revision: D13980799

Pulled By: cpojer

fbshipit-source-id: 6b9f1a1ae54ad9dba043005d683d6a221472c729
t-nanava pushed a commit to microsoft/react-native-macos that referenced this issue Jun 17, 2019
…dited. (facebook#21951)

Summary:
Fixes facebook#21243.
Fixes facebook#20908.

Credit goes to superandrew213 who provided the patch based on 0.56; this commit merges and resolved the conflict introduced in 0.57.
Pull Request resolved: facebook#21951

Differential Revision: D13980799

Pulled By: cpojer

fbshipit-source-id: 6b9f1a1ae54ad9dba043005d683d6a221472c729
@facebook facebook locked as resolved and limited conversation to collaborators Feb 7, 2020
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Feb 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Component: TextInput Related to the TextInput component. Platform: iOS iOS applications. Resolution: Locked This issue was locked by the bot.
Projects
None yet
4 participants