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

ESC[0m won't reset colors if followed by new line #18378

Closed
CarterLi opened this issue Feb 21, 2023 · 6 comments · Fixed by #19361
Closed

ESC[0m won't reset colors if followed by new line #18378

CarterLi opened this issue Feb 21, 2023 · 6 comments · Fixed by #19361
Labels
blocked Don't land until something else happens first (see task list) bug

Comments

@CarterLi
Copy link

Explain what happens

echo -e '\033[41;31m!\033[0m\n'

Expected: (iTerm2)

image

Actual: (Cockpit)

image

Version of Cockpit

285

Where is the problem in Cockpit?

Terminal

Server operating system

Fedora

Server operating system version

Linux eoitek.net 6.2.0-0.rc8.20230217gitec35307e18ba.60.fc39.x86_64 #1 SMP PREEMPT_DYNAMIC Fri Feb 17 16:19:48 UTC 2023 x86_64 GNU/Linux

What browsers are you using?

Chrome, Edge

System log

No response

@garrett
Copy link
Member

garrett commented Feb 21, 2023

This is not a Cockpit bug, but a terminal emulation bug.

We use xterm.js to render the console, specifically version 5.1.0 at the moment (this is the latest release), with the canvas addon that is version 0.3.0.

That said, VS Code also uses xterm.js — and I tested it in VS Codium and got this:

image

So at least one implementation is correct... but they're using a different renderer by default (at least on my system).

As VS Code uses Electron, I also tried Cockpit in Chrome (as they'd effectively use the same browser engine). The this bug still shows up in Cockpit's terminal:

image

But in VS Codium have "auto" as my renderer:

image

When I switch the renderer to "canvas", I see the bug in VS Codium too:

image

Here's the upstream issue @ xterm.js:

We'll have to wait for upstream to fix this and we'll update the dependency when they do.

@CarterLi
Copy link
Author

Let's wait for xterm.js change then

@KKoukiou KKoukiou added the blocked Don't land until something else happens first (see task list) label Feb 22, 2023
@CarterLi
Copy link
Author

The new version of xterm has been released for several weeks. Does the problem still exist?

@martinpitt
Copy link
Member

We can't update to xterm 5.2, as that breaks the accessibility tree: xtermjs/xterm.js#4571 .

@garrett
Copy link
Member

garrett commented Sep 20, 2023

xterm.js fixed the accessibility tree in the PR @ xtermjs/xterm.js#4637 — we should consider updating.

@jelly
Copy link
Member

jelly commented Sep 20, 2023

It's fixed in 5.3.0

jelly added a commit to jelly/cockpit that referenced this issue Sep 20, 2023
martinpitt pushed a commit that referenced this issue Sep 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Don't land until something else happens first (see task list) bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants