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

Dragging a tab from Visual Studio Code onto Windows Terminal causes it to crash. #7804

Closed
bhelyer opened this issue Oct 2, 2020 · 5 comments · Fixed by #8461
Closed

Dragging a tab from Visual Studio Code onto Windows Terminal causes it to crash. #7804

bhelyer opened this issue Oct 2, 2020 · 5 comments · Fixed by #8461
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Crash Crashes are real bad news.
Milestone

Comments

@bhelyer
Copy link

bhelyer commented Oct 2, 2020

Environment

Windows build number: Microsoft Windows NT 10.0.19041.0
Windows Terminal version (if applicable): 1.3.2651.0
VS Code: 1.49.2

Steps to reproduce

  1. Open Windows terminal (Powershell or Cmd, doesn't seem to matter)
  2. Open Visual Studio Code
  3. Drag the 'Welcome' tab (or any other) onto Windows Terminal

(Note that this occured because I missed when using VS Code, not because I thought it would do something meaningful.)

Expected behavior

Nothing, or an error to be generated. I don't rightly know what VS Code sends out when you drag a tab, I'm just expecting Windows Terminal not to crash.

Actual behavior

Windows Terminal closes, taking all my tabs with it as punishment for my failure to accurately use a mouse.

@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Oct 2, 2020
@DHowett
Copy link
Member

DHowett commented Oct 2, 2020

Whoa, that's wild. That's totally unexpected.

@DHowett DHowett added Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Severity-Crash Crashes are real bad news. labels Oct 2, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Oct 2, 2020
@DHowett DHowett added this to the Terminal v1.5 milestone Oct 2, 2020
@DHowett
Copy link
Member

DHowett commented Oct 2, 2020

Looks like VSCode generates a malformed clipboard/drop object; we should be more resilient.

The WinRT DataItems API is failing when we ask "give us the files you said were dropped on me" here (2852):

if (e.DataView().Contains(StandardDataFormats::StorageItems()))
{
auto items = co_await e.DataView().GetStorageItemsAsync();
if (items.Size() > 0)

@DHowett DHowett removed the Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting label Oct 2, 2020
@skyline75489
Copy link
Collaborator

I can easily reproduce this. And it is easy to fix the crash by simply catching it. But I kinda want it to work and haven't figured out a way to do so.

For reference, both Edge and Firefox handles the tab dropping from VSCode as a file URI (file://something). Surprisely explorer.exe handles it as an actual file and will paste it to the current directory.

A side note: Should we also support ApplicationLInk and WebLink? Then you can drag and drop an URL from browser and it will be pasted into the terminal. I've tested it and it works pretty well.

@ghost ghost added the In-PR This issue has a related PR label Dec 2, 2020
@ghost ghost closed this as completed in #8461 Dec 2, 2020
ghost pushed a commit that referenced this issue Dec 2, 2020
This PR improves the clipboard handling logic of "drag and drop" in
TermControl, making it more useful and less likely to crash.

* Added support for two more categories of content, `ApplicationLink`
  and `WebLink`.
* Reordered the ifs, making `StorageItem` the last clause. With WT being
  a text-oriented application, I think we can safely assume that the
  content being pasted is likely to be text/links.
* Catch possible exceptions during
  `e.DataView().GetStorageItemsAsync()`.

Closes #7804
@ghost ghost added Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. and removed In-PR This issue has a related PR labels Dec 2, 2020
DHowett pushed a commit that referenced this issue Jan 25, 2021
This PR improves the clipboard handling logic of "drag and drop" in
TermControl, making it more useful and less likely to crash.

* Added support for two more categories of content, `ApplicationLink`
  and `WebLink`.
* Reordered the ifs, making `StorageItem` the last clause. With WT being
  a text-oriented application, I think we can safely assume that the
  content being pasted is likely to be text/links.
* Catch possible exceptions during
  `e.DataView().GetStorageItemsAsync()`.

Closes #7804

(cherry picked from commit 60f1b0b)
@ghost
Copy link

ghost commented Jan 28, 2021

🎉This issue was addressed in #8461, which has now been successfully released as Windows Terminal v1.5.10271.0.:tada:

Handy links:

@ghost
Copy link

ghost commented Jan 28, 2021

🎉This issue was addressed in #8461, which has now been successfully released as Windows Terminal Preview v1.6.10272.0.:tada:

Handy links:

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Terminal The new Windows Terminal. Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Crash Crashes are real bad news.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants