improve download, allow download of big files, works on ff and chrome #5466

kittaakos · 2019-06-14T08:49:48Z

This is the follow-up of #4890

This PR improves the download by letting the native browser handle it
instead of fetching it in the background. This also adds a unique link
which expires in 1 minute. Tested on chrome 73.0 and
firefox 60.5

Signed-off-by: Uni Sayo unibtc@gmail.com

kittaakos · 2019-06-14T10:25:13Z

I have tried it in Gitpod:

I could download a file,
multiple ones and folders (packed into a .tar),
download while another download was in progress,
also, I could download the entire Theia workspace without blocking the UI or experiencing offline on the status bar.
The Copy Download Link worked as well.

@svenefftinge, please review/approve. I am not allowed to do it for a PR with my own changes. Thanks!

akosyakov

It's strange that i have to press download twice to download.

Do we really need Copy Download Link? If so i would prefer to have a separate menu item. If i suppose to share it 1m timeout seems to be really small.

packages/workspace/src/browser/workspace-commands.ts

akosyakov · 2019-06-19T12:47:20Z

Generally that's really good, UI is not blocked at all during preparing download.

kittaakos · 2019-07-04T06:44:40Z

Do we really need Copy Download Link? If so i would prefer to have a separate menu item. If i suppose to share it 1m timeout seems to be really small.

From @uniibu:

And lastly, to be able to use a separate downloader, you will need the download link, so I added a Copy Download Link menu option which works for singe and multiple(tar) files/folders

kittaakos · 2019-07-04T06:48:38Z

It's strange that i have to press download twice to download.

I did not experience this when I was updating the PR, so I have retested it again; it works. (Tried in Chrome and FF with Gitpod.)

akosyakov · 2019-07-05T08:41:25Z

@kittaakos I meant that i have to click Download in the navigator and then in the notification again. For a single file it does not make sense. I'm not sure whether notification makes sense at all, we could simply start downloading and a user can cancel it via the browser.

uniibu · 2019-07-05T18:41:57Z

Do we really need Copy Download Link? If so i would prefer to have a separate menu item. If i suppose to share it 1m timeout seems to be really small.

From @uniibu:

And lastly, to be able to use a separate downloader, you will need the download link, so I added a Copy Download Link menu option which works for singe and multiple(tar) files/folders

Hi! Its been awhile.. Just want to be clear that, If my memory serves me right, the Download Link acts like a temporary url shortener so you can use it to download big files in parallel using 3rd party downloader and is not meant to be used for sharing. IMO, the 1 minute expiry should be enough for someone to copy it and paste it on their downloader. Even if the link expires as long as the download is already started, it will not be interrupted.

kittaakos · 2019-07-05T20:17:58Z

use it to download big files in parallel using 3rd party downloader

👍 This. Thank you for the clarification.

akosyakov · 2019-07-08T07:22:57Z

But a special case should not harm the standard scenario? Is it good that a user has to click twice and see notification for a single file?

As for Copy Download Link should not it be a separate menu item and then you immediately get a link which you paste somewhere?

This PR improves the download by letting the native browser handle it instead of fetching it in the background. This also adds a unique link which expires in 1 minute. Tested on chrome 73.0 and firefox 60.5 Signed-off-by: Uni Sayo <unibtc@gmail.com>

kittaakos · 2019-07-10T09:28:59Z

I have made the following changes:

From now on there are two separate commands: Download and Copy Download Link.
Download remains in the 6_downloadupload menu group.
Copy Download Link has been added to the Edit > 2_clipboard main menu group, and to the 5_cutcopypaste group for the Navigator context menu.
When Download is selected, the download will start automatically right after the prepare phase. Neither further user interaction is required, nor the user will be notified.
When Copy Download Link is selected, the link will be copied to the clipboard.

kittaakos · 2019-07-10T09:47:08Z

Copy download link does not always work on FF.
If it works, it does not copy the link to the clipboard. I do not know if that was the case before my changes. I have to check.

kittaakos · 2019-07-10T09:48:40Z

Copy download link does not always work on FF.

The notification is discarded immediately, it is hard to see for small file(s).
But the link is not copied to the clipboard.

document.execCommand(‘cut’/‘copy’) was denied because it was not called from inside a short running user-generated event handler.

kittaakos · 2019-07-10T10:39:03Z

I do not know how the copy/paste works in FF, but it seems, it is entirely unsupported the way we do: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/Interact_with_the_clipboard

kittaakos · 2019-07-10T11:15:26Z

Updating the copy/paste functionality is not straightforward, as we do not have type definitions with the current typescript@3.1.3 version. However, it is already available within the most recent TypeScript.
See here: https://github.com/microsoft/TypeScript/blob/a4cddd4647154550fe603ec2b65a93cf8b2099dd/lib/lib.dom.d.ts#L10509

kittaakos · 2019-07-10T13:22:22Z

So it seems, in FF one has to actually click on a button to invoke the clipboard-copy, I am going to disable it if the browser is not Chrome.

kittaakos · 2019-07-10T13:23:58Z

PoC: https://jsfiddle.net/kittaakos/ru18g04p/
It works in Chrome but not in FF. One has to remove display: none and type into the input, lose the focus and see the the text from the input was not copied to the clipboard.

From now on, either the download link can be copied to the clipboard or the download can be triggered without any further user interaction. Note: `Copy Download Link` works in Chrome only. #5466 (comment) Signed-off-by: Akos Kitta <kittaakos@typefox.io>

kittaakos · 2019-07-10T13:34:18Z

Ready for the review.

akosyakov

Download worked smooth, without double clicking. 👍

Copy download link worked but was confusing that a link is only in the clipboard after download is prepared. Would be better to pregenerate a link and give it immediately. Otherwise on big downloads it looks broken first time.

I'm going to approve it since it don't care much about Copy Download Link right now. Someone can improve it later.

kittaakos · 2019-07-12T09:25:49Z

Thank you for your help, @uniibu 🥇

502647092 · 2019-07-13T03:11:33Z

@kittaakos @akosyakov
Copy Download Link Break Ctrl + C on edit

kittaakos mentioned this pull request Jun 14, 2019

[filesystem] improve download and allow downloading of big files and parallel downloads. #4890

Closed

svenefftinge mentioned this pull request Jun 14, 2019

Download trouble gitpod-io/gitpod#414

Closed

kittaakos marked this pull request as ready for review June 14, 2019 09:55

kittaakos requested a review from svenefftinge June 14, 2019 10:25

akosyakov reviewed Jun 19, 2019

View reviewed changes

packages/workspace/src/browser/workspace-commands.ts Show resolved Hide resolved

akosyakov added the filesystem issues related to the filesystem label Jul 5, 2019

kittaakos force-pushed the download-big branch from 4fc9345 to fe8b3b1 Compare July 10, 2019 09:22

kittaakos force-pushed the download-big branch from fe8b3b1 to 81fcfd2 Compare July 10, 2019 13:34

akosyakov approved these changes Jul 12, 2019

View reviewed changes

kittaakos merged commit b2fbe6e into master Jul 12, 2019

kittaakos deleted the download-big branch July 12, 2019 09:25

This was referenced Jul 13, 2019

Copy and Paste is Break by #5466 #5706

Closed

Direct Download Url Support #5707

Closed

kittaakos mentioned this pull request Jul 15, 2019

Revert "improve download, allow download of big files, works on ff and chrome" #5708

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

improve download, allow download of big files, works on ff and chrome #5466

improve download, allow download of big files, works on ff and chrome #5466

kittaakos commented Jun 14, 2019

kittaakos commented Jun 14, 2019

akosyakov left a comment

akosyakov commented Jun 19, 2019

kittaakos commented Jul 4, 2019

kittaakos commented Jul 4, 2019

akosyakov commented Jul 5, 2019

uniibu commented Jul 5, 2019

kittaakos commented Jul 5, 2019

akosyakov commented Jul 8, 2019

kittaakos commented Jul 10, 2019

kittaakos commented Jul 10, 2019

kittaakos commented Jul 10, 2019 •

edited

Loading

kittaakos commented Jul 10, 2019

kittaakos commented Jul 10, 2019 •

edited

Loading

kittaakos commented Jul 10, 2019

kittaakos commented Jul 10, 2019

kittaakos commented Jul 10, 2019

akosyakov left a comment

kittaakos commented Jul 12, 2019

502647092 commented Jul 13, 2019

improve download, allow download of big files, works on ff and chrome #5466

improve download, allow download of big files, works on ff and chrome #5466

Conversation

kittaakos commented Jun 14, 2019

This is the follow-up of #4890

kittaakos commented Jun 14, 2019

akosyakov left a comment

Choose a reason for hiding this comment

akosyakov commented Jun 19, 2019

kittaakos commented Jul 4, 2019

kittaakos commented Jul 4, 2019

akosyakov commented Jul 5, 2019

uniibu commented Jul 5, 2019

kittaakos commented Jul 5, 2019

akosyakov commented Jul 8, 2019

kittaakos commented Jul 10, 2019

kittaakos commented Jul 10, 2019

kittaakos commented Jul 10, 2019 • edited Loading

kittaakos commented Jul 10, 2019

kittaakos commented Jul 10, 2019 • edited Loading

kittaakos commented Jul 10, 2019

kittaakos commented Jul 10, 2019

kittaakos commented Jul 10, 2019

akosyakov left a comment

Choose a reason for hiding this comment

kittaakos commented Jul 12, 2019

502647092 commented Jul 13, 2019

kittaakos commented Jul 10, 2019 •

edited

Loading

kittaakos commented Jul 10, 2019 •

edited

Loading