jimmywarting / StreamSaver.js

StreamSaver writes stream to the filesystem directly asynchronous
https://jimmywarting.github.io/StreamSaver.js/example.html
MIT License
3.97k stars 413 forks source link

Add Android into the list of blob fallback #270

Closed gwdp closed 2 years ago

gwdp commented 2 years ago

Simple and dirty...

Possibly closing: https://github.com/jimmywarting/StreamSaver.js/issues/195 and https://github.com/jimmywarting/StreamSaver.js/issues/236

jimmywarting commented 2 years ago

Android devices that runs built in chrome, brave or firefox should not have to use the blobFallback strategy. It only makes it worse for those who already supports streaming (mostly by big giants). this will only solve it for some app developers that uses WebViews.

I rather prefer to enable developer to turn on use blobFallback instead so they can do there own detection.

But really it should be a more better feature detection if it can or can't use service worker to mimic a server. not by browser sniffing

gwdp commented 2 years ago

Thanks for your reply @jimmywarting

As fair as my tests went, I can assure streamsaver didn't work for me on any Chrome environment, being Android embedded webview, Chrome app on Android or even on iOS.

Checking more closely, it seems it does support all js features, but the behavior of the browser is to close the stream right after first bytes were received. So, not sure if can achieve this detection by any other means.. (but agree it could be improved)

Also, I didn't got the "It only makes it worse for those who already supports streaming".

jimmywarting commented 2 years ago

Also, I didn't got the "It only makes it worse for those who already supports streaming".

dose android devices that supports streaming data to the device can write chunks of data all the time... when the blobFallback is being used then browsers needs to keep the data in memory to generate a blob and download everything at once, hence making it worse

gwdp commented 2 years ago

On my understanding, there were no Android devices that would support this. Can you confirm you have seen this working on Android?

I have tested on Chrome app and embedded webview on a few Android devices (emulator only); Download always fail. (Tested using "official" examples via GitHub link). That's why that assumption on the user agent was right for me.

I understand the blob fallback concept, but rather having this than not working at all.

gwdp commented 2 years ago

Hey @jimmywarting, just some additional information around this.. I'm still in the works to mitigate this issue.

Narrowing down a little more the issue, and it seems to happen on all mobile Chrome browsers when not using secure context.

First test, If I manually set isSecureContext to true, even when not on SSL, it does work on mobile chrome browsers (non-ssl).

Going further, and removing isSecureContext conditional from this test, make things work again on non-secure contexts (mobile Chrome). https://github.com/jimmywarting/StreamSaver.js/blob/5b9ef44b8b388ee95aae0f0b2feb5be5830b9463/StreamSaver.js#L96

Tracking down this change and it seems we could have this conditional removed from there, but unsure what other consequences it would bring for other environments and cases and I'm not seeing and testing at the moment. Could you advise me if removing this would be something acceptable or we should have another test for this specific case (mobile Chrome, non SSL)