[LOW] [P2P] Allow for QR codes to be easily saved or copied on all platforms

[MitchExpensify]

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:

On desktop:

1. Click any room header
2. Click "Share code"
3. Right click the QR code

Expected Result:

Have the option to "Save image as", "Copy image", or Download the QR code image

Actual Result:

No option to easily save or copy the QR code image

Workaround:

Copy or save the QR code image on mobile

Platforms:

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

• [ ] Android / native
• [ ] Android / Chrome
• [ ] iOS / native
• [ ] iOS / Safari
• [x] MacOS / Chrome / Safari
• [x] MacOS / Desktop

Version Number: v1.3.19-7 Reproducible in staging?: Yes Reproducible in production?: Yes Email or phone of affected tester (no customers): All Notes/Photos/Videos:

Expensify/Expensify Issue URL: Issue reported by: @MitchExpensify Slack conversation: Internal convo: https://expensify.slack.com/archives/C03U7DCU4/p1685041725518029, #bug-zero convo tbd

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~013d2f9dd751374d8e
• Upwork Job ID: 1663602988972146688
• Last Price Increase: 2023-05-30

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[melvin-bot[bot]]

Job added to Upwork: https://www.upwork.com/jobs/~013d2f9dd751374d8e

[melvin-bot[bot]]

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

[JmillsExpensify]

Great point reminder that we should always optimize for cross-platform everywhere!

[robertjchen]

@chrispader Would you possibly have some free cycles to look into adding QR code download for Web? I recall you mentioning that we may have to make some upstream changes to the library? 🤔

[chrispader]

Yes, i can take a look at that! :)

[chrispader]

Yes, so the react-native-view-shot library currently doesn't work on web, so i'm gonna investigate the problem and open an upstream PR. We could probably also open a separate PR in our an own fork?

[robertjchen]

yup, sounds like a plan! 🙇

[chrispader]

Working on this issue in the upstream library (PR)

[chrispader]

The PR is working and making screenshots on web works now too, though there is one problem.

The library that is creating a canvas from the html canvas doesn't have access to the profile picture/avatar image that is received from CloudFront because of CORS policy regulations.

Is there any way to add these CORS headers to the S3 Bucket/AWS or would this violate Expensify's security?

cc @robertjchen

[chrispader]

Which leads to images like this:

[chrispader]

The Expensify/App PR for this feature is here

[robertjchen]

Yep, I'll look into the CORs changes!

[melvin-bot[bot]]

@robertjchen @chrispader 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!

[chrispader]

@robertjchen are there any updates on the CORS changes?

[robertjchen]

Nothing yet, will report back before EOW 👍

[melvin-bot[bot]]

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

[robertjchen]

Apologies for the delay, some firefighting last week.

[melvin-bot[bot]]

@robertjchen @chrispader this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@robertjchen @chrispader this issue is now 4 weeks old and preventing us from maintaining WAQ. This should now be your highest priority. Please post below what your plan is to get a PR in review ASAP. Thanks!

[melvin-bot[bot]]

@robertjchen, @chrispader 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[robertjchen]

This is on me! I'm looking into this question:

Is there any way to add these CORS headers to the S3 Bucket/AWS or would this violate Expensify's security?

The straight answer is that we generally want to only allow requests from clients on Expensify domains (otherwise someone could potentially use us as an image hosting site by uploading random images and linking them elsewhere).

Don't think the profile photos being hosted elsewhere, so it must be a local CORs issue when rendering... will need to dive a bit deeper 👀

[melvin-bot[bot]]

@robertjchen, @chrispader Eep! 4 days overdue now. Issues have feelings too...

[robertjchen]

Which leads to images like this:

@chrispader could you let me know if/what the CORs error is displayed in the browser console when rendering that screenshot? 🙏

[melvin-bot[bot]]

Triggered auto assignment to @greg-schroeder (NewFeature), see https://stackoverflowteams.com/c/expensify/questions/14418#:~:text=BugZero%20process%20steps%20for%20feature%20requests for more details.

[melvin-bot[bot]]

Triggered auto assignment to Design team member for new feature review - @shawnborton (NewFeature)

[chrispader]

Hey @robertjchen!

There is no error displayed in the browser's console, because thehtml2canvas library internally just isn't able to retrieve any images from a different domain.

html2canvas does not get around content policy restrictions set by your browser. Drawing images that reside outside of the origin of the current page taint the canvas that they are drawn upon. If the canvas gets tainted, it cannot be read anymore. If you wish to load images that reside outside of your pages origin, you can use a proxy to load the images.

https://html2canvas.hertzen.com/proxy

[chrispader]

That's why images from different domains (such as d1wpcgnaa73g0y.cloudfront.net for my profile picture)cannot be rendered and therefore are not in the screenshot...

[chrispader]

To have the profile pictures in the screenshot, we would need to create a proxy that runs inside the "Expensify" Domain, just to retrieve the images from the same "origin"/domain. I guess this would be a way to big overhead, right?

html2canvashas a useCORS config option, that's why i though we might be able to display the images anyway, if the CORS headers are valid.

[greg-schroeder]

What's the status on this one @robertjchen?

[robertjchen]

Thanks for the detailed investigation here! 🙇

Yep- that's the core of the issue- the profile pic is coming out of the CloudFront domain, whereas the New Dot Web app is hosted under new.expensify.com.

💡 I'll look into whether we could create a new subdomain for profile pics e.g. profiles.expensify.com/acc27a...128.jpg. This way we can keep things within the same domain and not have a too permissive CORS policy.

[robertjchen]

Created an internal proposal for infrastructure changes, will report back after that's done.

[chrispader]

@robertjchen from my understanding, the profile pics would also need to be under new.expensify.com. profiles.expensify.com would already be a different origin/domain, see https://en.wikipedia.org/wiki/Same-origin_policy#Origin_determination_rules

[robertjchen]

Yep, but I think we can then relax the origin policy to allow for anything *.expensify.com rather than * 🤔

[chrispader]

hmm, not 100% sure, but yea i think that should be possible 👍

[chrispader]

Where is the origin policy defined/set?

[robertjchen]

It's sent by the server hosting the new.expensify.com site. At the moment, it looks like we're not sending anything which defaults to the Same Origin Policy (so only assets loaded from new.expensify.com specifically is allowed).

[robertjchen]

Awesome, we now have a new assets.expensify.com subdomain:

aka https://d1wpcgnaa73g0y.cloudfront.net/1b2f6f12cc003b3f46c2dd8fbb21bf8e3769e873.png -> https://assets.expensify.com/1b2f6f12cc003b3f46c2dd8fbb21bf8e3769e873.png

Could you help me perform a quick test to see if it works with assets loaded from the assets.expensify.com domain rather than CloudFront? 🙏

[chrispader]

Just tested it with the new domain, but it still doesn't work. I suppose, we'll still have to add the Access-Control-Allow-Origin header with *.expensify.com?

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Access-Control-Allow-Origin

[robertjchen]

Ah, you were right originally- looks like *.expensify.com isn't allowed 🤔

It seems like we have to send the proper CORs headers on-the-fly based on the origin of the request. I'm not sure if we can get these profile pics onto the new.expensify.com domain without some major changes (which might be more work than doing the former).

I'll do some more testing on configuration for the first option and report back!

[greg-schroeder]

Okay where did we land on this one @robertjchen?

[robertjchen]

On vacay, but will try to figure out the remaining hurdle after I get back on Wednesday. We're halfway there!

[greg-schroeder]

Sounds good!

[robertjchen]

Coming up with a plan and getting it buddy-checked, since what I'm planning on doing has the potential to break all of NewDot Web if the settings are right.

[robertjchen]

Plan to be reviewed in the corresponding internal issue: #300566 since it contains backend details that should not be public.

[greg-schroeder]

Thanks @robertjchen!

[robertjchen]

Getting proposal reviewed before I break all of NewDot, should have the changes applied by EOW

[greg-schroeder]

Is there a place I can follow along on that discussion/proposal @robertjchen ?