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

New 1.4 VT colors bug, foreground color not applied to " " (space) cells when hyperlinked #7700

Closed
PhMajerus opened this issue Sep 22, 2020 · 6 comments · Fixed by #7738
Closed
Labels
Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) 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.
Milestone

Comments

@PhMajerus
Copy link

PhMajerus commented Sep 22, 2020

Come on, you knew this was coming, it's almost the same as #5502 😅

So we now have hyperlinks that show dotted underline under characters that are part of a link (since 1.4.2652.0)
You're probably delaying taking the attributes into account until they have a visible effect, which is sensible.
So foreground colors are not applied until a non-space character is shown.

But for hyperlinks, you're using the foreground color to draw the dotted underline, so you should now check if cell is a non-space or part of a hyperlink.

The following should only have the "Red" 3 cells with a red foreground color, but instead the red foreground is kept for two extra cells until the 'h' character is encountered, incorrectly making the dotted underline red in those cells.
echo -e '\e]8;;http://www.example.com/\aSample \e[31mRed\e[m hyperlink\e]8;;\a'

image

@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 22, 2020
@PhMajerus PhMajerus changed the title New 1.4 VT colors bug, atributes are not applied to " " (space) cells when hyperlinked New 1.4 VT colors bug, foreground color not applied to " " (space) cells when hyperlinked Sep 22, 2020
@DHowett
Copy link
Member

DHowett commented Sep 25, 2020

YUP, you caught us

@DHowett DHowett added Area-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) Product-Conpty For console issues specifically related to conpty and removed Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Sep 25, 2020
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Sep 25, 2020
@DHowett DHowett added this to the Terminal v1.5 milestone Sep 25, 2020
@DHowett
Copy link
Member

DHowett commented Sep 25, 2020

@PankajBhojwani this is an error carried forward by HasIdenticalVisualRepresentationForBlankSpace that impacts conpty and the dx renderer -- we have to treat hyperlink like underline (has vs not has) instead of just checking if they're the same. Ideally we'll do both.

@ghost ghost added the In-PR This issue has a related PR label Sep 25, 2020
@PankajBhojwani
Copy link
Contributor

Thanks for catching this! Fix is in PR

@PhMajerus
Copy link
Author

@PankajBhojwani Thanks for fixing it.

@ghost ghost closed this as completed in #7738 Sep 28, 2020
@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 Sep 28, 2020
DHowett pushed a commit that referenced this issue Oct 19, 2020
@ghost
Copy link

ghost commented Nov 11, 2020

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

Handy links:

@ghost
Copy link

ghost commented Nov 11, 2020

🎉This issue was addressed in #7738, which has now been successfully released as Windows Terminal Preview v1.5.3142.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-Rendering Text rendering, emoji, complex glyph & font-fallback issues Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2) 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.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants