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

Parsing bug in 24bit VT colors #1681

Closed
PhMajerus opened this issue Feb 9, 2017 · 9 comments
Closed

Parsing bug in 24bit VT colors #1681

PhMajerus opened this issue Feb 9, 2017 · 9 comments
Assignees
Labels

Comments

@PhMajerus
Copy link

VT escape sequences can define both background and foreground, or reset them to default colors.
When using 39 or 49 to reset the foreground or background respectively, these two commands reset both at the same time instead.

Setting foreground then resetting background shouldn't reset the foreground as well, and setting background then resetting foreground shouldn't reset the background.
In RS2, setting one using RGB, then reset the other one ends up resetting both. This worked fine (although limited to 16colors mapping) in RS1.
This has been introduced with the support for real 24bit colors (without mapping) and is still present in 15031.

Repro steps:
The following command should display "TestTest" in light blue :
echo -e "\E[38;2;64;128;255mTest\E[49mTest\E[m"
Instead, the second "Test" is shown in default color, but 49 shouldn't reset the color set by 38;2;R;G;B, it should only reset the background color.

Same problem happens to background color if foreground color is reset after :
echo -e "\E[48;2;64;128;255mTest\E[39mTest\E[m"
Again, 39 shouldn't reset the background set by 48:2;R;G;B, it should only reset the foreground color.

This issue breaks any software that controls both foreground and background and uses 39 and 49 to reset either color. It doesn't only break bash in WSL, it also breaks the same VT in PowerShell or any console app as the problem is in the conhost VT parser.

@PhMajerus
Copy link
Author

I found out other control sequences, such as underline or reverse, also reset the RGB colors.
It seems the parser sets all console attributes as a block, based on the previous state, but doesn't keep track of 24-bit colors between calls.

@zadjii-msft
Copy link
Member

@PhMajerus thanks for the very well detailed bug report! I think you're right that this is the wrong behavior. I don't think that this fix will make it in time for Creators Update, but I've added MSFT:11027962 to track it for the next release.

@PhMajerus
Copy link
Author

Thanks for looking into this, it didn't get fixed for RS2, but I really hope it can get fixed in a CU before RS3.
I also found a new ANSI/VT display bug that's probably related, but this time has to do with how the console displays chunks of colored text: #1907

@gpakosz
Copy link

gpakosz commented Apr 16, 2017

@zadjii-msft I have two questions if you don't mind. When you say

I've added MSFT:11027962 to track it for the next release.

  • is MSFT:11027962 something internal?
  • does "next release" mean console will get updated through Windows update? Or is it another ~6 months cycle?

Thanks

@zadjii-msft
Copy link
Member

@gpakosz Yes, when I prefix arbitrary numbers with "MSFT:" those are our internal work tracking numbers.

By "next release" I mean whatever comes after Creator's Update. We VERY rarely service conhost.exe, if ever. And just because it's being tracked for the next release doesn't necessarily mean it'll make the cut - we currently have a ton of work that's booked, so this will have to get triaged just like everything else, though it seems like a simple fix so it shouldn't be too hard to get in.

@zadjii-msft
Copy link
Member

zadjii-msft commented Jun 7, 2017

@PhMajerus Got this one pinned down. It's in code review as we speak :)

@zadjii-msft
Copy link
Member

Should be out to Insiders sometime in the next few months.

@PhMajerus
Copy link
Author

@zadjii-msft That's great news, thanks. I'll test all the VT things again from Win32 CUI when it arrives in my ring.

@PhMajerus
Copy link
Author

Thanks @zadjii-msft
Closing this issue as \x1B[39m and \x1B[49m are fixed.
Related \x1B[7m \x1B[27m issue documented in new bug : #2583

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants