Custom default colors are problematic for legacy apps

[j4james]

Environment

Windows build number: Version 10.0.18362.719
Windows Terminal version (if applicable): commit b46d393

Steps to reproduce

1. Build a recent version of the OpenConsole solution.
2. From the tools directory run opencon, to get a cmd shell.
3. Open the Properties dialog and set the Screen Background to color index 3 (cyan).
4. And on the Terminal tab, check Use Separate Background, and set some shade of red, e.g. 100/0/0.
5. Execute cls.

Expected behavior

I'd expect the screen to be cleared with the same colors used to render the prompt. This is what it looks like in my existing Windows cmd shell.

Actual behavior

The screen is cleared with a cyan background, while the prompt is rendered with a red background.

The problem here is the active background color has ColorType::IsDefault, but the console APIs only deal in legacy attributes, which can't represent a "default" color. So when cls looks up the active background color, the legacy API returns the Screen Background index, which is cyan. And that's the color that cls then uses to fill the screen. The prompt in the meantime is just using the active background color, which is a genuine default color that maps to red.

The reason why this used to work, is the FillConsoleOutputAttribute API had a hack (since removed in PR #3100) that magically converted a legacy color that matched the legacy version of the active color into that equivalent TextColor value. However, that only applied when VT mode was enabled, so it wouldn't work for most legacy apps, and also relied on the active color just happening to match the color you wanted to fill with, which is by no means guaranteed.

So one option is we could add that hack back (or something similar), but I don't think that's the right solution. Other than the limitations mentioned already, it's only solving part of the problem. I know from working on issue #2661 that this is likely to effect us in other ways once conpty is passing through the full attributes correctly (the fact that conpty maps black to default is actually shielding us from these issues).

I have an idea for how we might address this, but I wanted to think it through some more before writing up the details. In the meantime, though, I thought it best to get this issue filed so you're at least aware of the problem.

[DHowett]

Man, I hate this issue. We wrote about it in the blog post that announced the "experimental terminal features" tab (which was intended to let users make conhost more like a terminal emulator), and... actually surprisingly, nobody complained about it. I wonder if that's because nobody used it?

This is ugly, and we did have those heuristics to determine whether the application was simply setting what it thought were the defaults.

Incidentally, the treatment of "matches default colors" as "wants default" is the root cause of #293.

[DHowett]

I did, at one point, try to make life horrible for applications.

d342182

(hax branch - no warranty - no disclaimers)

There was one unused legacy attribute flag on the API surface. I got to wondering, "What if?"

[zadjii-msft]

On a scale of 1-horrible, how bad was that?

I was considering something similar where we use that bit as "the fg and/or bg is default colored, not indexed", and then if you queried the current attributes and the FG (or BG) byte matched the FG (or BG) byte of _wFillAttribute, then the app could infer "oh, it's not index 0x7, it's default colored.". Then, at least apps could roundtrip the default attributes. They'd ask "what are the current attrs" and we'd give them (HAX | wFillAttrs) as "use the default attrs", and then setting the attributes to HAX | wFillAtrs _wouldn't change anything.

Though, if an app just wants to set 0x07 (not HAX | 0x07), then we'd still clear the default-ness of the current attributes. It's impossible to know who's roundtripping attributes vs just blindly setting them like this. So it's not a magic bullet 😕

[DHowett]

It was a little horrible. PowerShell doesn't exactly round-trip them, for example: it recomposes them out of what it thinks they meant. :|

[j4james]

My idea was somewhat simpler than that. I was just going to suggest that a legacy background index of 0 maps to the default background, and a legacy foreground index of 7 maps to the default foreground (where 0 and 7 are whatever the legacy Screen Background and Screen Text values are set to).

Legacy apps would still just see their usual range of 16 colors, but when they use the "default" colors (typically white on black), those colors would be stored as ColorType::IsDefault and would respond to default palette updates.

I think this is actual crucial for PowerShell, which seems to mix the VT default background and legacy black indiscriminately. If we don't do something to address that, it's going to look a mess once #2661 is fixed. This is how it currently appears in my 2661 branch:

It's possible that's a result of something I've done wrong, but I'm fairly certain it's just this black/default mismatch that's the problem.

[j4james]

OK, so I knocked together a POC of the solution I was proposing, and it's definitely an improvement, but there are still a few weird color artifacts in PowerShell. Here's what it looks like in my current branch (which includes the conpty narrowing fix).

1. Note how the nothing parameter, and most of the Write-Host line have the background color in black rather than the default color. That's because PS seems to query the default colors using legacy APIs (getting back 0x07), but then uses VT sequences to select those colors. Using a legacy API to set the color would have automatically mapped it back to the default background, but SGR 40 gets you a real black.

2. Also note the Hello produced by Write-Host has the background set to blue rather than black. In this case it is actually using a legacy API to set the color, and with my fix in place, the 0 color gets mapped back to the default background. This is also the reason why the error message background is blue, where the legacy PowerShell would have had it in black (although that seems an improvement to me).

On the plus side, though, there are definitely things that are improved by this patch. You can see the difference it has made by comparing it to a PowerShell session in conhost, with the Use Separate Background option set to a shade of blue, but with the Screen Background and Screen Text still set to the default black and white (which is more or less what you're getting in a conpty session).

The Hello has the right background color, as does the error message (assuming that's what was intended), but the nothing parameter is still wrong, and the prompt suddenly switches to the wrong color after the error message is output.

The reason for the latter issue is because most PowerShell commands seem to use legacy APIs to set their colors. So restoring the default attributes typically means setting the colors to white on black. And without my fix in place, that won't translate back to the real default colors.

Bottom line: I think what I'm proposing is a reasonable solution. The results in PowerShell aren't perfect, but that is really PowerShell's problem. A pure legacy app would probably fare much better than their current legacy-vt hybrid approach. That said, maybe these are issues that have already been fixed - I haven't tried testing with the latest PS version.

[DHowett]

I'm comfortable with us piloting the workaround that 0/7 are "default" when they come through the legacy API, yeah. It seems like we could safely(?) extend that to "active buffer defaults = default", but I'm not sure the risk there.

Unfortunately, PS6 and PS7 are not significantly better in this regard and we may need to gate #6506 on a fix for this. :/

[j4james]

I'm comfortable with us piloting the workaround that 0/7 are "default" when they come through the legacy API, yeah. It seems like we could safely(?) extend that to "active buffer defaults = default", but I'm not sure the risk there.

I'd love to be able to ignore the active buffer default (assuming we're talking about _wFillAttribute), because that's a pain to access in certain parts of the code. But that will be a breaking change in conhost for anyone that sets the Screen Text and Screen Background properties the way Powershell used to do (I'm not sure if that's still the case).

Unfortunately, PS6 and PS7 are not significantly better in this regard and we may need to gate #6506 on a fix for this. :/

I figured that might be necessary.

[DHowett]

That old powershell shortcut is, unfortunately, checked into the Windows source tree and isn't going anywhere. ☹️

Now, if people enable the weird woo-woo experimental settings on that LNK file and stuff goes wrong, I legit don't care. Terminal and other PTY hosts are unimpacted.

I'm more concerned about applications that set the default attributes programmatically and then try to print in those attributes. This could be an as-yet-unexploded minefield

[j4james]

I'm more concerned about applications that set the default attributes programmatically and then try to print in those attributes. This could be an as-yet-unexploded minefield

I don't think that's possible, at least as far I could tell. Those attributes are initialized at startup from the CONSOLE_API_CONNECTINFO, the link settings, and the registry. And at runtime they can be manually changed via the Properties menu. But I couldn't see any console API that would enable an app to set them programatically. I might have missed something though.

[DHowett]

So, hmm. I was going to remark on the split between SCREEN_INFORMATION::SetAttributes and SCREEN_INFORMATION::SetDefaultAttributes, but it looks like SetDefaultAttributes just calls SetAttributes and then triggers a redraw. It does look like the API SetConsoleScreenBufferInfoEx lets you change the "default attributes", but as far as I can tell those are actually just the active attributes?

I stand corrected! There's no API that sets the "default" attributes -- we just have really well and truly mixed up what the word "default" means in SCREEN_INFORMATION. 😄

[j4james]

It does look like the API SetConsoleScreenBufferInfoEx lets you change the "default attributes", but as far as I can tell those are actually just the active attributes?

That's correct. The naming is extremely confusing. I've had to make copious notes tracking what these different attributes mean and where they're used. That's the only way I can make sense of the code.

[j4james]

I had what I thought was an inspirational idea. What if we allow profiles to override the default legacy attributes, so PowerShell could set its default legacy background index to 1 (i.e. blue). That's essentially what make the original PowerShell work the way it does. That turned out to be more complicated than I thought, but I eventually got it working in my POC, and this was the result:

On the plus side, Write-Host with a black background actually displays as black now. On the down side, other parts of the command line that used to render with a black background are now a garish blue. The black wasn't correct either, but the blue seems somehow worse.

However, if you tweak the Campbell PowerShell color scheme so the dark blue color is the same shade as the background color, you get what I think is the perfect rendering:

But that's a whole lot of special case tweaks that all need to be in alignment for this to look right. Anyone trying to use a different color scheme is going to find things looking a mess again. Only now you're going to get bug reports about DarkBlue being rendered as black (or whatever the default background is).

In short, I'd say this isn't worth the effort, but I thought I should at least present my results here in case anything thinks this approach is worth pursuing.

[j4james]

One other thing I should add regarding the above test. I was a little concerned about the fact that the dir error was displayed with the default background color, since I was actually expecting that to be black. But it turns that is just a difference between powershell.exe and pwsh.exe - the former displays the error with a black background while the latter uses the default background color (technically the "startup" color).

Not that it makes much of a difference - it's just good to know that PR #6506 is actually doing the right thing here, and it is possible to get it exactly matching the conhost behaviour, assuming you're willing to jump through enough hoops.

[ghost]

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

Handy links:

• Release Notes
• Store Download