Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add episode download button to media item controls dropdown. #1247

Merged
merged 3 commits into from
Apr 10, 2024

Conversation

jj-style
Copy link
Contributor

@jj-style jj-style commented Feb 7, 2024

Add a dropdown on episodes to download the episode as an MP3 file.

Fixes #1099.

Tested on Linux on firefox (gecko) and ungoogled-chromium (chromium) based browsers.

Screenshots:

On episode page

episode

On episodes list

list

Download on firefox

firefox

Download on chrome (ungoogled-chromium)

chrome

@mitchdowney
Copy link
Member

Hey @jj-style thank you so much for this PR, and I really sorry for the delay. I have been almost completely out of Podverse development for 3 months, but I'm finally getting back in the swing of things.

There are some issues I am running into with this, and I am wondering if you would be able to help improve it. Part of why I had not worked on this is I wasn't sure if there is a consistent cross browser solution possible. I have tried testing so far with this podcast.

  1. On both Firefox and Chrome, I attempted to download episodes from that podcast from localhost:3000, and I am getting CORS errors in the terminal.

Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at https://traffic.libsyn.com/secure/thejimmydoreshow/TJDS_20240401_Podcast_-_33024_12.33PM.mp3?dest-id=19460. (Reason: CORS header ‘Access-Control-Allow-Origin’ missing). Status code: 504.

Unfortunately since we don't host the mp3s ourselves, we don't have the ability to change the headers of the mp3 file servers. Is this a problem that you think might be specific to my localhost environment, that might go away in prod? Do you have any ideas for fixing this? The only option I can think of would be to proxy requests through our own servers, but that is generally frowned upon by podcasters, and would result in heavy network data transfers that we'd rather not have to support.

However, I was able to get a file to download successfully for this podcast, using the Brave browser. I have a CORS disabling extension on Brave, but even when I turn that off I can still download episodes of this podcast.

  1. In the case where the episode downloaded successfully on Brave, it appears the browser waited to download the entire mp3 before popping up the native UI "Save file" window. For this one episode I ended up pressing Download Episode multiple times, as it appeared that the episode was not downloading, because it took 8 seconds for the download to complete.

Would it be possible to provide an indicator to the user that the download is in progress? We have a ToastsHandler component that might be a good fit for something that says the title of the episode and a 0-100% progress indicator next to it.

  1. If 1 and 2 can be solved, but there are still some browsers where this feature will work, and browsers where it will not, could you make the feature so it only displays on the browsers where the feature is known to work?

Some other thoughts for implementation...

A) Should the "Download Episode" button simply open a new tab, loading directly to the mp3's URL, and then leave it up to the user to determine how to download it from there? Not thrilled about this as people may still write us and say it "doesn't work"...so that might need a modal that pops up one time explaining why they will be navigated to a new tab, and that they will need to figure out how to download from there. Not even sure right now if that UX would acceptable for implementation :[

B) If this feature's usability is heavily browser dependent, maybe we could make a setting for it on the Settings page, with an explanation that it may not work for all web browsers? One hang up there is that currently the Settings page will only display for logged-in / Premium users, so we would have to also make the Settings page display for non-logged in users. That shouldn't be so difficult if it comes down to it, and if it would be an acceptable solution for you, I could work on that myself.

Ok sorry, I don't want to be a total perfectionist about this implementation, but I do want it to at least work in a way where we shouldn't receive support emails and bug reports saying that the Download Episode button doesn't work. Please let me know if you have any thoughts or recommendations.

@jj-style
Copy link
Contributor Author

jj-style commented Apr 7, 2024

Hi Mitch, no worries at all good to see you back. I heard on Linux Unplugged a while back you were having a bit of a tough time with the project so thought I would try help out with some issues!

Sorry I didn't do enough testing on different podcasts/browsers and find these issues.

I will have to do some digging regarding CORS - surely the files must be accessible as the browser is able to stream them when hitting play so maybe there are some headers/different type of request that can be made to bypass the CORS.

Good shout on the indicator yeah I should be able to at the least disable the button while it's downloading to prevent multiple download attempts.

In terms of making it available in certain browsers yeah that should be doable if we define it in some config somewhere. Though it may be difficult to define as there are a lot of browsers and I'm not sure if platform or package (flatpak,snap etc.) would affect anything.

Yeah no worries I agree if you are putting something in production it needs to be robust. I will try and look at the CORS and disabling multiple downloads hopefully those will sort it as it would be unfortunate to have to open the media link in a new tab and then ctrl+s, but that could be done as a fallback

@mitchdowney
Copy link
Member

@jj-style appreciate your help and reply! Again sorry to be difficult with the PR...it just happens that download file buttons seem to be a tricky issue with web browsers.

I will have to do some digging regarding CORS - surely the files must be accessible as the browser is able to stream them when hitting play so maybe there are some headers/different type of request that can be made to bypass the CORS.

I wonder if browsers handle CORS rules differently with "load in audio element" vs "download file"?

This might also be territory where a PWA is intended to handle behaviors that browsers may typically block.

I don't want to trouble you with too much work on this. If there are at least some browsers that "download episode" would definitively work on, I could possibly help with some user agent identification work to target the feature on those browsers...but the issue there is you mentioned that Firefox was working for you, but I ran into CORS issues with Firefox. If you're unable to reproduce those issues feel free to kick it back to me and I'll investigate further. Maybe there is something specific about my computer setup that was giving me problems.

@jj-style
Copy link
Contributor Author

jj-style commented Apr 8, 2024

Yeah this is odd so that podcast is working in Firefox when running on localhost for me.
The response headers have access-control-allow-origin * set which should mean that you can request the content from origins that aren't that website (i.e. podverse). Is that header present on your request?

request

download

I found https://developer.mozilla.org/en-US/docs/Web/HTML/Element/audio#crossorigin attribute on the audio element which I will try although I can't seem to replicate the issue so will try install a different browser and see if I can get the same CORS error you are seeing.

I wonder if browsers handle CORS rules differently with "load in audio element" vs "download file"?

This could be be the case yeah as it streams the audio instead of downloading the blob all in one go.

I'll keep looking around

Edit: just tried on a new install of brave and edge and still not getting any CORS issue :(

@mitchdowney
Copy link
Member

Edit: just tried on a new install of brave and edge and still not getting any CORS issue :(

Ok thanks for investigating. How about, if you can make it so the UI gives some feedback to let the user know a download is in progress, then I'll merge it into prod, and we'll see how it goes? Can always revert the change if it becomes a pain point. Maybe releasing it into the wild will shed some light on browser differences.

@jj-style
Copy link
Contributor Author

jj-style commented Apr 9, 2024

If you're happy to try that that sounds good, just conscious of creating a load of issues from users.

But yeah I will try and add the toast handler to show the progress and possibly disable the button while it is downloading too this evening

Just out of interest do you know why download episode text appears red? I could not seem to find where/why this was happening or how to change it

@jj-style
Copy link
Contributor Author

jj-style commented Apr 9, 2024

Hi @mitchdowney,

I have had a go implementing the toasts (info at start of download, success/error at the end depending on result).
See the video and screenshot below for examples - let me know what you think :)

success:
success

error:
error

Edit: sorry for the grainy GIF - it recorded in webp and I butchered the ffmpeg command to convert it!

@mitchdowney
Copy link
Member

Looks great! I'll deploy it to prod now.

@mitchdowney mitchdowney merged commit e40394a into podverse:develop Apr 10, 2024
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.

MP3 Download via Web
2 participants