Skip to content

Conversation

@danferns
Copy link
Contributor

@danferns danferns commented Jun 7, 2021

This PR enables loading samples with file names like "C#5.wav".

Currently, this throws an error because special characters like "#" need to be encoded in the right format before being passed into fetch() as the URL.

The solution is to use encodeURIComponent() to convert these characters into their UTF-8 encoded escape sequences (eg. "#" -> "%23"). This is done for each level of the file path to also encode any special characters that may be present in the folder names.

file names with special symbols like "C#5.wav" would not get parsed correctly. used encodeURIComponent() for each level of the file path to fix that.
@danferns danferns marked this pull request as draft June 8, 2021 06:14
danferns added 2 commits June 8, 2021 14:58
encodeURIComponent() used to encode the URL protocol ("https://" etc.) as well, causing errors when loading external samples and data URIs. Now, it only encodes the file path.
@danferns
Copy link
Contributor Author

danferns commented Jun 8, 2021

I found that the previous solution did not handle non-relative URLs properly. It would encode the colon in the protocol (eg "https:" -> ""https%3A"), which threw errors when loading external samples as well as when loading data URIs.

I have fixed this by creating a Location object through the <a> tag. This separates the domain, the protocol and the file path.

Now, only the file path is passed into encodeURIComponent(). And since location.hash stores the substring of the URL containing the # symbol and everything after it, the full file path becomes location.pathname + location.hash.

I have tested this with external URLs, relative URLs, and a small wav data URI. It works as expected in all three cases! :)

@danferns danferns marked this pull request as ready for review June 8, 2021 11:19
@tambien tambien merged commit f08b317 into Tonejs:dev Oct 13, 2021
@tambien
Copy link
Contributor

tambien commented Oct 13, 2021

thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants