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

Compatibility overhaul #92

Merged
merged 16 commits into from
Apr 12, 2019
Merged

Conversation

TexKiller
Copy link
Contributor

@TexKiller TexKiller commented Apr 1, 2019

Table of Contents

Introduction

This PR is meant to introduce full support to Firefox in all contexts while maintaining (and lightly improving) the existing Chrome/Opera support.
In the process of making this I have discovered and extensively tested many methods of doing each step StreamSaver.js does in all contexts/browsers, and have isolated the best method for each.

I am considering four StreamSaver.js steps:

  1. Starting the Service Worker
  2. Sending message port to the Service Worker
  3. Starting the download
  4. Keeping the Service Worker alive

PS: Step 4 should be unnecessary if Transferable Streams are supported. In step 2, instead of sending only the message port, we would also send the stream.

I am also considering four contexts:

  1. Background page of a Web Extension
  2. Web Extension's page loaded in a tab
  3. HTTP page
  4. HTTPS page

Why is Firefox harder

Firefox requires some stronger methods mainly because of the following reasons:

  • Web Extensions on Firefox run on their own processes, which means they can't interact with Service Workers that are running on regular tab processes (and vice versa);
  • Web Extensions can't (yet) start Service Workers from their own moz-extension://* URLs on Firefox - they can only start Service Workers from external HTTPS sites;
  • Web Extension's background pages on Firefox are incapable of starting downloads without using a tab/window;
  • Web Extension's background pages on Firefox are incapable of opening popups using window.open;
  • Firefox doesn't have a bug that Chrome and Opera have. This bug allows a closed popup to keep a Service Worker alive forever (or until the entire browser is closed).

The first two reasons imply that Web Extensions on Firefox have to either start the Service Worker from a popup (outside of the Web Extension's process, since it will be from a tab with an external HTTPS site) or from the <iframe> (inside the Web Extension's process).

But since the download link will always be from an outside HTTPS URL, if you open it on a popup it will be loaded from outside of the Web Extension's process.
If the Service Worker was started from inside the <iframe>, it won't be available for that tab's process, and the 404 page will be shown.

However, if you load the download link inside another <iframe>, it will still be inside the Web Extension's process, and it will have the Service Worker available.
That way we can have tabs with Web Extension's moz-extension://* pages start both the Service Worker and the download without opening any popups.
Too bad the background page can't start a download from the <iframe> on Firefox 😣

On a side note, if it were possible to start a Service Worker from moz-extension://* URLs, if a download URL from that protocol were loaded on a tab then the Web Extension's process would be used, and it would have access to the Service Worker started from the <iframe>.
But for that the 1344561 bug on bugzilla has to be fixed first.

The fourth reason means popups have to be opened from the background page using Web Extension exclusive functions, like chrome.tabs.create.
Those functions don't provide an easy way to pass a message port to the popup, and the only way I could find of doing that was through a Shared Worker started from the popup iself.

Last but not least, since the closed popup can't keep the Service Worker alive forever on Firefox, we need another way of doing that. We can't ping the Service Worker from HTTP pages (because of security restrictions) or from Web Extension's pages if the Service Worker is started from a popup (because of different processes).
For those situations the only way I could find to ping the Service Worker without keeping a popup open was through a Shared Worker.

Compatibility table

I know this is a lot of text. In an attempt to facilitate the understanding of everything this PR does and why, I have made a (very ugly 😥) table with collapsable cells.
You can click on a cell to read an explanation of what the method does and why the other methods are worse than the chosen one (click on them to see the reason).

Firefox

Chrome/Opera

Web ExtensionWebWeb ExtensionWeb
Back- ground PageTabHTTPHTTPSBack- ground PageTabHTTPHTTPS
Start
Service
Worker
'From
chrome
.tabs.
create
Create a new tab with mitm.html from a domain on HTTPS using the chrome.tabs.create function.

Worse alternatives:

 From
iframe
It is possible (using mitm.html from a domain on HTTPS outside of the Web Extension), but the Service Worker is trapped inside the extension's process and then it is impossible to start the download, at least without an ugly <iframe> inside a popup, which requires loading twice before the download starts (once for the popup page, and again for the download link inside the <iframe>).

One advantage of this, however, is that it opens only one popup instead of two, but the same can be accomplished if we reuse the same port between downloads (maybe in a future PR).
 From
window
.open
Web Extensions on Firefox cannot open popup windows using window.open from the background page.
 From
iframe
Open mitm.html from a domain on HTTPS inside an <iframe>.

Worse alternatives:

'
 From
chrome
.tabs.
create
It is possible (using mitm.html from a domain on HTTPS outside of the Web Extension), but it opens popups, which is not necessary.

One small advantage of this, however, is that it doesn't require starting the download from the <iframe>.
 From
window
.open
It is possible (using mitm.html from a domain on HTTPS outside of the Web Extension), but it requires user interaction and opens popups, both of which are not necessary.
 From
window
.open
Open mitm.html from a domain on HTTPS in a popup using the window.open function.

Worse alternatives:

'From
chrome
.tabs.
create
The chrome.tabs.create function is only available for Web Extensions, and the chosen window.open function facilitates communication with the mitm.html page.
 From
iframe
Pages loaded inside <iframe> on HTTP pages are incapable of interacting with Service Workers.
 From
iframe
Open mitm.html inside an <iframe>.

Worse alternatives:

'From
chrome
.tabs.
create
The chrome.tabs.create function is only available for Web Extensions, and it opens popups, which is not necessary.
 From
window
.open
It is possible, but it requires user interaction and opens popups, both of which are not necessary.
 From
iframe
Open mitm.html inside an <iframe>.

Worse alternatives:

'From
chrome
.tabs.
create
It is possible, but it opens popups, which is not necessary.
 From
window
.open
It is possible, but it opens popups, which is not necessary.
 From
iframe
Open mitm.html inside an <iframe>.

Worse alternatives:

'From
chrome
.tabs.
create
It is possible, but it opens popups, which is not necessary.
 From
window
.open
It is possible, but it requires user interaction and opens popups, both of which are not necessary.
 From
window
.open
Open mitm.html in a popup using the window.open function.

Worse alternatives:

'From
chrome
.tabs.
create
The chrome.tabs.create function is only available for Web Extensions, and the chosen window.open function facilitates communication with the mitm.html page.
 From
iframe
Pages loaded inside <iframe> on HTTP pages are incapable of interacting with Service Workers.
 From
iframe
Open mitm.html inside an <iframe>.

Worse alternatives:

'
 From
chrome
.tabs.
create
The chrome .tabs.create function is only available for Web Extensions, and it opens popups, which is not necessary.
 From
window
.open
It is possible, but it requires user interaction and opens popups, both of which are not necessary.
Send messa- ge port
 'By
Shared
Worker
Send the message port through a Shared Worker that was first created by the popup and is then opened inside an <iframe> on the background page.

Worse alternatives:

  By
iframe
It would be possible if the Service Worker were on the Web Extension's process, but that would complicate the download. See the <iframe> alternative to starting the Service Worker for more information.
  By
popup
It is not possible, since the background page cannot open popups using window.open on Firefox and the tabs created with chrome.tabs.create have no way of receiving message ports directly.
  By
iframe
Send the message port through iframe.contentWindow.postMessage.

Worse alternatives:

 'By
Shared
Worker
It is possible, but not necessary.
 By
popup
It is possible if a popup is first created with window.open to start the Service Worker, but doing that is not necessary.
 By
popup
Send the message port through popup.postMessage.

Worse alternatives:

 'By
Shared
Worker
It is possible, but not necessary.
  By
iframe
It is not possible. See the <iframe> alternative to start the Service Worker for more information.
  By
iframe
Send the message port through iframe.contentWindow.postMessage.

Worse alternatives:

 'By
Shared
Worker
It is possible, but not necessary.
 By
popup
It is possible if a popup is first created with window.open to start the Service Worker, but doing that is not necessary.
  By
iframe
Send the message port through iframe.contentWindow.postMessage.

Worse alternatives:

 'By
Shared
Worker
It is possible, but not necessary.
 By
popup
It is possible if a popup is first created with window.open to start the Service Worker, but doing that is not necessary.
  By
iframe
Send the message port through iframe.contentWindow.postMessage.

Worse alternatives:

 'By
Shared
Worker
It is possible, but not necessary.
 By
popup
It is possible if a popup is first created with window.open to start the Service Worker, but doing that is not necessary.
 By
popup
Send the message port through popup.postMessage.

Worse alternatives:

 'By
Shared
Worker
It is possible, but not necessary.
  By
iframe
It is not possible. See the <iframe> alternative to start the Service Worker for more information.
  By
iframe
Send the message port through iframe .content Window .post Message.

Worse alternatives:

 'By
Shared
Worker
It is possible, but not necessary.
 By
popup
It is possible if a popup is first created with window .open to start the Service Worker, but doing that is not necessary.
Start down- load
 'On
chrome
.tabs.
create
Create a new tab with the download link using the chrome.tabs.create function, and remove it using the chrome.tabs.remove function after the download starts.

Worse alternatives:

  On
iframe
As far as I know it is not possible to start downloads from the background page on Firefox without creating a tab.

It would be possible to have the <iframe> inside a tab created with chrome.tabs.create, but that is bad. See the <iframe> alternative to start the Service Worker for more information.
  On
window
.open
Web Extensions on Firefox cannot open popup windows using window.open from the background page.
  On
iframe
Create a hidden <iframe> with the download link, and remove it after the download starts.

Worse alternatives:

  On
window
.open
It would be possible if the Service Worker were started from a popup, but opening popups is not necessary.
 'On
chrome
.tabs.
create
It would be possible if the Service Worker were started from a popup, but opening popups is not necessary.
  On
iframe
Create a hidden <iframe> with the download link, and remove it after the download starts.

Worse alternatives:

 'On
chrome
.tabs.
create
The chrome.tabs.create function is only available for Web Extensions, and opening a popup is not necessary.
  On
window
.open
It is possible, but opening a popup is not necessary.
  On
iframe
Create a hidden <iframe> with the download link, and remove it after the download starts.

Worse alternatives:

 'On
chrome
.tabs.
create
The chrome.tabs.create function is only available for Web Extensions, and opening a popup is not necessary.
  On
window
.open
It is possible, but opening popups is not necessary.
  On
iframe
Create a hidden <iframe> with the download link, and remove it after the download starts.

Worse alternatives:

 'On
chrome
.tabs.
create
It is possible, but not necessary.
  On
window
.open
It is possible, but opening popups is not necessary.
  On
iframe
Create a hidden <iframe> with the download link, and remove it after the download starts.

Worse alternatives:

 'On
chrome
.tabs.
create
It is possible, but not necessary.
  On
window
.open
It is possible, but opening popups is not necessary.
  On
window
.open
Open a popup with the download link using the window.open function, and close it after the download starts.

Worse alternatives:

 'On
chrome
.tabs.
create
The chrome.tabs.create function is only available for Web Extensions, and opening a popup is not necessary.
  On
iframe
Chrome and Opera won't let Service Workers handle loading of pages inside the <iframe> on insecure pages, so the download can't be started this way.
  On
iframe
Create a hidden <iframe> with the download link, and remove it after the download starts.

Worse alternatives:

 'On
chrome
.tabs.
create
The chrome .tabs.create function is only available for Web Extensions, and opening a popup is not necessary.
  On
window
.open
It is possible, but opening popups is not necessary.
Keep Service Worker alive
'Ping
 from 
Shared
Worker
''*
Ping the Service Worker every 29 seconds from inside the Shared Worker.
* The Shared Worker must have been started by the popup, otherwise it will reside in the Web Extension's process and it wont work.

Worse alternatives:

 Ping
 from
iframe
It would be possible if the Service Worker were started from the <iframe>, but that would create problems. See the <iframe> alternative to starting the Service Worker for more information.
Ping
from
popup
That is not possible without Chrome/Opera's bug.
 Ping
 from
iframe
Ping the Service Worker every 29 seconds from inside the <iframe> with mitm.html.

Worse alternatives:

'Ping
 from 
Shared
Worker
It is possible, but not necessary.
Ping
from
popup
That is not possible without Chrome/Opera's bug.
'Ping
 from 
Shared
Worker
Ping the Service Worker every 29 seconds from inside the Shared Worker.

Worse alternatives:

 Ping
 from
iframe
It is not possible. See the <iframe> alternative to start the Service Worker for more information.
Ping
from
popup
It is not possible without Chrome/Opera's bug.
 Ping
 from
iframe
Ping the Service Worker every 29 seconds from inside the <iframe> with mitm.html.

Worse alternatives:

'Ping
 from 
Shared
Worker
It is possible, but not necessary.
Ping
from
popup
It is not possible without Chrome/Opera's bug.
 Ping
 from
iframe
Ping the Service Worker every 4m30s from inside the <iframe> with mitm.html.

Worse alternatives:

'Ping
 from 
Shared
Worker
It migh be possible (I don't think I tested this), but it is not necessary anyway.
Ping
from
popup
It is possible, but not necessary.
 Ping
 from
iframe
Ping the Service Worker every 4m30s from inside the <iframe> with mitm.html.

Worse alternatives:

'Ping
 from 
Shared
Worker
It migh be possible (I don't think I tested this), but it is not necessary anyway.
Ping
from
popup
It is possible, but not necessary.
Ping
from
popup
  *
Ping the Service Worker every 4m30s from inside the closed popup with mitm.html.
* Only works because of a bug.

Worse alternatives:

'Ping
 from 
Shared
Worker
It migh be possible (I don't think I tested this), but it is not necessary anyway.
 Ping
 from
iframe
It is not possible. See the <iframe> alternative to start the Service Worker for more information.
 Ping
 from
iframe
Ping the Service Worker every 4m30s from inside the <iframe> with mitm.html.

Worse alternatives:

'Ping
 from 
Shared
Worker
It migh be possible (I don't think I tested this), but it is not necessary anyway.
Ping
from
popup
It is possible, but not necessary.

Changes to the code

Here I shall try to explain how the code was changed and why.

StreamSaver.js

Three new detections were added alongside the secure context detection: moz-extension: protocol detection, background page detection and Firefox detection. Those are necessary for the branching that happens later.

The secure context detection now results in true for Web Extensions pages on Firefox that are on open tabs (the background page is excluded). This facilitates the branching, since Web Extension pages on tabs on Firefox should function in a very similar way to the secure pages.

A new parameter was added to indicate the URL of the ping.html page, used to connect to the Shared Worker needed in some contexts.

The code that was responsible for creating and sending a message to the <iframe> on HTTPS pages was put in its own function, so it can be used in other places where the <iframe> is needed (like for the <iframe> with ping.html).

The code that was responsible for starting the download was merged into one function with the code that would open the popup. This function loads an URL in the correct place, which can be:

This is where most of the branching actually happens.

A Math.random() value is now inserted into the hash of the mitm.html popup created by the background page of a Web Extension on Firefox. This hash serves two purposes:

  1. Let the page know when it needs to load the Shared Worker;
  2. Let the Shared Worker identify the right Service Worker instance when being messaged from the background page.

sw.js

The message port received when registering a download is now stored as a property of the stream.
This is later used to notify StreamSaver.js when the Service Worker is used to start the download (this message triggers the removal of the download link's popup/<iframe> when necessary).

The 'Mocking a download request' message is removed so it won't interfere with the message that warns about the download starting.

After the Service Worker responds to a request for the download link it sends a message back to StreamSaver.js to notify about the download starting, as mentioned above.

mitm.html

For this to work on every scenario, a couple of changes were necessary.

Before this, the Service Worker was only registered after receiving the message to register a download. However, since the background page of Web Extensions on Firefox has no way to transfer ports to mitm.html directly, that would never happen.

Therefore, we would need the Shared Worker to be started right away, so the background page doesn't have to wait for a random amount of time until it is. Since it is also in the Shared Worker that the "pinging" happens, I have altered the code to start the Service Worker right away regardless of any messages (in the meantime, messages are stored for later processing). Once the Service Worker is registered, only for the context of Web Extension's background pages the Shared Worker is also started (only for this context; the hash is what tells this is the case).

This also has the benefit of only trying to register/recover the Service Worker once, insted of once per download.

The only other change is on the setInterval time: now it is 29 seconds if on Firefox, and 4.5 minutes everywhere else.

ping.html

This is a new file created to be a bridge to the Shared Worker, mainly to support "pinging" the Service Worker from unsafe contexts or different processes.

It is a VERY simple code that only starts the Shared Worker and forwards any message it receives to it.

ping.js

This is a new file created to be the code of the Shared Worker.

It serves two purposes:

  • Pinging the Service Worker to keep it alive;
  • Transfering message ports to/from different processes to allow for the Web Extension's background pages on Firefox to communicate with the Service Worker directly.

Changes to functionality

Here I shall try to explain how the workings of the library were changed and why.

Any context not listed here should keep the previous functionality unchanged.

Download from <iframe>

Any place where the download was being started by navigating to the download link is now changed to load the download link inside the <iframe> and remove the <iframe> after the download starts.
This was done because navigating away from the page closed all open connections, and that sometimes affects other code not from StreamSaver.js.

Pinging on Chrome/Opera

The pinging was happening every 29 seconds for Firefox compatibility, but now StreamSaver.js pings every 29 seconds only on Firefox, restoring the 4.5 minutes ping on Chrome/Opera.

Web Extension's background page on Chrome/Opera

The download was started with a popup created with the chrome.tabs.create function.
Now it is started with <iframe>.
This means no popups are ever opened on this context.

HTTP pages on Firefox

The Service Worker was getting killed after about 30 seconds, since nothing was keeping it alive.
Now, the Shared Worker is used to keep it alive for as long as necessary.

Web Extensions on Firefox

Web Extensions on Firefox were not supported before, but now they are.
Web Extension's pages loaded on tabs are capable of starting downloads without opening a single popup, but background pages need to open two popups per download.
No user interaction is required, though.

Examples for testing

I have created some example pages / Web Extensions to test all the changes in all contexts.

Each test has its own separate URL path for mitm.html and sw.js, so you can test them all simultaneously without one affecting the other.

Web Extension's background page download

This is a simple Web Extension. The only thing it does is start a download from the background page, transfering one 'a' letter per second during the course of one hour.

Download the extension and extract it: https://texkiller.eu.org/StreamSaver/compatibility-back.zip

How to test it

Firefox v65+
Chrome v52+
Opera v39+
  • Navigate to the URL about:debugging#addons
  • Make sure the "Enable add-on debugging" checkbox is marked
  • Click on "Load Temporary Add-on...
  • Select the file manifest.json inside the extracted folder
  • Navigate to the URL chrome://extensions/
  • Make sure the "Developer mode" checkbox is marked
  • Click on "Load unpacked"
  • Select the extracted folder
  • Navigate to the URL opera://extensions/
  • If you don't see the button labeled "Load unpacked extension...", click on "Developer mode"
  • Click on "Load unpacked extension..."
  • Select the extracted folder
  • The sample extension should load and the download should be started automatically from the background page

Web Extension's page on a tab download

Another simple Web Extension. The only thing it does is open a page from the Web Extension into a tab with a button that, when pressed, starts a download transfering one 'a' letter per second during the course of one hour.

Download the extension and extract it: https://texkiller.eu.org/StreamSaver/compatibility-tab.zip

How to test it

Firefox v65+
Chrome v52+
Opera v39+
  • Navigate to the URL about:debugging#addons
  • Make sure the "Enable add-on debugging" checkbox is marked
  • Click on "Load Temporary Add-on...
  • Select the file manifest.json inside the extracted folder
  • Navigate to the URL chrome://extensions/
  • Make sure the "Developer mode" checkbox is marked
  • Click on "Load unpacked"
  • Select the extracted folder
  • Navigate to the URL opera://extensions/
  • If you don't see the button labeled "Load unpacked extension...", click on "Developer mode"
  • Click on "Load unpacked extension..."
  • Select the extracted folder
  • The sample extension should load and a tab should show a page with a button
  • Click the button to test

Web page on HTTP

This is a simple web page on the HTTP protocol.
It shows a button that, when clicked, starts a download transfering one 'a' letter per second during the course of one hour.

How to test it

Firefox v65+
Chrome v52+
Opera v39+

Web page on HTTPS

This is a simple web page on the HTTPS protocol.
It shows a button that, when clicked, starts a download transfering one 'a' letter per second during the course of one hour.

How to test it

Firefox v65+
Chrome v52+
Opera v39+

Things to look forward to

Here I shall mention changes in browsers that could open up new alternatives to the currently selected.

Transferable Streams

Explanation: https://github.com/whatwg/streams/blob/master/transferable-streams-explainer.md

Transferable Streams are expected to ease some of the library aspects, such as:

  • There should no longer be a need to keep the Service Worker alive by pinging;
  • There should no longer be a need to keep a shared message port between the page and the Service Worker;

A Chrome implementation seems to be underway, and should be on stable Chrome in one of the next versions.
I could find no bug on Bugzilla that addresses this matter specifically, though, so Firefox might take some time to get there.

Starting Service Worker from moz-extension://*

1344561 bug on Bugzilla: https://bugzilla.mozilla.org/show_bug.cgi?id=1344561

With this bug fixed, it should be always better to start the Service Worker from the <iframe> instead of the chrome.tabs.create function on Web Extensions on Firefox.

This would mean background pages require one less popup there, and Web Extension's pages on tabs might be able to start the download by just navigating to the download URL.

The Shared Worker would be unnecessary on Web Extensions, with HTTP pages on Firefox being the only situation where it would be needed.

@TexKiller
Copy link
Contributor Author

TexKiller commented Apr 1, 2019

Wow, this took a loooong time to make, and it is huge. I'm sorry for that. :/

@jimmywarting
Copy link
Owner

image

I'm sure it would have taken you a long time to make it. Need to test this, but not right now.

I tried this two on my Android using Brave
https://texkiller.eu.org/StreamSaver/compatibility/https/
http://texkiller.eu.org/StreamSaver/compatibility/http/

Did work just fine. But when i tried it out on my desktop chrome browser i started to notice an error. It is not something wrong with the streamsaver or your code but it's the stream polyfill. You see, chrome have already implemented Transfarable, Transform & Writable streams so it's likely that the polyfill is not needed then, but it overwrites the global stream apis anyway. That causes a problem. readable streams are longer no longer transfarable even doe the variable transfarableSupport say that it's supported (but that was b/c streamsaver.js was loaded before the polyfill) so the streamsaver fails

It's also a problem with a mix of native+polyfilled version
You can't pipe a fetch response to a unnecessary polyfilled WritableStream

rs = new ReadableStream() // polyfilled
ws = new WritableStream() // polyfilled

rs.pipeTo(ws) // ok (polyfilled <-> polyfilled)
rs = await new Response('').body // native
rs.pipeTo(ws) // fail (native <-> polyfilled)

Think we should do something about this problem, eg store the native TransformStream before the polyfill loads or something. The polyfill is still required by does that don't have WritableStream. Some browser only started out with ReadableStream

@TexKiller
Copy link
Contributor Author

TexKiller commented Apr 1, 2019

Oh, I see you have the "Experimental Web Platform features" flag enabled to test the transferable streams on Chrome. I hadn't thought of testing that, so I didn't try to fix that problem.

I have now included a very small commit to fix that. It works on my Google Chrome v73.0.3683.86 32bits with the "Experimental Web Platform features" flag enabled.

I have basically renamed and used the transferableSupport compatibility variable to store the compliant TransformStream class if transferable streams are supported, and then I use it to create the TransformStream.

@TexKiller
Copy link
Contributor Author

I've just noticed that the anchor links from the Table of Contents weren't working. They are now.

@jimmywarting
Copy link
Owner

also notice that the stream polyfill also has a ponyfill, another idea that comes from some other packages are like

import fetch from 'node-fetch'
fetch.promise = bluebird

what if we made that for streamsaver?

streamsaver.writableStream = require('web-stream-polyfill/ponyfill').WritableStream

Thoughts?

@TexKiller
Copy link
Contributor Author

TexKiller commented Apr 1, 2019

I think that is a nice idea. :)

In addition to that, in order to easily support older browsers, we could also check for the global WebStreamsPolyfill variable. That way the ponyfill could be just loaded with <script src="https://cdn.jsdelivr.net/npm/[email protected]/dist/ponyfill.js"> and nothing else would be required.

I think we should still keep a reference to the TransformStream class from commit 4f52692, since this way many people that use the polyfill would still have it working (as long as they load StreamSaver.js before the polyfill).

Do you agree? Do you want me to include those changes on this PR?

@jimmywarting
Copy link
Owner

Yaa i agree
The default value would be

Streamsaver.writablestream = window. Writablelestream || WebStreamPolyfill.Writablestream

@jimmywarting
Copy link
Owner

jimmywarting commented Apr 1, 2019

If you could include it it would be great

Need to update the readme also...

Maybe do the same for transform also? Streamsaver.transform = something

@TexKiller
Copy link
Contributor Author

TexKiller commented Apr 1, 2019

Done.

Also, all the Examples for testing have been updated to use the ponyfill instead of the polyfill.

Edit: Didn't see your comment about the readme and the transform... I'll do those too.

@TexKiller
Copy link
Contributor Author

TexKiller commented Apr 1, 2019

Do you want me to switch the polyfill for the ponyfill on the readme and on the example?

Edit: Sorry about the force-pushes bellow. I was just fixing some typos and identation on the transform commit, and didn't want to leave a bunch of commits behind.

@TexKiller TexKiller force-pushed the compatibility branch 2 times, most recently from 3271ecb to f5f0a0c Compare April 1, 2019 22:38
@jimmywarting
Copy link
Owner

can leave the readme out until we make a new release on gh-pages and on npm and Tag it

@TexKiller
Copy link
Contributor Author

TexKiller commented Apr 8, 2019

Sorry for the time without advancing this, I have another project that is taking up all of my time, but it should be done in about two days and I'll get back to this.

@jimmywarting
Copy link
Owner

It's okey. Don't feel pressured.

@TexKiller
Copy link
Contributor Author

TexKiller commented Apr 12, 2019

Ok, so... Just found another problem.

Apparently, Chrome is also picky on security. Turns out the <iframe> inside HTTP pages ignores Service Workers, and loads the 404 page.

You can (not anymore) try it on the HTTP example on Chrome: it should never start the download. Make sure the old version isn't cached.

I think maybe we'll have to open the download on a popup there. On Firefox it seems to work the way it is now in every context, and the other contexts seem to work fine on Chrome. I'll do some testing with the popup.

Edit: It seems to be working with the popup on HTTP on Chrome. I'll update all the tests, clean up all the browsers and test everything again.

@TexKiller
Copy link
Contributor Author

TexKiller commented Apr 12, 2019

And yet another problem:

It seems Firefox sometimes won't execute intervals in time (apparently due to load, I think), so the Service Worker was dying before the pinging happened.

I'm thinking of changing the delay for the ping interval on Firefox to 10secs, so there is no chance it doesn't happen at least once in 30 seconds. What do you think?

@jimmywarting
Copy link
Owner

Apparently, Chrome is also picky on security. Turns out the <iframe> inside HTTP pages ignores Service Workers, and loads the 404 page.

I did know that already, hence why i used a popup in the first place. the spec says if the top origin is not a secure context it should prevent any service worker from installing

@TexKiller
Copy link
Contributor Author

I knew we couldn't install Service Workers there, I just didn't know a Service Worker installed on a popup would be incapable of intercepting requests made there. It intercepts them fine on Firefox.

Anyway, what do you think about the delay on the ping interval on Firefox? That seems to be the only thing left to fix before it all works.

@jimmywarting
Copy link
Owner

fine by me

@jimmywarting
Copy link
Owner

Cool! All done?

Any breaking changes that require a major bump?

Summary of what have change?

@TexKiller
Copy link
Contributor Author

Well, I think it should all work now, but I would like to run all the tests again on all platforms to be sure. I'll be done with that in one hour, I think.

What has changed on this PR is all listed on Changes to the code and Changes to functionality. Just ignore the stuff about navigating to the download link, as that has been removed.

@TexKiller
Copy link
Contributor Author

TexKiller commented Apr 12, 2019

Ok, tests done. Found a couple of peculiarities this time:

  • Chrome with the Transferable Streams flag set won't start the download on HTTP only. Other protocols are fine with Transferable Streams, and without it all protocols work. I don't know why that is happening now, it didn't happen before. Maybe the popup is messing with something.

  • Version 58 of Opera won't allow the download popup to be opened on HTTP pages (only the mitm.html popup opens, the intercept-nr one is blocked). Version 40 of Opera, however, does allow it to open. I think they might have applied a security patch somewhere in between, and that security patch is not on Chrome, so Chrome doesn't have this problem.

@TexKiller
Copy link
Contributor Author

TexKiller commented Apr 12, 2019

I have updated the PR description to reflect the recent changes.

I have also changed all examples to include the code from example.html, as well as the simple button I was using. All the tests I ran were using my button, though, so there could be issues with some of the default examples that were not detected when I tested.

@jimmywarting jimmywarting merged commit e5a24a7 into jimmywarting:master Apr 12, 2019
@TexKiller TexKiller deleted the compatibility branch April 12, 2019 19:15
@clivebor
Copy link

clivebor commented May 7, 2019

Hi @jimmywarting

Thanks for this project, it's working great for us.

Will you be creating a new release with this compatibility overhaul?

@jimmywarting
Copy link
Owner

Have some more things i have to deal with before making a new release

VadimZhiltsov added a commit to VadimZhiltsov/StreamSaver.js that referenced this pull request May 15, 2019
* WritableStream is not necessary

* Update README.md

* Compatibility overhaul (jimmywarting#92)

* Compatibility overhaul

* Use transferable TransformStream regardless of polyfill loaded after StreamSaver.js

* Allow setting WritableStream class and check for ponyfill

* Allow setting TransformStream class

* lock TransformStream

* updated the example.html with ponyfill

* Use iframes to download

* sw.js claim immediately

* now using isSecureCotext checking to decide if it should use popup or iframe to install sw.js

* Start download on popup for insecure pages not on Firefox

* Change ping interval delay on Firefox to 10secs

* Tunnel readableStream through MessageChannel

Main -> MS -> sw.js

closes jimmywarting#94

* Update README.md

* Update README.md

* Update README.md
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.

3 participants