[$250] Editing comment with image attachment fails to load the image when cache expires

[m-natarajan]

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:
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):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @kidroca
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1715767781452439

Action Performed:

1. Type text and add an image attachment.
2. After sending the comment, inspect the tag for data-expensify attributes.
3. Edit the comment and make a trivial change to the text before the attachment.
4. Observe that the tag no longer has data-expensify attributes.
5. Disable cache in dev tools, or clear cache and reload the page.
6. Observe that the edited comment no longer loads the image.
   LINK: https://staging.new.expensify.com/r/5708510475809890

Expected Result:

Editing a comment, clearing cache, and reloading the page should display the comment and the image attachment.

Actual Result:

Clearing cache and reloading shows the comment and a placeholder box instead of the image attachment.
More details here: #41952 (comment)

Workaround:

unknown

Platforms:

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

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

330751938-58ed8062-5520-41f2-a971-dc291af9956f.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~018082d3734c0fe3cf
• Upwork Job ID: 1792320613124546560
• Last Price Increase: 2024-06-10

Issue Owner

Current Issue Owner: @

[melvin-bot]

Triggered auto assignment to @jliexpensify (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.

[m-natarajan]

@mallen tagging you as per this comment

[jliexpensify]

@m-natarajan you've tagged the wrong person. cc @mallenexpensify

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~018082d3734c0fe3cf

[melvin-bot]

Triggered auto assignment to Contributor Plus for review of internal employee PR - @rushatgabhane (Internal)

[mallenexpensify]

Labeled internally and added to #vip-vsb cuz it doesn't have to do with money. All yours JLi

[melvin-bot]

@rushatgabhane, @jliexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[jliexpensify]

Not overdue, waiting on an Engineer to pick this up.

[melvin-bot]

@rushatgabhane @jliexpensify this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[jliexpensify]

Not overdue

[melvin-bot]

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

[rushatgabhane]

internal. not overdue

[rushatgabhane]

🎀 👀 🎀 assign internal

[melvin-bot]

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

[amyevans]

Sorry, I don't have bandwidth to take this, unassigning!

[mountiny]

focusing on workspace feeds wave-collect

[jliexpensify]

Not overdue

[mountiny]

I was talking with @mjasikowski that he might be able to take this on later in the week

[melvin-bot]

@rushatgabhane, @jliexpensify, @mountiny Whoops! This issue is 2 days overdue. Let's get this updated quick!

[jliexpensify]

Not overdue

[mountiny]

@mjasikowski will help out here 💪

[jliexpensify]

Assigned!

[mjasikowski]

FInally have time to dig into this.

[mountiny]

was able to repro still

Screen.Recording.2024-09-25.at.10.12.04.mp4

[mjasikowski]

Current findings:

• Web-Expensify does strip extra attributes form img tags via sanitizeAndValidateComment
• Unfortunately, Expensify-Common does this as well in ExpensiMark.ts, making the tags stripped before we even get to the UpdateComment API call

[mjasikowski]

There's a draft expensify-common PR now adding attribute caching support to ExpensiMark.ts, I'll try to do a full test tomorrow.

[mjasikowski]

It turns out that only expensify-common fix is enough, and that's ready in Expensify/expensify-common#808

[mjasikowski]

PR still under review

[mjasikowski]

https://github.com/Expensify/Web-Expensify/pull/43729 was needed after all, submitted that too

[jliexpensify]

Hi @mjasikowski - just a heads up to let you know I'll be OOO from the 3rd to 14th.

I'll prep a Payment Summary, I think this is all that's needed from me:

• C+: @rushatgabhane $250 (New.Dot payment)