SGR 7 (reverse video) makes text invisible in Windows Terminal

[j4james]

Environment

Windows build number: Version 10.0.18362.657
Windows Terminal version (if applicable): 0.11.1121.0

Steps to reproduce

1. Open a bash shell in Windows Terminal.
2. Execute printf "\e[mNORMAL \e[7;38m REVERSE \e[m\n"

Expected behavior

This is a weird test case, because conpty wouldn't usually forward SGR 7, so it's relying on a "feature" in conhost that forces it to pass-through any sequence that the dispatcher doesn't understand. In this case I'm relying on SGR 38 with a missing type parameter to trigger this pass-through. What this means is that the sequences that actually get sent down the conpty pipe look more like this:

\e[7;38mNORMAL \e[30;47m REVERSE \e[m\n

So ignoring the fact that the "pass through" feature is generally a bad idea, and this sequence is wrong to start with, I'd expect it to be interpreted as black on white (i.e. reversed), followed by white on black (colors changed to black on white, but still reversed).

Actual behavior

SGR 7 somehow seems to make the text invisible, so the first part of the line is blank, and the second half is white on black. Even stranger, it makes the first part of the shell prompt on the following line invisible too.

This may seem like an unimportant edge case, but I actually discovered this bug while experimenting with a fix for #2661 (which ended up causing more problems than it solved). Until we can get Windows Terminal to handle all the SGR sequences correctly, I don't think we'll be able to fix issues like #2661.

[j4james]

Another easy way to demonstrate this issue is by changing this line:

terminal/src/cascadia/TerminalCore/TerminalDispatchGraphics.cpp

Line 135 in d711d73

return _terminalApi.BoldText(bold);

to this:

    return _terminalApi.ReverseText(bold);

That's essentially making the bold attribute be interpreted as reversed video, and thus any bold text becomes invisible.

[j4james]

Well I finally figured this out. It's a lot more obvious what's going on if you have the acrylic effect enabled. What's happening is it's testing to see if the background color is default, and if so it makes the background transparent. When reverse video is enabled, though, the background is no longer rendered with the background color, so that test isn't valid. See here:

terminal/src/cascadia/TerminalCore/terminalrenderdata.cpp

Lines 61 to 66 in fe3f528

	// We only care about alpha for the default BG (which enables acrylic)
	// If the bg isn't the default bg color, then make it fully opaque.
	if (!attr.BackgroundIsDefault())
	{
	return 0xff000000 | bgColor;
	}

Essentially we need an extra check there to make sure the background is also made opaque when reverse video is enabled. Long term it'll get more complicated once we support DECSCNM mode on the WT side, but that still shouldn't be that big a deal.

Is it worth doing a PR for this now? It's only a one line change.

[DHowett-MSFT]

I'd absolutely appreciate a PR here. Puzzlingly, I love learning about the deficiencies in Terminal's VT implementation. We think conhost's is okay, so when we eventually do want to converge it's another datapoint in favor of why and when and how.

[DHowett-MSFT]

And thank you for looking at 2661. It's my great white whale issue. 😄