[$250] Cannot select a word in native iOS composer if composed message have an emoji

m-natarajan commented 1 week ago

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

Version Number: 9.0.2-0 Reproducible in staging?: Yes Reproducible in production?: Yes If this was caught during regression testing, add the test name, ID and link from TestRail: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: @mountiny Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1719405668302249

Action Performed:

Launch the app
Initiate a DM with any user
Compose a message with emoji
Try selecting a word by long press or double tab
Expected Result:

Able to select word by long press or by double tapping

Actual Result:

Unable to select by long press or by double tapping

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?
- [ ] Android: Native
- [ ] Android: mWeb Chrome
- [x] iOS: Native
- [ ] iOS: mWeb Safari

https://github.com/Expensify/App/assets/38435837/05f9d34d-e019-45a6-bde3-e89d0c3e5db3

https://github.com/Expensify/App/assets/38435837/e4f0067b-b30c-4928-9784-b2ccc8a75760

[ ] MacOS: Chrome / Safari
[ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01a59336e899fcaa47
Upwork Job ID: 1806116311980257786
Last Price Increase: 2024-06-27
Automatic offers:

QichenZhu | Contributor | 102976322

melvin-bot[bot] commented 1 week ago

Triggered auto assignment to @lschurr (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

melvin-bot[bot] commented 1 week ago

Job added to Upwork: https://www.upwork.com/jobs/~01a59336e899fcaa47

melvin-bot[bot] commented 1 week ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mollfpr (External)

mountiny commented 1 week ago

cc @BartoszGrajdek @tomekzaw

BartoszGrajdek commented 1 week ago

Let's add this to the Live Markdown project @mountiny

sandipanghos commented 1 week ago

@mountiny is this still open for proposal? I am working on it

QichenZhu commented 1 week ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

Cannot select a word in the native iOS composer if the message contains an emoji.

What is the root cause of that problem?

Originally, React Native had an issue with TextInput when using emojis.

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.

React Native fixed this by ignoring all attributes in such cases.

https://github.com/facebook/react-native/blob/9269429bb955d47088793a2e32454fddb712152b/packages/react-native/Libraries/Text/TextInput/RCTBaseTextInputView.mm#L130-L167

However, this fix caused Expensify/react-native-live-markdown#293 and #41390, and was reverted by Expensify/react-native-live-markdown#318.

What changes do you think we should make in order to solve the problem?

Making clones of newText and oldText and removing the NSOriginalFont and NSFont attributes from them before calling isEqualToAttributedString.

-    return [newText isEqualToAttributedString:oldText];
+    NSMutableAttributedString *newTextCopy = [newText mutableCopy];
+    NSMutableAttributedString *oldTextCopy = [oldText mutableCopy];
+    [newTextCopy removeAttribute:@"NSFont" range:NSMakeRange(0, newTextCopy.length)];
+    [oldTextCopy removeAttribute:@"NSFont" range:NSMakeRange(0, oldTextCopy.length)];
+    [oldTextCopy removeAttribute:@"NSOriginalFont" range:NSMakeRange(0, oldTextCopy.length)];
+    return [newTextCopy isEqualToAttributedString:oldTextCopy];

What alternative solutions did you explore? (Optional)

N/A

melvin-bot[bot] commented 4 days ago

@mollfpr, @lschurr Huh... This is 4 days overdue. Who can take care of this?

lschurr commented 4 days ago

@mountiny is this open for proposals? Should @mollfpr be reviewing?

tomekzaw commented 4 days ago

@QichenZhu Could you please provide more details about the suggested fix for this problem?

QichenZhu commented 3 days ago

Proposal

@tomekzaw Sure, here's the updated proposal. Thanks!

tomekzaw commented 3 days ago

Thanks, sounds good to me. Can we proceed with the PR so we can test out the changes?

mollfpr commented 3 days ago

I don't have much experience with objective-c, but I believe @tomekzaw reviewed.

Probably @QichenZhu can provide a video of the result before the assignment?

QichenZhu commented 3 days ago

@mollfpr

Before:

https://github.com/Expensify/App/assets/57348009/64c3a205-2541-4c64-a87e-f2202a4990ca

After:

https://github.com/Expensify/App/assets/57348009/b0900df6-a29c-48d4-a822-2ce1a74e9d9a

mollfpr commented 3 days ago

Thanks, @QichenZhu. It looks good to me!

Let's move forward with @QichenZhu proposal then!

🎀 👀 🎀 C+ reviewed!

melvin-bot[bot] commented 3 days ago

Triggered auto assignment to @stitesExpensify, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

melvin-bot[bot] commented 3 days ago

📣 @QichenZhu 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

QichenZhu commented 3 days ago

@tomekzaw @mollfpr The PR is ready for review.

Expensify / App