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

Drop files from Explorer to CustomEditor (webview) doesn't work #182449

Closed
maxbublik opened this issue May 14, 2023 · 12 comments · Fixed by #209211
Closed

Drop files from Explorer to CustomEditor (webview) doesn't work #182449

maxbublik opened this issue May 14, 2023 · 12 comments · Fixed by #209211
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug help wanted Issues identified as good community contribution opportunities insiders-released Patch has been released in VS Code Insiders verified Verification succeeded webview Webview issues
Milestone

Comments

@maxbublik
Copy link

Type: Bug

Hi guys, thanks a lot for your amazing work! Unfortunately after searching existing issues and documentation, I still experience an issue. I developed a tiny extension to showcase the bug https://github.com/maxbublik/vscode-customeditor-drop

The extension offers a CustomEditor for *.drops.json files. When such a file is opened via the given custom editor, you can drag-n-drop (presumably anything) onto the webview and the drop event information is appended to the underlying json file.

Drop a file from Explorer tree view: ❌ does not work
Drop an item with dataTransfer.setData('text/uri-list', value); manually set in another webview: ✅ OK
Drop a file from OS: ✅ OK

dnd-customeditor-explorer.mov

VS Code version: Code - Insiders 1.79.0-insider (2575777, 2023-05-12T05:24:44.544Z)
OS version: Darwin arm64 21.4.0
Modes:
Sandboxed: Yes

System Info
Item Value
CPUs Apple M1 (8 x 24)
GPU Status 2d_canvas: enabled
canvas_oop_rasterization: disabled_off
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
video_decode: enabled
video_encode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
webgpu: enabled
Load (avg) 2, 2, 2
Memory (System) 16.00GB (1.09GB free)
Process Argv --crash-reporter-id 47e2bfbb-a348-459d-8975-8945e0235016
Screen Reader no
VM 0%
Extensions: none
A/B Experiments
vsliv695:30137379
vsins829:30139715
vsliv368:30146709
vsreu685:30147344
python383cf:30185419
vspor879:30202332
vspor708:30202333
vspor363:30204092
vslsvsres303:30308271
pythontb:30258533
pythonptprofiler:30281269
vshan820:30294714
pythondataviewer:30285072
vscod805:30301674
bridge0708:30335490
bridge0723:30353136
cmake_vspar411:30581797
vsaa593cf:30376535
pythonvs932:30404738
cppdebug:30492333
vsclangdf:30492506
c4g48928:30535728
dsvsc012:30540252
pynewext54:30618038
pylantcb52:30590116
pyind779:30611226
pythonsymbol12:30651887
showlangstatbar:30737417
azdwalk:30721579
pythonms35:30671666
f08j5886:30680471
24365598:30687740
pythonfmttext:30716741
pythoncmvfstr:30726892
fixshowwlkth:30724385
hidesbindicator:30724476
pythongtdpath:30726887

@mjbvz mjbvz added this to the May 2023 milestone May 15, 2023
@mjbvz mjbvz added the bug Issue identified by VS Code Team member as probable bug label May 17, 2023
@mjbvz mjbvz modified the milestones: May 2023, Backlog May 17, 2023
@mjbvz
Copy link
Collaborator

mjbvz commented May 17, 2023

Thank you for the great repo steps!

The root cause is:

This code surpasses pointer events on the webview element when any element in VS Code editor dom is being dragged. This is what lets you drag and drop over a webview (or any text editor) to open the dropped file

Here's the behavior we want:

  • If the user drags a file over a webview without holding shift, they see the editor drop overlay. Dropping the file opens in that editor group
  • Once the user presses shift while dragging, they instead start dropping the content into the webview. The drop overlay should now go away
  • Once the user releases shift, the drop overlay shows again

Coordinating the drag events across the different iframes involved here gets a little tricky. We may need to capture the events inside the webview iframes and re-broadcast them up to VS Code, similar to what we already do for keypresses

@mjbvz mjbvz added help wanted Issues identified as good community contribution opportunities webview Webview issues labels May 17, 2023
@ManPython

This comment was marked as off-topic.

@maxbublik
Copy link
Author

Hi, Matt, thanks a lot for your message!

What I do observe: when the user drags a file over a webview with holding shift and the drop overlay is absent, the webview still does not receive neither dragover nor drop events.
This is a bug, right? Or did I miss some configuration?

I assume the whole drag-n-drop machinery (getting tricky recently) is evolving around a core VSCode pattern: dropping a file onto an editor group should open that file in that editor group. What if webview providers could explicitly announce dropTypes (the same way as native VSCode trees). Therefore, certain webviews could skip the pattern and handle dragover and drop events on their own if the original drag event contains one of the announces dataTransfer.types. The video demonstrates this at timecode 0:12-0:17. Does it break the UX significantly?

@mjbvz
Copy link
Collaborator

mjbvz commented May 18, 2023

What if webview providers could explicitly announce dropTypes

I don't think that helps. If the users is dragging a file or url-list, we want the behavior I described above: first we show the drop overlay and then when the user presses shift, we allow dropping into the webview

My current understanding is that requires coordinating the drag events across different iframes and that's what gets tricky

@maxbublik
Copy link
Author

Hi Matt, @mjbvz, do you have a rough estimation of the fix to arrive?

@drvkize0
Copy link

Wonder if this will work? I do notice when holding shift I can't drag from file explorer.

windowDidDragStart(): void {
    if( !shiftIsDown)
		this._startBlockingIframeDragEvents();
}

@swordensen
Copy link
Contributor

swordensen commented Mar 31, 2024

I basically added a new listener on the WebviewWindowDragMonitor that should re-enable the customWebView pointer events allowing us to drag and drop files as expected.

Once the user releases shift, the drop overlay shows again

I recognized that if a user presses shift while dragging and dropping a file on to any editor then the full drag and drop is cancelled. Are we sure we want to show the drop overlay again if the user stops pressing shift?

[Edit] I can make another PR if we want to disable the WebView if the user releases shift

[Edit 2] This is my first PR to VSCode so I do apologize if I did not submit it appropriately. I also recognize that Matt is incredibly busy so I won't @ him. But, to whomever ends up reviewing this, please @ me if there are any issues and I'll try to address them ASAP. This issue is a blocker for an extension I would like to make

@mjbvz mjbvz closed this as completed in 51917e8 Jun 19, 2024
@vscodenpa vscodenpa added unreleased Patch has not yet been released in VS Code Insiders insiders-released Patch has been released in VS Code Insiders and removed unreleased Patch has not yet been released in VS Code Insiders labels Jun 19, 2024
@rzhao271 rzhao271 modified the milestones: Backlog, June 2024 Jun 25, 2024
bricefriha pushed a commit to bricefriha/vscode that referenced this issue Jun 26, 2024
…g and Drop Events (microsoft#209211)

* added shift key listener to re-enable drag events for webview microsoft#182449

* adding drag and drag over listeners to webview iframe(s)

updating html hash

adding shift release listener to drag and drag over events

adding new custom drag event handler to allow event bubbling

adding new event types

correcting comment:

removing drag over listener from drag monitor since we are converting this to drag events

making webview drag monitor stateful and adding missing events to iframe dom html file

Revert "Merge remote-tracking branch 'microsoft/main' into swordensen"

This reverts commit 47e3877, reversing
changes made to 8f71927.

Revert "making webview drag monitor stateful and adding missing events to iframe dom html file"

This reverts commit 8f71927.

Revert "removing drag over listener from drag monitor since we are converting this to drag events"

This reverts commit bfae5f1.

Revert "correcting comment:"

This reverts commit 696facb.

Revert "Merge branch 'main' into main"

This reverts commit 78ed36c, reversing
changes made to 7b96d0e.

Revert "adding new event types"

This reverts commit 7b96d0e.

Revert "adding new custom drag event handler to allow event bubbling"

This reverts commit 39971e0.

Revert "adding shift release listener to drag and drag over events"

This reverts commit 4f5d9e8.

Revert "updating html hash"

This reverts commit 6a474cb.

Revert "adding drag and drag over listeners to webview iframe(s)"

This reverts commit 6688ec6.

adding harmless change to see if this causes smoketests and integration tests to fail

adding sha hash

Revert "adding sha hash"

This reverts commit 830baa8.

Revert "adding harmless change to see if this causes smoketests and integration tests to fail"

This reverts commit ceb55f9.

Revert "Revert "adding drag and drag over listeners to webview iframe(s)""

This reverts commit 1baf483.

Revert "Revert "adding shift release listener to drag and drag over events""

This reverts commit 40bc29c.

Revert "Revert "adding new custom drag event handler to allow event bubbling""

This reverts commit 44f4fda.

Revert "Revert "adding new event types""

This reverts commit 67f1aaa.

removing internal drag listener

removing types. Maybe it's causing build issues?

renaming drag event to WebviewDragEvent to avoid collision with native DragEvent type

resolving conflict with correcting comment

Revert "Revert "Merge branch 'main' into main""

This reverts commit 1a4d711.

Revert "Revert "Merge branch 'main' into main""

This reverts commit 8520787.

resolving conflict

Revert "resolving conflict"

This reverts commit c5b3f7f.

Revert "Merge remote-tracking branch 'microsoft/main' into swordensen"

somehow i got assigned these changes in the process of unreverting my merges

fix: serialization of newline characters

removing console log

forgot the security hash when removing console log

* updating sha
@rzhao271 rzhao271 added the verified Verification succeeded label Jun 26, 2024
@rzhao271
Copy link
Contributor

rzhao271 commented Jun 26, 2024

@mjbvz on Windows, dropping a file from the explorer or from the drops view into the editor ends up opening the file or link. With the extension sample above, I dropped in a spreadsheet from the explorer, and Excel opened. Is that expected behaviour?

Edit: verified that that behaviour also occurs in Stable.

@swordensen
Copy link
Contributor

@mjbvz on Windows, dropping a file from the explorer or from the drops view into the editor ends up opening the file or link. With the extension sample above, I dropped in a spreadsheet from the explorer, and Excel opened. Is that expected behaviour?

Edit: verified that that behaviour also occurs in Stable.

This is likely due to the fact that default drop events are not prevented.

I can look into resolving this.

@ManPython
Copy link

The title is wrong described. The move any file as D&D to VSC not opening a editor with new tab.
And it's not related with security issue or maybe it is.. as "workspace" and "namespace" around dir, where outside dir of project files can't be opened, but need ctr+shift+N as new instance VSC in case intuition want suggest it should be easy opened to change something or run test script.

@swordensen
Copy link
Contributor

@rzhao271 Because this issue is closed I created a new issue to track the bug you reported here: #218626. I think I know how to fix it so you can assign it to me if you want

@ManPython I honestly don't know what you are trying to say. Can you clarify?

@swordensen
Copy link
Contributor

@mjbvz can this issue be re-opened since my PR was reverted? I'll open another PR "soonTM" I just figured it should point at this issue.

@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Aug 3, 2024
mjbvz added a commit to mjbvz/vscode that referenced this issue Aug 7, 2024
Yoyokrazy pushed a commit that referenced this issue Aug 7, 2024
zongou pushed a commit to zongou/vscode that referenced this issue Aug 13, 2024
mustard-mh pushed a commit to gitpod-io/openvscode-server that referenced this issue Sep 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug help wanted Issues identified as good community contribution opportunities insiders-released Patch has been released in VS Code Insiders verified Verification succeeded webview Webview issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants