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.

Sign up for GitHub

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

Jump to bottom

Improve clipboard handling in "drag and drop" scenario #8461

Merged
2 commits merged into from
Dec 2, 2020
Merged

Improve clipboard handling in "drag and drop" scenario #8461

2 commits merged into from
Dec 2, 2020

Conversation

skyline75489
Copy link
Collaborator

@skyline75489 skyline75489 commented Dec 2, 2020 •
edited by DHowett
Loading

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 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 Dec 2, 2020
@skyline75489
Copy link
Collaborator Author

skyline75489 commented Dec 2, 2020 •
edited
Loading

During the development of this PR, I found that simply putting Text before StorageItem would fix the crash described in #7804, although the content being pasted isn't really useful. This is what I got which seems like a internal URI used by VSCode:

walkThrough:/c%3A/Program%20Files/Microsoft%20VS%20Code/resources/app/out/vs/workbench/contrib/welcome/page/browser/vs_code_welcome_page?%7B%22moduleId%22%3A%22vs%2Fworkbench%2Fcontrib%2Fwelcome%2Fpage%2Fbrowser%2Fvs_code_welcome_page%22%7D

But just in case there is any other possible exceptions, I added the catch anyway.

For WebLink, you can drag a hyperlink in Firefox and drop it in WT. It does not work without this PR. It should work now.

For ApplicationLink I didn't find an application that actually produces this kind of content, but I assume it should be the same as WebLink, so I added it alongside WebLink.

carlos-zamora
carlos-zamora approved these changes Dec 2, 2020
View reviewed changes
Copy link
Member

@carlos-zamora carlos-zamora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@DHowett DHowett added the AutoMerge Marked for automatic merge by the bot when requirements are met label Dec 2, 2020
@ghost
Copy link

ghost commented Dec 2, 2020

Hello @DHowett!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@ghost ghost merged commit 60f1b0b into microsoft:main Dec 2, 2020
@skyline75489 skyline75489 deleted the chesterliu/dev/drag-drop branch December 2, 2020 23:31
@skyline75489
Copy link
Collaborator Author

Yes! The comment is exactly what I want to add but failed to compose. You have read my mind 😄

@DHowett
Copy link
Member

DHowett commented Dec 3, 2020

😎

DHowett pushed a commit that referenced this pull request Jan 25, 2021
@skyline75489 @DHowett
Improve clipboard handling in "drag and drop" scenario (#8461)
305946c 
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

🎉Windows Terminal v1.5.10271.0 has been released which incorporates this pull request.:tada:

Handy links:

@ghost ghost mentioned this pull request Jan 28, 2021
Closed
@ghost
Copy link

ghost commented Jan 28, 2021

🎉Windows Terminal Preview v1.6.10272.0 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@carlos-zamora carlos-zamora carlos-zamora approved these changes

@DHowett DHowett DHowett approved these changes

Assignees
No one assigned
Labels
Area-UserInterface Issues pertaining to the user interface of the Console or Terminal AutoMerge Marked for automatic merge by the bot when requirements are met 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.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dragging a tab from Visual Studio Code onto Windows Terminal causes it to crash.
3 participants
@skyline75489 @DHowett @carlos-zamora