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

explorer: support for dnd remote resources #98817

Closed
wants to merge 3 commits into from

Conversation

isidorn
Copy link
Contributor

@isidorn isidorn commented May 29, 2020

Fixes #93599

This PR is Work in Progress and it:

  • Introduces a new remote type transfer CodeRemoteFiles
  • Handles this on the Explorer side

What is missing: as @bpasero pointed out "local window needs to use some service (we probably have to add it) to resolve the content. This service could broadcast to windows (via electron-main) and ask the remote window to provide the contents".

Currently the explorer simply uses the workingCopyService which forwards everything to the fileService and the explorer simply passes the correct remote URIs.
So where would be the best place to live for this service that communicates to the other window?
Could this be done as part of the fileService?

Alternative is for the explorer to treat remote resources differently and use a remoteWorkingCopyService to copy remote resources.

@bpasero let me know what you think
fyi @steven166

@bpasero
Copy link
Member

bpasero commented May 29, 2020

Could this be done as part of the fileService?

Tricky, the file service might already have a provider registered for the vscode-remote scheme, so I wonder how this would actually work given you could have 2 windows opened with different remotes?.

Interestingly @mjbvz brought up a related question in b84597e where he needs to access the contents of a remote file from the electron-main side (if I understood correctly). He pinged Alex who maybe has ideas too.

I was initially worried that putting the contents of the entire file into the transfer might be problematic, but now that I have learned about File and its stream capabilities, I wonder if there is any other way to transport this data over using standard browser mechanisms that does not require the full text to be there in memory. Maybe something to explore as alternative?

@isidorn
Copy link
Contributor Author

isidorn commented May 29, 2020

@bpasero thanks for the quick response.
Yeah, I can explore putting the stream in the drag and drop data. Let me start googling...

@isidorn
Copy link
Contributor Author

isidorn commented May 29, 2020

@bpasero did some googling and the only thing found is what you are already using for web dnd here. What I do not fully understand is why don't we take the same approach as we do for the web in the remote case? When we start dragging can't we fill in the same data as being done nativly when DnD from desktop starts? Or that is somehow magically done and we can not replicate it in the remote case?

@bpasero
Copy link
Member

bpasero commented May 30, 2020

Or that is somehow magically done and we can not replicate it in the remote case?

Yeah that is what I was suggesting, finding out if we can actually do the same thing that happens when you drag a file from the desktop. If we could add a File object into the data transfer, we could probably do this very efficiently. But I am not aware of a way to do this, since I did not research for it.

@isidorn
Copy link
Contributor Author

isidorn commented Jun 3, 2020

After some googling I have pushed a new approach for this, that we programitically add the File object to the data transfer as suggested here

Testing it out now...

@isidorn
Copy link
Contributor Author

isidorn commented Jun 3, 2020

Could not make it work.
originalEvent.dataTransfer!.items.add is a no-op when I execute it.
All other modifications of the items and files simply throw.

And the stack overflow answer which I linked seems to have worked when the files was an array and you could manipulate it on the fly.

This is how my items look like (since I can not modify them in any way)
Screenshot 2020-06-03 at 17 54 09

This is how it looks like when you upload it from the desktop
Screenshot 2020-06-03 at 17 54 56

@bpasero
Copy link
Member

bpasero commented Jul 2, 2020

@isidorn upon more thinking I had another idea: every remote exposes its resources through a simple HTTP server. We use this for example to be able to load images in markdown preview when shown on remote or for example to load themes when installed inside the remote.

If for each vscode-remote resource you call DOM.asDomUri(resource) you will end up with a URI such as:

vscode-remote-resource://127.0.0.1:50595/vscode-remote-resource?path=%2FUsers%2Fbpasero%2FDesktop%2Ftest-ts%2FCHANGELOG.md&tkn=82959925-0713-4e4b-8491-6b9a5c0a3f48

Which every VSCode window can resolve (we register a protocol handler for vscode-remote-resource).

Maybe you should simply introduce a new CodeDataTransfers.REMOTE_FILES: 'CodeRemoteFiles' that includes these resources and then on the receiving side to get at the data, resolve the contents through a HTTP fetch. I am not entirely sure, but maybe we even register a file system provider for vscode-remote-resource (Matt would probably know).

I would think there is 2 places where we can leverage this:

  • dropping a remote file into the explorer should create the file with contents and name at that location
  • dropping a remote file into the editor area could open an untitled file with associated file path and contents

@isidorn
Copy link
Contributor Author

isidorn commented Jul 2, 2020

@bpasero cool idea and thanks for the writeup.
If time permits will experiment today a bit with this, however assinging this milestone to August so this does not drop from the radar once I come back from vacatoin.

@isidorn isidorn added this to the August 2020 milestone Jul 2, 2020
@bpasero
Copy link
Member

bpasero commented Jul 3, 2020

One thing I realised now is that we do not seem to have any file system provider registered for the vscode-remote-resource. Though we have one for http and https. We seem to simply be rewriting the protocol to http here:

url: request.url.replace(/^vscode-remote-resource:/, 'http:'),

So we probably need to do something like that, or use fetch to get at the contents.

@bpasero
Copy link
Member

bpasero commented Jul 3, 2020

Ok I looked into it a bit more and I think I am stuck. Even though we handle vscode-remote-resource in electron main, it looks like fetch requests are failing:

Fetch API cannot load vscode-remote-resource://127.0.0.1:57060/vscode-remote-resource?path=%2FUsers%2Fbpasero%2FDesktop%2Ftest-ts%2FCHANGELOG.md&tkn=822c5a94-9678-43e3-8155-808278539822. URL scheme must be "http" or "https" for CORS request.

@deepak1556 @mjbvz or @alexdima maybe you could advise. The question is this:

  • we want to support loading remote contents (from a drag and drop operation) in local environments that are NOT connected to the remote
  • we do not want to put the entire contents into the clipboard because files can be very large
  • we think about leveraging asDomUri to get at the contents from that "local" window
  • we currently do not register a file system provider for vscode-remote-resource
  • we could register a FetchFileSystemProvider for this resource to get at the contents
  • however this requires fetch to work for this resource which fails currently

@deepak1556
Copy link
Collaborator

@bpasero for fetch api to work with custom protocol, supportFetchAPI: true permissions has to be explicitly registered like how we do for vscode-webview-resource https://github.com/microsoft/vscode/blob/master/src/main.js#L101

@bpasero
Copy link
Member

bpasero commented Jul 4, 2020

I think I got something to work, thanks for the help. Summary:

  • we need to registerSchemesAsPrivileged (scheme: vscode-remote-resource, supportFetchAPI: true)
  • we need to change our CSP in workbench.html to connect-src 'self' https: vscode-remote-resource:;
  • we need to register a file system provider for vscode-remote-resource via fileService.registerProvider(Schemas.vscodeRemoteResource, new FetchFileSystemProvider())
  • in our DND code instead of putting in the vscode-remote URI, we need to call asDomUri instead

This should allow to DND files over from the explorer to copy it into a local explorer.

There is another case where we could use it: when dragging editors around. Currently you cannot drop an editor to a local instance because the URI is vscode-remote and not understood by the local instance.

@isidorn
Copy link
Contributor Author

isidorn commented Sep 3, 2020

We did not have time to look more into this. Moving to next milestone so I try to make something out of Ben's findings.

@isidorn isidorn modified the milestones: August 2020, September 2020 Sep 3, 2020
@isidorn isidorn modified the milestones: September 2020, On Deck Sep 28, 2020
@joaomoreno joaomoreno changed the base branch from master to main February 15, 2021 08:51
@isidorn
Copy link
Contributor Author

isidorn commented Aug 18, 2021

Closing as no plan to merge.
fyi @JacksonKearl as inspiration if you end up looking into improving dnd

@isidorn isidorn closed this Aug 18, 2021
@github-actions github-actions bot locked and limited conversation to collaborators Oct 2, 2021
@isidorn isidorn deleted the isidorn/remoteToLocalDnD branch November 2, 2023 14:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't drag files from a remote vscode window to a local window
3 participants