react-native-community / upgrade-helper

⚛️ A web tool to support React Native developers in upgrading their apps.
https://react-native-community.github.io/upgrade-helper
MIT License
3.67k stars 112 forks source link

Fix encoding issues when using Copy button #365

Closed Menardi closed 1 year ago

Menardi commented 1 year ago

Summary

This PR fixes an issue with the Copy button, where unicode characters were not correctly encoded. This is because the current implementation uses the GitHub API to fetch and decode file contents in base64 format. This results in characters such as the copyright symbol being copied as ©. This is particularly noticeable in android/gradlew, which also uses « and » in its comments, ending up as « and ». This is a common issue when using base64, described as The Unicode Problem on MDN.

One solution to this issue would be to use TextDecoder, but the code can get a bit hard to read because the base64 first needs to be converted to a binary array. I decided to instead fetch the file directly and use its contents, as it avoids any other potential unforeseen base64 issues, as well as slightly reducing the request size (no API overhead).

To make this fix work, I had to change the base url used by getBinaryFileURL to https://raw.githubusercontent.com/. Using https://github.com/.../raw caused CORS failures, which were resolved by changing to the base url. This change also makes the function consistent with the existing getReleasesFileURL and getDiffURL functions, which already use https://raw.githubusercontent.com/.

Test Plan

I tested this in Chrome, Safari and Firefox on MacOS. All of these browsers previously copied unicode text incorrectly. With the changes in this PR, they now correctly copy unicode text.

I also tested that downloading and viewing files still works, as I changed the url for getBinaryFileURL to resolve CORS issues.

What are the steps to reproduce?

To see the issue in the current version, try comparing 0.71.12 to 0.72.1. Go to android/gradlew and press the Copy button. You should see that the copyright symbol incorrectly has an  in front of it.

Doing the same copy on this PR's branch should copy the contents correctly, without this extra Â.

Checklist

pvinis commented 1 year ago

wow, great find and fix! thank you! Screenshot 2023-07-06 at 15 34 12

Menardi commented 1 year ago

Thanks for such a quick review and merge @pvinis!