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

Streamsaver does not clean up iframes #134

Open
acuntex opened this issue Nov 22, 2019 · 7 comments
Open

Streamsaver does not clean up iframes #134

acuntex opened this issue Nov 22, 2019 · 7 comments

Comments

@acuntex
Copy link

acuntex commented Nov 22, 2019

When using StreamSaver.js in a long living SPA, you see that StreamSaver.js does not cleanup the iframes, thus poluting the DOM.

Repro: Just download and look at the dom :-)

@jimmywarting
Copy link
Owner

I tried to clean it up before but for some reason it didn't work so well,
Don't remember what it was exactly, in some edge cases the download never started if the iframe where removed before any bytes have been written to the iframe - so it would just be safer to leave it there, all it is is an empty iframe - not consuming any dom, js or css

backlog: #94

@acuntex
Copy link
Author

acuntex commented Nov 23, 2019

Just an idea: Can we somehow tag the iframes with a custom attribute and a timestamp? So that an application can identify these iframes and clean up "old" iframes.

I've noticed performance issues if there are more than 100 iframes. Not 100% sure if it's because of this but this is my best guess.

@jimmywarting
Copy link
Owner

jimmywarting commented Nov 23, 2019

sure, we could "tag" them with something. <iframe data-streamsaver> maybe?
otherwise another solution is to get them with src css-selector

$(iframe[src^="${streamSaver.mitm.replace(/mitm.*/, location.hostname)}"])

^ means startsWith

@acuntex
Copy link
Author

acuntex commented Nov 28, 2019

Yes, but i would maybe also include a timestamp... just in case so that an automatic cleanup by the application does not kill the current download.

@jimmywarting
Copy link
Owner

fyi when native file system gets implemented, then we might not need iframes or service workers :)

@jimmywarting
Copy link
Owner

jimmywarting commented Dec 18, 2019

more than 100 iframes

why don't you zip everything?

@acuntex
Copy link
Author

acuntex commented Dec 18, 2019

fyi when native file system gets implemented, then we might not need iframes or service workers :)

yay :-)

why don't you zip everything?

It's a timing issue. This would be only possible if all files would be present at the same time. In a long running application this might not be the case :-(

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

No branches or pull requests

2 participants