-
Notifications
You must be signed in to change notification settings - Fork 3k
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 payment 2024-01-02] [$500] App unusable on Safari 16.3 #33215
Comments
Triggered auto assignment to @anmurali ( |
Bug0 Triage Checklist (Main S/O)
|
@anmurali we might already know the fix. Let's not make it external just yet. |
@anyongjitiger Tagging you in here as you worked on the PR that (probably) caused this. |
Not going to count this as a regression, as it's impossible to test old safari versions... so we'll treat it as a new issue, with a regular bounty |
@rojiphil are you interested in working on this bug? |
@Julesssss |
ProposalPlease re-state the problem that we are trying to solve in this issue.In older versions of Safari (i.e. 16.3) , the app does not load. What is the root cause of that problem?Older versions of Safari do not support lookbehind regex. So, here, the regex uses lookbehind regex. This is the root cause. What changes do you think we should make in order to solve the problem?We can solve the problem using capture groups as follows here
What alternative solutions did you explore? (Optional) |
@Julesssss |
Thank you, I'll assign a C+ for the PR review. |
Job added to Upwork: https://www.upwork.com/jobs/~01b9ae6039965bf1b0 |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @alitoshmatov ( |
📣 @rojiphil 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
I have very old macOS 12.6 (Safari 16.0) so if assigned C+ is not able to test (because they don't have old version), I can help |
The PR is ready for review. |
I can review but giving priority to assigned reviewers first |
Bumping @Santhosh-Sellavel, which Safari version are you running? |
I'm on 17.1.2 |
Feel free to review this one @situchan, I'm not sure how to test this one! cc: @Julesssss |
📣 @situchan 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
PR merged awaiting deployment |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.16-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2024-01-02. 🎊 After the hold period is over and BZ checklist items are completed, please complete any of the applicable payments for this issue, and check them off once done.
For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
@anmurali |
@rojiphil, @Julesssss, @anmurali, @situchan Eep! 4 days overdue now. Issues have feelings too... |
Paid. I don't think we would add any regression tests for this. |
CC @rojiphil @jjcoffee
Version Number: ?
Reproducible in staging?:
Reproducible in production?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1702869444870729
Action Performed:
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
View all open jobs on GitHub
Upwork Automation - Do Not Edit
The text was updated successfully, but these errors were encountered: