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

An empty OSC8 followed by an SGR 0 fails to terminate hyperlink over ConPTY at the bottom of the screen (!) #7597

Closed
DHowett opened this issue Sep 10, 2020 · 5 comments · Fixed by #7608
Closed

An empty OSC8 followed by an SGR 0 fails to terminate hyperlink over ConPTY at the bottom of the screen (!) #7597

DHowett opened this issue Sep 10, 2020 · 5 comments · Fixed by #7608
Assignees
@PankajBhojwani
Labels
Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-1 A description (P1) Product-Conpty For console issues specifically related to conpty Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Blocking We won't ship a release like this! No-siree.
Milestone
Terminal v1.4

Comments

@DHowett
Copy link
Member

DHowett commented Sep 10, 2020

Behavior:

image

There might be something in conpty that should be calling the new SetDefaultMeta... method.
@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 Sep 10, 2020
@DHowett
Copy link
Member Author

DHowett commented Sep 10, 2020

/cc @PankajBhojwani

@DHowett DHowett added Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-1 A description (P1) Product-Conpty For console issues specifically related to conpty Severity-Blocking We won't ship a release like this! No-siree. and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Sep 10, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 10, 2020
@DHowett DHowett added this to the Terminal v1.4 milestone Sep 10, 2020
@DHowett
Copy link
Member Author

DHowett commented Sep 10, 2020 •
edited
Loading 

printf '\e[01;32m\e]8;;file://dhowett-sl/mnt/c/Users/duhowett/_viminfo\a_viminfo\e]8;;\a\e[0m\n'

@PankajBhojwani
Copy link
Contributor

PankajBhojwani commented Sep 11, 2020 •
edited
Loading

This is interesting - the SGR0 is not necessary to repro this, it happens because of both the '\e[01;32m' at the beginning and the '\n' at the end:

printf '\e[01;32m\e]8;;http://example.com\aLink\e]8;;\a\n'

will repro this whereas

printf '\e]8;;http://example.com\aLink\e]8;;\a\n' (remove '\e[01;32m') or
printf '\e[01;32m\e]8;;http://example.com\aLink\e]8;;\a' (remove '\n')

will not

Update: don't even need the '01', just

printf '\e[32m\e]8;;http://example.com\aLink\e]8;;\a\n' will repro

@PankajBhojwani PankajBhojwani mentioned this issue Sep 11, 2020
Merged
2 tasks
@ghost ghost added the In-PR This issue has a related PR label Sep 11, 2020
@PankajBhojwani
Copy link
Contributor

Found the issue (in PR now) - though I suspect similar things will pop up eventually because of old code where SGR reset was no different from resetting only the default meta attributes

@ghost ghost added Needs-Tag-Fix Doesn't match tag requirements 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 Sep 11, 2020
DHowett pushed a commit that referenced this issue Sep 11, 2020
@PankajBhojwani
Fix OSC8 termination over the PTY after SGR 0 (#7608)
88d1527 
We were prematurely clearing the hyperlink ID by resetting the
_lastTextAttributes. We should only clear the fields we want
cleared.

Fixes #7597.
@ghost
Copy link

ghost commented Sep 22, 2020

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

Handy links:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@PankajBhojwani PankajBhojwani

Labels
Area-VT Virtual Terminal sequence support Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Tag-Fix Doesn't match tag requirements Priority-1 A description (P1) Product-Conpty For console issues specifically related to conpty Resolution-Fix-Committed Fix is checked in, but it might be 3-4 weeks until a release. Severity-Blocking We won't ship a release like this! No-siree.
Projects
None yet
Milestone
Terminal v1.4
Development

Successfully merging a pull request may close this issue.

Fix OSC8 termination over the PTY after SGR 0 microsoft/terminal
2 participants
@DHowett @PankajBhojwani