[$125] Web - Chat - PNG attachment's background colour turns to white
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: v9.0.73-7 Reproducible in staging?: Yes Reproducible in production?: Yes Issue reported by: Applause Internal Team
Action Performed:
- Open https://staging.new.expensify.com
- Click on the plus icon in the compose box of any chat.
- Select a PNG image without a background and send it to the chat.
Expected Result:
PNG image file thumbnail should be shown as it is without added background colour.
Actual Result:
The image's background turns to white after some seconds.
Workaround:
Unknown
Platforms:
- [ ] Android: Standalone
- [ ] Android: HybridApp
- [ ] Android: mWeb Chrome
- [ ] iOS: Standalone
- [ ] iOS: HybridApp
- [ ] iOS: mWeb Safari
- [x] MacOS: Chrome / Safari
- [ ] MacOS: Desktop
Screenshots/Videos
https://github.com/user-attachments/assets/5ba1a1c7-afe6-4db7-8b24-b4556c512e0e
Upwork Automation - Do Not Edit
- Upwork Job URL: https://www.upwork.com/jobs/~021866612835147904206
- Upwork Job ID: 1866612835147904206
- Last Price Increase: 2024-12-10
Issue Owner
Current Issue Owner: @hungvu193
Triggered auto assignment to @mallenexpensify (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.
⚠️ Could not update price automatically because there is no linked Upwork Job ID. The BZ team member will need to update the price manually in Upwork.
Job added to Upwork: https://www.upwork.com/jobs/~021866612835147904206
Triggered auto assignment to Contributor-plus team member for initial proposal review - @hungvu193 (External)
Was able to reproduce. Unsure if this is a recent regression, was going to post in #expensify-open-source to check but it might not matter if we want to fix it either way. Set starting price at $125 cuz I'm hoping it's a quick fix.
Edited by proposal-police: This proposal was edited at 2024-12-11 02:05:38 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
The image's background turns to white after some seconds.
What is the root cause of that problem?
The AddAttachment API returns an image URL with the .1024.jpg extension. And we render image here, as a result, the app renders the image as a JPEG instead of PNG, causing the transparency in the background to be lost
"html": "<img src=\"https:\/\/www.expensify.com\/chat-attachments\/8887844905473165339\/w_fd9ef52869d8f5923663098fbd26f7c08f8e3fa1.png.1024.jpg\" data-expensify-source=\"https:\/\/www.expensify.com\/chat-attachments\/8887844905473165339\/w_fd9ef52869d8f5923663098fbd26f7c08f8e3fa1.png\" data-name=\"images__1_-removebg-preview.png\" data-expensify-height=\"221\" data-expensify-width=\"228\" \/>"
AddAttachment API
What changes do you think we should make in order to solve the problem?
We can replace .1024.jpg in the FE by using previewSource.replace(/\.png\.(1024|320)\.jpg$/, '.png').
That code only replaces if the previewSource format is .png.1024.jpg or .png.320.jpg.
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
N/A
What alternative solutions did you explore? (Optional)
N/A
Updated my proposal https://github.com/Expensify/App/issues/53881#issuecomment-2533336193
Edited by proposal-police: This proposal was edited at 2024-12-11 07:04:55 UTC.
Proposal
Please re-state the problem that we are trying to solve in this issue.
PNG attachment's background colour turns to white
What is the root cause of that problem?
The RCA for this ticket is that previewSource is using htmlAttribs.src, which is the HTML image link
https://github.com/Expensify/App/blob/6207c96523a447817bcb84681603c3b2fa74c14a/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.tsx#L60
example link: https://dev.new.expensify.com:8082/chat-attachments/4704811877564310284/w_c651b5ce012983297284c509ccdd4d10dea5e567.png.1024.jpg
This URL, which includes 1024.jpg, still loads the image with a white background, as shown above, because JPG images do not support transparency.
What changes do you think we should make in order to solve the problem?
Because previewSource is using htmlAttribs.src to get the previewSource link and then appending the 1024.jpg suffix, I think we should use htmlAttribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE] instead of htmlAttribs.src, if htmlAttribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE] is available. This is similar to the logic used to get the source. previewSource and source should use the same logic to get the URL consistently, like this:
https://github.com/Expensify/App/blob/f8f12c2dcb6d3b62663fc4edea50cd543e44e74d/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.tsx#L60
Change to
const previewSource = tryResolveUrlFromApiRoot(isAttachmentOrReceipt ? attachmentSourceAttribute : htmlAttribs.src);
POC
https://github.com/user-attachments/assets/0b0a1b71-df5d-443d-958e-e4dbb9fc6253
What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?
N/A
What alternative solutions did you explore? (Optional)
Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.
@hungvu193 👀 on @huult 's proposal plz. Thx
Surre
@hungvu193, @mallenexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
I'll review today!
I can reproduce this issue. Reviewing shortly!
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
Still under reviewing
Thanks for the proposals, everyone.
Based on what we have here: https://github.com/Expensify/App/blob/b6ebedb611a549bb755aee934ca7f7707ce13798/src/components/HTMLEngineProvider/HTMLRenderers/ImageRenderer.tsx#L39-L47
htmlAttribs.src refers to the thumbnail, so it's better to keep it as the preview image instead of using attachmentSourceAttribute (aka htmlAttribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE]).
Removing .1024.jpg from previewSource feels like a workaround to me.
I think this issue should be handled on the BE, and it's best to ask the Internal Engineer about the .1024.jpg extension of the preview source.
🎀 👀 🎀
Triggered auto assignment to @francoisl, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
@hungvu19 Can you review my proposal? My proposal does not require any backend changes or path removal. Thank you
htmlAttribs.srcrefers to the thumbnail, so it's better to keep it as the preview image instead of usingattachmentSourceAttribute(aka htmlAttribs[CONST.ATTACHMENT_SOURCE_ATTRIBUTE]).
Hey, I commented here, I don't think using the full-sized image for thumbnail image is a good idea.
@francoisl, @hungvu193, @mallenexpensify Eep! 4 days overdue now. Issues have feelings too...
@francoisl @hungvu193 @mallenexpensify this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!
Not overdue.
Upwork job price has been updated to $250
Not overdue, @francoisl when you're back can you take a look at my comment above. Thanks
@francoisl, @hungvu193, @mallenexpensify Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!
Not overdue
@francoisl @hungvu193 @mallenexpensify this issue is now 4 weeks old, please consider:
- Finding a contributor to fix the bug
- Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
- If you have any questions, don't hesitate to start a discussion in #expensify-open-source
Thanks!
Anytime a receipt or chat attachment is uploaded, Expensify generates additional .1024.jpg and .320.jpg thumbnail versions of it too. I don't know why it's hardcoded to .jpg instead of using the original image's format, that code is over 14 years old, and people who made the original decision no longer work here.
It would be fairly simple to create a .1024.png thumbnail for PNG images and preserve the transparency, however we have a bunch of places in the code that expect .1024.jpg/.320.jpg thumbnails, and more importantly we'd risk breaking thumbnail links for all existing PNG images and receipts.
As much as I agree that removing .1024.jpg from the source image sounds like a workaround, it sounds like the safer option. We could also do a small adjustment and just remove .png.1024.jpg, so that we keep using the thumbnail for other image formats than png.
Updated proposal to only remove .png.1024.jpg or .png.320.jpg
Cool. Let's go with @linhvovan29546 's proposal 🎀 👀 🎀