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

It seems like DA1 is broken in 1.22 (the response doesn't include a valid CSI (\x1b)) #17813

Closed
zadjii-msft opened this issue Aug 28, 2024 · 1 comment · Fixed by #17833
Closed

It seems like DA1 is broken in 1.22 (the response doesn't include a valid CSI (\x1b)) #17813

zadjii-msft opened this issue Aug 28, 2024 · 1 comment · Fixed by #17833
Labels
Area-Input Related to input processing (key presses, mouse, etc.) In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Conpty For console issues specifically related to conpty
Milestone
Terminal v1.23

Comments

@zadjii-msft
Copy link
Member

It seems like DA1 is broken in this release. When I query Sixel support by sending \x1b[c, the response doesn't include a valid CSI (\x1b) in the beginning:

[?61;4;6;7;14;21;22;23;24;28;32;42c

and it should be:

\x1b[?61;4;6;7;14;21;22;23;24;28;32;42c

I'm using Microsoft.WindowsTerminalPreview_1.22.2362.0_arm64.zip

Originally posted by @sxyazi in #17809 (comment)
@microsoft-github-policy-service microsoft-github-policy-service bot 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 Aug 28, 2024
@DHowett
Copy link
Member

DHowett commented Aug 28, 2024

This is probably “the documentation said I can read VT replies without enabling VT INPUT mode, but the new ConPTY breaks that” as reported by @alabuzhev

We drop the ESC during processed input handling. See discussion in #17729.

@zadjii-msft zadjii-msft mentioned this issue Aug 28, 2024
Closed
@zadjii-msft zadjii-msft added Product-Conpty For console issues specifically related to conpty Area-Input Related to input processing (key presses, mouse, etc.) Issue-Bug It either shouldn't be doing this or needs an investigation. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Aug 28, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot removed the Needs-Tag-Fix Doesn't match tag requirements label Aug 28, 2024
@github-project-automation github-project-automation bot moved this to To Cherry Pick in 1.22 Servicing Pipeline Aug 28, 2024
@lhecker lhecker mentioned this issue Aug 29, 2024
Open
@j4james j4james mentioned this issue Aug 29, 2024
Closed
@lhecker lhecker mentioned this issue Aug 30, 2024
Merged
@microsoft-github-policy-service microsoft-github-policy-service bot added the In-PR This issue has a related PR label Aug 30, 2024
@j4james j4james mentioned this issue Sep 3, 2024
Closed
@lhecker lhecker closed this as completed in 7b50f12 Sep 4, 2024
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Sep 4, 2024
@DHowett DHowett moved this from To Cherry Pick to Cherry Picked in 1.22 Servicing Pipeline Sep 4, 2024
DHowett pushed a commit that referenced this issue Sep 4, 2024
@lhecker @DHowett
Avoid dropping Esc characters from VT responses (#17833)
be99f93 
`GetChar` checks if the vkey is VK_ESCAPE. `CharToKeyEvents` however
tries really hard to figure out the vkeys of all characters.
To avoid these issues all we need to do is to simply use the existing
`WriteString` function we already use for all other VT responses.
If it's good for conhost responses, it's good for ConPTY responses.

Additionally, this removes another `IsVtInputEnabled` which was
redundant with `WriteString` which worked exactly the same internally.

Closes #17813
Closes #17851
Probably also related to #17823

* Wrote a small app to send and receive a DA1 request. It works ✅
* WSL already worked to begin with (and still works now) ✅
* No double-encoding of mouse input events ✅

(cherry picked from commit 7b50f12)
Service-Card-Id: PVTI_lADOAF3p4s4AmhmQzgSa4RU PVTI_lADOAF3p4s4AmhmQzgStRA0
Service-Version: 1.22
@DHowett DHowett moved this from Cherry Picked to Shipped in 1.22 Servicing Pipeline Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Area-Input Related to input processing (key presses, mouse, etc.) In-PR This issue has a related PR Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Product-Conpty For console issues specifically related to conpty
Projects
1.22 Servicing Pipeline
Status: Shipped
Milestone
Terminal v1.23
Development

Successfully merging a pull request may close this issue.

Avoid dropping Esc characters from VT responses microsoft/terminal
3 participants
@DHowett @zadjii-msft and others