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.

Sign up for GitHub

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

Jump to bottom

Box drawing characters with escape sequence Esc ( 0 are broken since 3.5.0 #1565

Closed
astefanutti opened this issue Jul 12, 2018 · 14 comments
Closed

Box drawing characters with escape sequence Esc ( 0 are broken since 3.5.0 #1565

astefanutti opened this issue Jul 12, 2018 · 14 comments
Assignees
@Tyriar
Milestone
3.5.1

Comments

@astefanutti
Copy link
Contributor

Since Xterm.js version 3.5.0, box drawing characters using the escape sequence Esc ( 0, like \x1b(0\x6A\x1b(B, do not render correctly, while it was working with previous versions.

See https://en.wikipedia.org/wiki/Box-drawing_character#Unix,_CP/M,_BBS

Details

  • Browser and browser version: Chrome
  • OS version: Mac OS X
  • xterm.js version: 3.5.0

Steps to reproduce

<!doctype html>
<html>
  <head>
    <link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/[email protected]/dist/xterm.css" />
    <script src="https://cdn.jsdelivr.net/npm/[email protected]/dist/xterm.js"></script>
  </head>
  <body>
    <div id="terminal"></div>
    <script>
      var term = new Terminal();
      term.open(document.getElementById('terminal'));
      term.write('Test \x1b(0\x6A\x1b(B\n')
    </script>
  </body>
</html>
@Tyriar Tyriar mentioned this issue Jul 12, 2018
Closed
@Tyriar
Copy link
Member

Tyriar commented Jul 12, 2018

I saw this reported in VS Code a while ago microsoft/vscode#53769, I can repro in xterm.js' demo but not VS Code.

@Tyriar
Copy link
Member

Tyriar commented Jul 12, 2018

Copying and pasting this:

screen shot 2018-07-12 at 8 03 12 am

Yields:

❯ echo -e '\x1b(0\x6A\x1b(B\n'
┘

@Tyriar
Copy link
Member

Tyriar commented Jul 12, 2018

This only happens with the canvas renderer

@astefanutti
Copy link
Contributor Author

I saw this reported in VS Code a while ago microsoft/vscode#53769, I can repro in xterm.js' demo but not VS Code.

I can reproduce the issue in VS Code as well.

@Tyriar
Copy link
Member

Tyriar commented Jul 12, 2018

This only happens using the static texture atlas (this is why I couldn't reproduce in VS Code).

@astefanutti 👍

@Tyriar
Copy link
Member

Tyriar commented Jul 12, 2018 •
edited
Loading

The glyph is referencing ascii code 106 (j)

screen shot 2018-07-12 at 8 09 19 am

@Tyriar
Copy link
Member

Tyriar commented Jul 12, 2018

The glyph in 3.4:

screen shot 2018-07-12 at 8 17 50 am

@Tyriar
Copy link
Member

Tyriar commented Jul 12, 2018

3.4 CharData: [131840, "┘", 1, 9496]
3.5 CharData: [131840, "┘", 1, 106]

@Tyriar
Copy link
Member

Tyriar commented Jul 12, 2018

I think #1399 is root cause

@Tyriar
Copy link
Member

Tyriar commented Jul 12, 2018

/cc @jerch

Tyriar added a commit to Tyriar/xterm.js that referenced this issue Jul 12, 2018
@Tyriar
Fix charset drawing
edc38ef 
Fixes xtermjs#1565
Tyriar added a commit to Tyriar/xterm.js that referenced this issue Jul 12, 2018
@Tyriar
Fix charset drawing
61fc4ca 
Fixes xtermjs#1565
@Tyriar Tyriar mentioned this issue Jul 12, 2018
Merged
@Tyriar Tyriar self-assigned this Jul 12, 2018
@Tyriar Tyriar added this to the 3.5.1 milestone Jul 12, 2018
@Tyriar
Copy link
Member

Tyriar commented Jul 12, 2018

@astefanutti thanks for the report, PR with fix is out.

Tyriar added a commit that referenced this issue Jul 12, 2018
@Tyriar
Fix charset drawing
5f7eb6e 
Fixes #1565
@jerch
Copy link
Member

jerch commented Jul 12, 2018

Oh well was wondering about that while doing #1399, guess this means "yes".

@Tyriar Thanks for fixing it. 👍

@astefanutti
Copy link
Contributor Author

@Tyriar thanks for the very quick reply and fix. I confirm it works!

@Tyriar
Copy link
Member

Tyriar commented Jul 14, 2018

@jerch curiously it's only an issue for the static cache though, didn't look in to why (I guess the key is based on the char instead)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Tyriar Tyriar

Labels
None yet
Projects
None yet
Milestone
3.5.1
Development

No branches or pull requests
3 participants
@astefanutti @Tyriar @jerch