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

[HOLD for upstream] [$2000] Android - Chat - back-to-back bold words don't always render as bold #15648

Closed
1 of 6 tasks
kbecciv opened this issue Mar 3, 2023 · 86 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Monthly KSv2 Not a priority Reviewing Has a PR in review

Comments

@kbecciv
Copy link

kbecciv commented Mar 3, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!


Issue found when executing PR #15501

Action Performed:

Precondition:
Use the following sequence of b characters for completing each test:

*b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b**b*
  1. Paste the b characters into the composer and remove any surrounding backticks and spaces.
  2. Confirm that the comment counter displays as 15012, and that the length is exceeded.
  3. Remove one b sequence from the comment. The comment should now be sendable.
  4. Send the comment and confirm that the delivered message consists only of b characters formatted in bold.

Expected Result:

All 100% of b characters formatted in bold

Actual Result:

50% of b characters formatted in bold

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.2.78.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

Notes/Photos/Videos: Any additional supporting documentation

Screen_Recording_20230303_125606_New.Expensify.1.mp4

Expensify/Expensify Issue URL:

Issue reported by: Applause - Internal Team

Slack conversation:

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0199575fa523903823
  • Upwork Job ID: 1635048377825824768
  • Last Price Increase: 2023-03-19
@kbecciv kbecciv added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 3, 2023
@melvin-bot melvin-bot bot locked and limited conversation to collaborators Mar 3, 2023
@MelvinBot
Copy link

Triggered auto assignment to @sonialiap (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

@MelvinBot
Copy link

MelvinBot commented Mar 3, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@MelvinBot
Copy link

@sonialiap Whoops! This issue is 2 days overdue. Let's get this updated quick!

1 similar comment
@MelvinBot
Copy link

@sonialiap Whoops! This issue is 2 days overdue. Let's get this updated quick!

@sonialiap
Copy link
Contributor

Will test tomorrow

@melvin-bot melvin-bot bot removed the Overdue label Mar 7, 2023
@sonialiap
Copy link
Contributor

I can reproduce the behavior.

On web the text appears 100% bolded, on android it's about 50% bolded

@MelvinBot
Copy link

Triggered auto assignment to @puneetlath (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

@puneetlath
Copy link
Contributor

Is this specific to "b" characters only? Or does it apply for any character?

And does it only happen if you are at the limit. Or does it happen with any number of back-to-back bolded characters?

@puneetlath
Copy link
Contributor

Also, is this an android-only issue?

@kbecciv
Copy link
Author

kbecciv commented Mar 8, 2023

Yes, this is Andriod app issue Only

@sonialiap
Copy link
Contributor

sonialiap commented Mar 9, 2023

I tested it with *h**h**h*, *a**b**c**d**e**f**g**h**i**j**k**l**m**n**o**p**q**r**s**t**u**v**w**x**y**z*, *Sonia* *Sonia* *Sonia* and all produced the same behavior.

I cannot track down how many iterations of the bolded letter/string cause the bolding to fluctuate. In the screenshot bellow you can see that in the first message (54 words) the bolding falls off after 11 words. But testing with 12 words doesn't affect the bolding, nor does 18 or 24. In a message of 78 words the bolding falls off after 35 words. Sending a message of 54 words again results in the same bolding. So the bolding failure isn't totally random, there is some logic to it.

I also tested with a paragraph of lorem ipsum. When I bold the entire paragraph as one (not each individual character/word) then the entire paragraph is correctly bolded.

image

@puneetlath
Copy link
Contributor

Very strange. Ok let's mark it external and see what contributors are able to dig up!

@puneetlath puneetlath added the External Added to denote the issue can be worked on by a contributor label Mar 12, 2023
@melvin-bot melvin-bot bot unlocked this conversation Mar 12, 2023
@melvin-bot melvin-bot bot changed the title Android - Chat - Only 50% of b characters formatted in bold [$1000] Android - Chat - Only 50% of b characters formatted in bold Mar 12, 2023
@MelvinBot
Copy link

Job added to Upwork: https://www.upwork.com/jobs/~0199575fa523903823

@MelvinBot
Copy link

Current assignees @puneetlath and @sonialiap are eligible for the External assigner, not assigning anyone new.

@MelvinBot
Copy link

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 12, 2023
@MelvinBot
Copy link

Current assignee @puneetlath is eligible for the External assigner, not assigning anyone new.

@puneetlath
Copy link
Contributor

@cubuspl42 any luck with the review?

@cubuspl42
Copy link
Contributor

@puneetlath The review is taking longer than I hoped. No word from Facebook's side since my last bump. I bumped the thread again, but I suggest switching to plan B.

  • Let's give the reviewer time to reply until the end of the week
  • If he doesn't reply, I'll implement the idea I mentioned here anyway. It should minimize the chance of any undesired observable behavior changes to close to zero, while still fixing the discussed bug
  • Then...
    • ...I submit that to the upstream PR
    • ...but at the same time we can start an internal PR on our fork

What do you think? This way we could unblock our issue, leaving the long-term upstream review asynchronous.

@puneetlath
Copy link
Contributor

puneetlath commented May 17, 2023

Looks like he's engaging again, so let's wait a bit more before opening the PR on our fork.

@cubuspl42
Copy link
Contributor

@puneetlath
I decided to provide an alternative approach, which is much simpler (while less general), more backward-compatible in corner cases, but most importantly still fixes the Expensify issue we're discussing.

A more general solution could be a potential next step that the React Native team can consider.

I hope that this will speed up the process.

@puneetlath
Copy link
Contributor

Ok cool, sounds good!

@cubuspl42
Copy link
Contributor

cubuspl42 commented May 26, 2023

@puneetlath

  1. [ANDROID] [FIXED] Fixed random styling for text nodes with many children facebook/react-native#36656 is merged
  2. For the purpose of a final test, I applied the commit in its final merged form (dcb4eb0) on top of v0.71.2-alpha.3:
android-fix-bold-1.mp4

What should be the next step? Should we wait for this change to be eventually picked up organically, or should I create a PR to our fork with a cherry-pick?

@melvin-bot
Copy link

melvin-bot bot commented Jun 2, 2023

@puneetlath, @cubuspl42, @sonialiap, @thesahindia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@puneetlath
Copy link
Contributor

Nice!! Great job getting that merged upstream!

Do you know how RN releases work? When could we expect it in a release?

@cubuspl42
Copy link
Contributor

cubuspl42 commented Jun 3, 2023

@puneetlath The React Native team is focused on releasing 0.72 now, and I don't think this change has any chance of being included, as it reached a Golden Release Candidate stage.

I'm trying to figure out whether we can expect this in 0.72.1, or if we need to wait for 0.73. I asked a question on the merged PR.

@cubuspl42
Copy link
Contributor

cubuspl42 commented Jun 5, 2023

I filed a "pick request".

According to this response, a realistic timeline is 0.72.rc6 if rc6 will be needed, or 0.72.1 otherwise.

@puneetlath
Copy link
Contributor

Nice! Let's go ahead and get you paid out @cubuspl42

@puneetlath
Copy link
Contributor

@cubuspl42 I've sent you a contract. @sonialiap can you please pay it out once @cubuspl42 accepts?

@thesahindia I didn't send you one since I don't think you were involved. Please let us know if I'm mistaken in that.

Really great job with this one!

@puneetlath
Copy link
Contributor

Also @sonialiap let's keep this open until we've confirmed that it has been incorporated in RN upstream.

@thesahindia
Copy link
Member

Correct! No compensation for me.

@puneetlath
Copy link
Contributor

@sonialiap I've gone ahead and paid this. So we just need to keep it open to track that the upstream PR gets incorporated. I'll just leave it assigned to me for that.

@puneetlath puneetlath added Monthly KSv2 and removed Daily KSv2 labels Jun 12, 2023
@puneetlath puneetlath changed the title [$2000] Android - Chat - back-to-back bold words don't always render as bold [HOLD for upstream] [$2000] Android - Chat - back-to-back bold words don't always render as bold Jun 12, 2023
@melvin-bot melvin-bot bot closed this as completed Aug 8, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 8, 2023

@puneetlath, @cubuspl42, @thesahindia, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

@puneetlath puneetlath reopened this Aug 8, 2023
@melvin-bot melvin-bot bot closed this as completed Aug 9, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 9, 2023

@puneetlath, @cubuspl42, @thesahindia, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

@puneetlath puneetlath reopened this Aug 9, 2023
@puneetlath
Copy link
Contributor

We're still waiting for the upstream fix to be incorporated.

@melvin-bot melvin-bot bot closed this as completed Oct 6, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 6, 2023

@puneetlath, @cubuspl42, @thesahindia, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

@cubuspl42
Copy link
Contributor

The fix should land in React Native 0.73

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors Monthly KSv2 Not a priority Reviewing Has a PR in review
Projects
None yet
Development

No branches or pull requests

9 participants