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

No bold text in terminal output #157

Closed
protamail opened this issue Jul 4, 2016 · 13 comments
Closed

No bold text in terminal output #157

protamail opened this issue Jul 4, 2016 · 13 comments

Comments

@protamail
Copy link

isBoldBroken() always returns true since it inserts the test span at body level where the font is most likely not mono-spaced. Instead, it should test at or below the node where terminal font is specified (.terminal). As an example, the below version will allow bold text to be rendered by terminal:

/**
 * @param container terminal's parent node
 */
function isBoldBroken(container) {
      container = container.getElementsByClassName("terminal")[0];
      var el = document.createElement('span');
      el.innerHTML = 'hello world';
      container.appendChild(el);
      var w1 = el.scrollWidth;
      el.style.fontWeight = 'bold';
      var w2 = el.scrollWidth;
      container.removeChild(el);
      return w1 !== w2;
}
@jerch
Copy link
Member

jerch commented Jul 4, 2016

Imo this can't be tested programmatically with some random characters in a reliable manner unless you test a critical subset. There are fonts that handle bold for some glyphs and dont for others (esp. bold box drawing chars are kinda broken in all monospace fonts).
Imo this needs handpicking and documentation of useable and unuseable fonts for a specific task (one font might handle bold cyrillic good while rendering the terminal unusable for greek etc...)

@parisk
Copy link
Contributor

parisk commented Jul 11, 2016

Hi @protasenko, thanks for reporting. Running echo "this is $(tput bold)bold$(tput sgr0) but this isn't" prints the bold text normally.

I think that adding a test which evaluates that printing a fixed sentence with a bold word that will place the xterm-bold class to the bold word would suffice.

If the host application of xterm does not display the bold characters after ensuring that this test succeeds most probably means that this is a font issue and not an xterm.js issue.

@protasenko are you OK with closing this issue?

@protamail
Copy link
Author

Hi @parisk, just cloned latest source and started demo app.js. Your echo test didn't show bold word. Now if I modify index.html like so: <body style="font-family: 'courier new'">... then the bold text shows as expected (assuming 'courier new' is installed on the system of course).
This goes to show the fact that isBoldBroken() test is done at the wrong DOM node document.body, whereas it should be done at #terminal node or below. document.body in most cases is not styled to use monospace font, therefore the isBoldBroken() is true even though there's nothing wrong with the actual terminal font.
Just as FYI, I'm running 'midnight commander' file manager in my version of xterm.js, and everything displays/aligns perfectly including pseudo-graphics, bold text, some files using national characters, etc. All that tested across chrome/IE11/FF, using variety of fonts including webfonts. In fact it renders pixel-perfect compared to my gnome-terminal using the same font. Great stuff!

@parisk
Copy link
Contributor

parisk commented Jul 20, 2016

Thanks @protasenko for the info. I am not sure what midnight commander is actually. Could you provide some more information about it and how it is being used along with xterm.js?

Maybe we should remove the isBoldBroken method. It is quite hacky and I believe that bold handling should be isolated into the presentation layer (CSS).

@sorgloomer
Copy link

Interesting. For me, bold texts began to be broken this week in chrome 54.

image

image

@sorgloomer
Copy link

Turns out I was on 90% zoom, that caused the bold font to have different character size than normal.

@parisk
Copy link
Contributor

parisk commented Nov 28, 2016

Interesting case @sorgloomer. Not sure if we can handle this programmatically though.

@sorgloomer
Copy link

I agree. I did a quick experiment with absolute positioned spans, one per each character, but the result is unbearably slow, and I didn't even try background coloring.

@uudruid74
Copy link

uudruid74 commented Jan 13, 2017 •
edited
Loading

You can "fake" bold in any font using CSS.

.terminal .xterm-bold {
font-weight: normal;
text-shadow: 0.4px 0.4px 0.3px, -0.4px -0.4px 0.3px;
}

@parisk : Midnight commander is a port of an old DOS program originally called Norton Commander

@Tyriar
Copy link
Member

Tyriar commented Jan 13, 2017

Shifting the shadow to the right and having no blur would probably be better so the text is not blurry. Still not ideal though.

.terminal .xterm-bold {
  text-shadow: 1px 0px 0px rgba(0, 0, 0, 1);
}

We could also use a fix similar to what we need to do for braille chars and wrap each char in a fixed width span?

@uudruid74
Copy link

The sub-pixel blur just softens the edges a bit since I like a smoother font. Shifting both left and right by sub-pixel instead of a full pixel right stops the text from running together too much. And don't specify rgba(0,0,0,1) since not all text is black :) But, I don't see why bold in a monospace font would stop being monospace!

@Tyriar
Copy link
Member

Tyriar commented Jan 17, 2017

Yeah bold shouldn't affect the width, this broken bold stuff is just a precaution when the font is broken AFAIK.

@tmyt tmyt mentioned this issue Mar 8, 2017
Merged
@andfoy andfoy mentioned this issue Jul 5, 2017
Closed
@Tyriar
Copy link
Member

Tyriar commented Jun 1, 2018

This issue is stale and no longer relevant with the canvas renderer/chars being forced to align with a grid.

@Tyriar Tyriar closed this as completed Jun 1, 2018
@Tyriar Tyriar added the stale label Jun 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@parisk @Tyriar @sorgloomer @jerch @uudruid74 @protamail