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

App considers number emojis as part of link #26632

Closed
6 tasks done
kavimuru opened this issue Sep 4, 2023 · 5 comments
Closed
6 tasks done

App considers number emojis as part of link #26632

kavimuru opened this issue Sep 4, 2023 · 5 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2

Comments

@kavimuru
Copy link

kavimuru commented Sep 4, 2023

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


Action Performed:

  1. Open the app
  2. Open any report
  3. Copy the link of report
  4. Open another report and paste in it
  5. In place of last digit, use equivalent number emoji i.e. if last digit is 1 in link, in emoji picker, search 1 and use emoji which has 1 in square background
  6. Send the message and observe that app considers emoji as part of link

Expected Result:

App should not consider any emoji as part of link

Actual Result:

App considers number emojis as part of link

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

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.3.62-1
Reproducible in staging?: y
Reproducible in production?: y
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

emojis.as.part.of.link.mp4
emoji.mp4

Expensify/Expensify Issue URL:
Issue reported by: @dhanashree-sawant
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1692637594208169

View all open jobs on GitHub

@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 4, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 4, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Sep 4, 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

@AmjedNazzal
Copy link
Contributor

Proposal

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

App considers number emojis as part of link

What is the root cause of that problem?

Our issue here lies in ExpensiMark, in specific it lies in URL_PATH_REGEX which is a regex used to decide the returned link of MARKDOWN_URL_REGEX.

The issue with the regex that is URL_PATH_REGEX is \w which is is matching any word character (a-z, A-Z, 0-9, and underscore) and that's causing the issue here because number emojis are structured as follows:

  1. The actual number represented in the emoji which is a regular ASCII character.
  2. Variation Selector-16" (VS-16) which looks like this "U+FE0F"
  3. Combining Enclosing Keycap which looks like this "U+20E3"

In combination, javascript for example will understand the 1 emojis as "\u0031\uFE0F\u20E3" with u0031 being the number 1 and as far as javascript is concerned these are 3 seperate items so it will take the first one and use it as it's an actual number which will be considered a match for \w.

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

What we should do here is use Negative Lookahead (?!...) and include the unicode that is used in number emojis which will be something like this (?!\ufe0f\u20e3]) which is telling javascript that if it sees a digit followed by these specific unicode characters that are used in emojis then it should not match \w

const URL_PATH_REGEX = `(?:${addEscapedChar('[.,=(+$!*]')}?\\/${addEscapedChar('[-\\w$@.+!*:(),=%~](?![\\ufe0f\\u20e3])')}*${addEscapedChar('[-\\w~@:%](?![\\ufe0f\\u20e3])')}|\\/)*`;

Result

Screen.Recording.2023-09-04.at.1.07.34.PM.mov

@zanyrenney
Copy link
Contributor

have asked a question in the channel

@zanyrenney
Copy link
Contributor

zanyrenney commented Sep 5, 2023

As this is a super niche edge case, and not a user-reported error. I am going to close this bug report. There is no real workflow where a user would make take this action of deleting part of a link and replacing the link with an emoji of the letter in the link unless it was in searching for breaking the App as opposed to a natural/user error workflow.

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. Daily KSv2
Projects
None yet
Development

No branches or pull requests

3 participants