starred/terminal
1
0
Fork 0
mirror of https://github.com/microsoft/terminal.git synced 2026-02-04 05:35:20 +00:00
Code Issues 19.0k Packages Projects Releases 20 Wiki Activity

Full-screen COLOR command no longer works right when resizing conhost and Terminal #16864

New Issue
Closed
opened 2026-01-31 05:25:27 +00:00 by claunia · 10 comments
claunia commented 2026-01-31 05:25:27 +00:00
Owner
Copy Link

Originally created by @zadjii-msft on GitHub (Feb 24, 2022).

Originally assigned to: @zadjii-msft on GitHub.

Open cmd window, set the color to 2f (green background, white text); observe the color change
Resize the window, the color changes back to black, with the "text" portion of the window still left in 2f.
This behavior can be repeated every time a resize happens.

This is the external half of MSFT:34761400

Yep, okay this is weird in conhost now. I bet this was caused by fixing #3848.

Since this one's a partner bug, we've gotta get this sorted out in this OS release.

Originally created by @zadjii-msft on GitHub (Feb 24, 2022). Originally assigned to: @zadjii-msft on GitHub. > Open cmd window, set the color to 2f (green background, white text); observe the color change > Resize the window, the color changes back to black, with the "text" portion of the window still left in 2f. > This behavior can be repeated every time a resize happens. This is the external half of MSFT:34761400 > Yep, okay this is weird in conhost now. I bet this was caused by fixing #3848. Since this one's a partner bug, we've gotta get this sorted out in this OS release.
claunia commented 2026-01-31 05:25:28 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Mar 2, 2022):

#5792

image

yes, yes it does

@zadjii-msft commented on GitHub (Mar 2, 2022): #5792 ![image](https://user-images.githubusercontent.com/18356694/156349282-4c9ffec4-40ed-409d-93cc-23c4100ca162.png) _yes, yes it does_
claunia commented 2026-01-31 05:25:29 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Mar 2, 2022):

for linking: #3848

Options

  • Pretty sure we can't gate this behavior on being in VT mode, b/c I think cmd.exe itself runs in VT mode (just, disables it for child processes). Check anyways?
  • We could target the fix from #5792 to only happen when exiting the alt buffer. That wouldn't fix https://github.com/microsoft/terminal/issues/3848#issuecomment-822656772 though
  • Something more clever. Is this sorta like #32? Like, maybe we need to be able to see that the empty spaces at the end of the line were filled with attributes that aren't the default attributes and reflow them as well
@zadjii-msft commented on GitHub (Mar 2, 2022): for linking: #3848 Options * [ ] Pretty sure we can't gate this behavior on being in VT mode, b/c I think cmd.exe itself runs in VT mode (just, disables it for child processes). Check anyways? * [ ] We could target the fix from #5792 to only happen when exiting the alt buffer. That wouldn't fix https://github.com/microsoft/terminal/issues/3848#issuecomment-822656772 though * [ ] Something more clever. Is this sorta like #32? Like, maybe we need to be able to see that the empty spaces at the end of the line were filled with attributes that aren't the default attributes and reflow them as well
claunia commented 2026-01-31 05:25:29 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Mar 2, 2022):

So, this is totally low key #32. It was just never a problem before, because when you'd do color 2f before, we'd start the buffer off with the attributes set to 2f, so you wouldn't notice that the colors didn't get extended to the end of the buffer. Like, if you did

wsl printf "\x1b[2Jfoo\x1b[5b\nbar\x1b[5b\nbaz\x1b[5b"

color 2f

wsl printf "\x1b[s\x1b[2;2H\x1b[41m[ FOO ]\x1b[u"

<resize window>

I bet it would have done the same thing with the red FOO line in conhost before this change. (need to find an old conhost to confirm)

That being said, people love their color commands so I guess we gotta sort this out now.

  • ❌ vb_release: image
  • ❌ rs5_release: image
  • ✅ rs4_release: image

Look, this has been busted since RS5. That's like, since October 2018. I wasn't the murderer (this time)!

@zadjii-msft commented on GitHub (Mar 2, 2022): So, this is totally low key #32. It was just never a problem before, because when you'd do `color 2f` before, we'd start the buffer off with the attributes set to `2f`, so you wouldn't notice that the colors didn't get extended to the end of the buffer. Like, if you did ``` wsl printf "\x1b[2Jfoo\x1b[5b\nbar\x1b[5b\nbaz\x1b[5b" color 2f wsl printf "\x1b[s\x1b[2;2H\x1b[41m[ FOO ]\x1b[u" <resize window> ``` I bet it would have done the same thing with the red FOO line in conhost before this change. ~(need to find an old conhost to confirm)~ ~That being said, people love their `color` commands so I guess we gotta sort this out now.~ * ❌ vb_release: ![image](https://user-images.githubusercontent.com/18356694/156378640-512e0cd1-016f-4906-89c6-e730d33ae7b4.png) * ❌ rs5_release: ![image](https://user-images.githubusercontent.com/18356694/156379515-eaa76286-51ff-4b9f-9fab-c25621ea5e0d.png) * ✅ rs4_release: ![image](https://user-images.githubusercontent.com/18356694/156380782-86dd1209-d101-430b-a63d-ca50cfc16f10.png) Look, this has been busted since RS5. That's like, since October 2018. I wasn't the murderer (this time)!
claunia commented 2026-01-31 05:25:29 +00:00
Author
Owner
Copy Link

@j4james commented on GitHub (Mar 2, 2022):

Yeah, I was going to say I thought this was probably the same thing as #32. Will be great if we can get that fixed.

@j4james commented on GitHub (Mar 2, 2022): Yeah, I was going to say I thought this was probably the same thing as #32. Will be great if we can get that fixed.
claunia commented 2026-01-31 05:25:29 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Mar 2, 2022):

Will be great if we can get that fixed.

it's my white whale 😆

@zadjii-msft commented on GitHub (Mar 2, 2022): > Will be great if we can get that fixed. it's my white whale 😆
claunia commented 2026-01-31 05:25:29 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Mar 2, 2022):

Yo @j4james since you've probably spent more time in the buffer than anyone else recently, would you mind taking a quick look at https://github.com/microsoft/terminal/compare/dev/migrie/b/32-attempt-2 ? I feel like I have to be missing something obvious. Like, this bug (#32) dates back to Windows 10, there's no way fixing it should be that easy. It's not perfect:

  • it's not very performant (we're already in reflow here so this is unlikely to be the worst of our worries)
  • It will still extend the attrs of the last column to the end of the row when reflowed, which can still result in some artifacts, but much less aggressive than before
    • we may be able to mitigate that by inserting a TextAttribute{} "bookend" to the end of the attr row, when we encounter the end of the old row.
  • If you have a bunch of colored spaces, then resize smaller, then resize bigger, they'll be gone (again, no worse than today)

I just feel like I must have missed something obvious. It couldn't have been that easy

@zadjii-msft commented on GitHub (Mar 2, 2022): Yo @j4james since you've probably spent more time in the buffer than anyone else recently, would you mind taking a quick look at https://github.com/microsoft/terminal/compare/dev/migrie/b/32-attempt-2 ? I feel like I have to be missing something obvious. Like, this bug (#32) dates back to Windows 10, there's no way fixing it should be that easy. It's not perfect: * it's not very performant (we're already in reflow here so this is unlikely to be the worst of our worries) * It will still extend the attrs of the last column to the end of the row when reflowed, which can still result in some artifacts, but much less aggressive than before - we may be able to mitigate that by inserting a `TextAttribute{}` "bookend" to the end of the attr row, when we encounter the end of the old row. * If you have a bunch of colored spaces, then resize smaller, then resize bigger, they'll be gone (again, no worse than today) I just feel like I must have missed something obvious. It couldn't have been that easy
claunia commented 2026-01-31 05:25:29 +00:00
Author
Owner
Copy Link

@j4james commented on GitHub (Mar 3, 2022):

The first thing I noticed is that it only works as far as the last non-blank line. For example, if you do printf "\e[42m\e[2J"; sleep 10, and then resizing while it's sleeping, the whole screen goes back to black. Is that an expected limitation?

Then there were a couple of things in the code that looked wrong to me, which I suspect might fail in certain edge cases. One example being the line widths not taking line rendition into account - I'd expect us to be using the TextBuffer GetLineWidth method to determine the max width, and in the case of the old buffer, that value should already be in cOldColsTotal.

And why are we using the min of the old width and new width for the source column range? Maybe I'm missing something, but that doesn't make sense to me. I would have thought the newAttrColumn only cared about the new width and the copyAttrCol only cared about the old width?

Then, not related to this fix per se, but I noticed that the active attributes are being reset when you resize. I suspect this is because the two lines below are in the wrong order (the old attributes are restored into the buffer that's about to be replaced):

71c75561e5/src/host/screenInfo.cpp (L1474-L1476)

I should also note that I've definitely had some other things looking wrong when messing about resizing randomly, but I need to experiment some more to get reproducible test cases.

@j4james commented on GitHub (Mar 3, 2022): The first thing I noticed is that it only works as far as the last non-blank line. For example, if you do `printf "\e[42m\e[2J"; sleep 10`, and then resizing while it's sleeping, the whole screen goes back to black. Is that an expected limitation? Then there were a couple of things in the code that looked wrong to me, which I suspect might fail in certain edge cases. One example being the line widths not taking line rendition into account - I'd expect us to be using the `TextBuffer` `GetLineWidth` method to determine the max width, and in the case of the old buffer, that value should already be in `cOldColsTotal`. And why are we using the min of the old width and new width for the source column range? Maybe I'm missing something, but that doesn't make sense to me. I would have thought the `newAttrColumn` only cared about the new width and the `copyAttrCol` only cared about the old width? Then, not related to this fix per se, but I noticed that the active attributes are being reset when you resize. I suspect this is because the two lines below are in the wrong order (the old attributes are restored into the buffer that's about to be replaced): https://github.com/microsoft/terminal/blob/71c75561e5df3db53cfe6f9da1173a46441b99ed/src/host/screenInfo.cpp#L1474-L1476 I should also note that I've definitely had some other things looking wrong when messing about resizing randomly, but I need to experiment some more to get reproducible test cases.
claunia commented 2026-01-31 05:25:30 +00:00
Author
Owner
Copy Link

@zadjii-msft commented on GitHub (Mar 3, 2022):

The first thing I noticed is that it only works as far as the last non-blank line. For example, if you do printf "\e[42m\e[2J"; sleep 10, and then resizing while it's sleeping, the whole screen goes back to black. Is that an expected limitation?

In that branch, yea. That's one of the TODOs that definitely needs to be done 😅

Then there were a couple of things in the code that looked wrong to me, which I suspect might fail in certain edge cases. One example being the line widths not taking line rendition into account - I'd expect us to be using the TextBuffer GetLineWidth method to determine the max width, and in the case of the old buffer, that value should already be in cOldColsTotal.

See this is exactly why I asked for your help - I totally forgot about that feature entirely. That can be fixed. Thanks!

And why are we using the min of the old width and new width for the source column range? Maybe I'm missing something, but that doesn't make sense to me. I would have thought the newAttrColumn only cared about the new width and the copyAttrCol only cared about the old width?

Yea, that's an artifact from the first iteration of the fix I forgot to remove. Whoops.

This was a great sanity check! I'll patch those up and see if I can't get some tests written too. If we can confidently say that the fix is "not perfect, but definitely no worse than before", that's good enough for me frankly.

Then, not related to this fix per se, but I noticed that the active attributes are being reset when you resize. I suspect this is because the two lines below are in the wrong order (the old attributes are restored into the buffer that's about to be replaced):

holy shit you're right, that's been there since forever. I wonder if that's a bug floating around on the repo somewhere.

@zadjii-msft commented on GitHub (Mar 3, 2022): > The first thing I noticed is that it only works as far as the last non-blank line. For example, if you do `printf "\e[42m\e[2J"; sleep 10`, and then resizing while it's sleeping, the whole screen goes back to black. Is that an expected limitation? > In that branch, yea. That's one of the TODOs that _definitely needs to be done_ 😅 > Then there were a couple of things in the code that looked wrong to me, which I suspect might fail in certain edge cases. One example being the line widths not taking line rendition into account - I'd expect us to be using the `TextBuffer` `GetLineWidth` method to determine the max width, and in the case of the old buffer, that value should already be in `cOldColsTotal`. > See this is exactly why I asked for your help - I totally forgot about that feature entirely. That can be fixed. Thanks! > And why are we using the min of the old width and new width for the source column range? Maybe I'm missing something, but that doesn't make sense to me. I would have thought the `newAttrColumn` only cared about the new width and the `copyAttrCol` only cared about the old width? > Yea, that's an artifact from the first iteration of the fix I forgot to remove. Whoops. This was a great sanity check! I'll patch those up and see if I can't get some tests written too. If we can confidently say that the fix is "not perfect, but definitely no worse than before", that's good enough for me frankly. <hr> > Then, not related to this fix per se, but I noticed that the active attributes are being reset when you resize. I suspect this is because the two lines below are in the wrong order (the old attributes are restored into the buffer that's about to be replaced): holy shit you're right, that's been there since forever. I wonder if that's a bug floating around on the repo somewhere.
claunia commented 2026-01-31 05:25:30 +00:00
Author
Owner
Copy Link

@ghost commented on GitHub (Apr 19, 2022):

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

Handy links:

@ghost commented on GitHub (Apr 19, 2022): :tada:This issue was addressed in #12637, which has now been successfully released as `Windows Terminal v1.12.1098`.:tada: Handy links: * [Release Notes](https://github.com/microsoft/terminal/releases/tag/v1.12.1098) * [Store Download](https://www.microsoft.com/store/apps/9n8g5rfz9xk3?cid=storebadge&ocid=badge)
claunia commented 2026-01-31 05:25:30 +00:00
Author
Owner
Copy Link

@ghost commented on GitHub (Apr 19, 2022):

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

Handy links:

@ghost commented on GitHub (Apr 19, 2022): :tada:This issue was addressed in #12637, which has now been successfully released as `Windows Terminal Preview v1.13.1098`.:tada: Handy links: * [Release Notes](https://github.com/microsoft/terminal/releases/tag/v1.13.1098) * [Store Download](https://www.microsoft.com/store/apps/9n8g5rfz9xk3?cid=storebadge&ocid=badge)
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
⛺ Reserved
A11yCO
A11yMAS
A11ySev1
A11ySev2
A11ySev3
A11yTTValidated
A11yUsable
A11yVoiceAccess
A11yWCAG
Area-Accessibility
Area-AtlasEngine
Area-AzureShell
Area-Build
Area-Build
Area-Chat
Area-CmdPal
Area-CodeHealth
Area-Commandline
Area-CookedRead
Area-DefApp
Area-Extensibility
Area-Fonts
Area-GroupPolicy
Area-i18n
Area-Input
Area-Interaction
Area-Interop
Area-Localization
Area-Output
Area-Performance
Area-Portable
Area-Quality
Area-Remoting
Area-Rendering
Area-Schema
Area-Server
Area-Settings
Area-SettingsUI
Area-ShellExtension
Area-ShellExtension
Area-ShellExtension
Area-Suggestions
Area-Suggestions
Area-TerminalConnection
Area-TerminalControl
Area-Theming
Area-UserInterface
Area-VT
Area-Windowing
Area-WPFControl
AutoMerge
Blocking-Ingestion
Culprit-Centennial
Culprit-WinUI
Disability-All
Disability-Blind
Disability-LowVision
Disability-Mobility
External-Blocked-WinUI3
Fixed
Gathering-Data
good first issue
HCL-E+D
HCL-WindowsTerminal
Help Wanted
Impact-Compatibility
Impact-Compliance
Impact-Correctness
Impact-Visual
In-PR
InclusionBacklog
InclusionBacklog-Windows TerminalWin32
InclusionCommitted-202206
Issue-Bug
Issue-Docs
Issue-Feature
Issue-Feature
Issue-Question
Issue-Samples
Issue-Scenario
Issue-Task
Needs-Attention
Needs-Author-Feedback
Needs-Bisect
Needs-Discussion
Needs-Repro
Needs-Tag-Fix
Needs-Tag-Fix
Needs-Triage
No-Recent-Activity
Priority-0
Priority-1
Priority-2
Priority-3
Product-Cmd.exe
Product-Colortool
Product-Colortool
Product-Colortool
Product-Conhost
Product-Conpty
Product-Meta
Product-Powershell
Product-Terminal
Product-WSL
pull-request
Mirrored from GitHub Pull Request
 Resolution-Answered
Resolution-By-Design
Resolution-Duplicate
Resolution-External
Resolution-Fix-Available
Resolution-Fix-Committed
Resolution-No-Repro
Resolution-Won't-Fix
Severity-Blocking
Severity-Crash
Severity-DataLoss
spam
this-will-be-a-breaking-change
Tracking-External
WindowsTerminal_Win32
Work-Item
zAskModeBug
zInbox-Bug
No Label Area-Output Issue-Bug Priority-1 Product-Conhost Resolution-Fix-Committed Tracking-External zAskModeBug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
claunia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/terminal#16864
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?