vercel / hyper

A terminal built on web technologies
https://hyper.is
MIT License
43.25k stars 3.51k forks source link

Sometimes, repeating spaces are stripped out #1103

Open denschub opened 7 years ago

denschub commented 7 years ago

Issue

I experience a weird issue when using weechat with two sidebars. It's probably best if I show you screenshots. This is how it should look like:

screen shot 2016-12-08 at 17 15 56

And this is how it looks like:

screen shot 2016-12-08 at 17 16 08

Apparently, sometimes duplicate spaces get striped/merged together. Now, here comes the crazy stuff:

Here is the HTML that's generated for a line that's missing spaces. The first line is "broken", the second line is exactly the same content after a refresh:

<!-- broken --><x-row>                  <span style="color: rgb(88, 88, 88); width: 7.2px; display: inline-block; overflow: visible; position: relative;">│                                                                                                          </span><span style="color: rgb(88, 88, 88); width: 7.2px; display: inline-block; overflow: visible; position: relative;">│</span><span style="color: rgb(174, 129, 255);"> </span><span style="color: rgb(73, 72, 62);">D</span><span style="color: rgb(73, 72, 62);">e</span><span style="color: rgb(73, 72, 62);">n</span><span style="color: rgb(73, 72, 62);">S</span><span style="color: rgb(73, 72, 62);">c</span><span style="color: rgb(73, 72, 62);">h</span><span style="color: rgb(73, 72, 62);">u</span><span style="color: rgb(73, 72, 62);">b</span><span style="color: rgb(73, 72, 62);"> </span><span style="color: rgb(73, 72, 62);"> </span><span style="color: rgb(73, 72, 62);"> </span><span style="color: rgb(73, 72, 62);"> </span></x-row>
<!-- good   --><x-row>                  <span style="color: rgb(88, 88, 88); width: 7.2px; display: inline-block; overflow: visible; position: relative;">│</span>                                                                                                          <span style="color: rgb(88, 88, 88); width: 7.2px; display: inline-block; overflow: visible; position: relative;">│</span><span style="color: rgb(174, 129, 255);"> </span><span style="color: rgb(73, 72, 62);">D</span><span style="color: rgb(73, 72, 62);">e</span><span style="color: rgb(73, 72, 62);">n</span><span style="color: rgb(73, 72, 62);">S</span><span style="color: rgb(73, 72, 62);">c</span><span style="color: rgb(73, 72, 62);">h</span><span style="color: rgb(73, 72, 62);">u</span><span style="color: rgb(73, 72, 62);">b</span><span style="color: rgb(73, 72, 62);"> </span><span style="color: rgb(73, 72, 62);"> </span><span style="color: rgb(73, 72, 62);"> </span><span style="color: rgb(73, 72, 62);"> </span></x-row>

It's pretty easy to spot that, in the broken case, the spaces are inside the span tag that colors the border line. This is bad since this span tag has a width: 7.2px style...

Since I was not able to build a simplified testcase, that's all the information I can provide at this moment. Maybe it's useful for someone.

dotcypress commented 7 years ago

Hey.

Could you try to reproduce this issue on this PR? https://github.com/zeit/hyper/pull/1111

denschub commented 7 years ago

Still reproducible, unfortunately.

denschub commented 7 years ago

So, fwiw, this is the HTML with your PR:

<!-- broken --><x-row> <span style="color: rgb(51, 255, 0);">8.</span>freenode                     <span class=" unicode-node" style="color: rgb(88, 88, 88);">│                                                                                                                                                                                                                  </span><span class=" unicode-node" style="color: rgb(88, 88, 88);">│</span><span style="color: rgb(0, 102, 255);"> </span><span style="color: rgb(255, 255, 255);">DenSchub   </span></x-row>
<!-- good   --><x-row> <span style="color: rgb(51, 255, 0);">8.</span>freenode                     <span class=" unicode-node" style="color: rgb(88, 88, 88);">│</span>                                                                                                                                                                                                                  <span class=" unicode-node" style="color: rgb(88, 88, 88);">│</span><span style="color: rgb(0, 102, 255);"> </span><span style="color: rgb(255, 255, 255);">DenSchub   </span></x-row>

Same story, but the width definition is inside your unicode-node class now. I've been looking around for a bit and I've noticed that this class should only get added if the length of their contents is 1:

if (container.style && runes(text).length === 1 && containsNonLatinCodepoints(text)) {
  container.className += ' unicode-node';
}

and that made me wonder. By default, innerText, which apparentl is used here somewhere in the chain of calls, strips whitespaces unless white-space: pre; is set. However, x-screen has that in its style attribute.

I've looked at window.getComputedStyle(container)["white-space"] inside that condition and realized that all those attributes are unset, which probably is the root cause of the issue.

@dotcypress is this helpful to you or should I continue debugging up to see where we could fix that?

dotcypress commented 7 years ago

@denschub thank you for help.

We need to check length of symbols, cause we need to set width for each non-latin character.

I'm trying to find place where spaces drop may happen. Do you have this issue with other programs(mc, nano, vtop, etc.)?

denschub commented 7 years ago

Sadly, no. :( Weechat is the only application where I have experienced this issue so far...

dotcypress commented 7 years ago

Maybe it happen because this: https://chromium.googlesource.com/chromiumos/platform/assets/+/factory-3004.B/chromeapps/hterm/js/lib_utf8.js#105

But i'm not sure, need more research! 😄

denschub commented 7 years ago

I guess the best way is to find out where/why the spaces are wrapped inside that span, but after a forced repaint via tmux they're no longer. I'd poke around with it, but so far, the devtools are unable to break at that source location for some reason, need to fix that first... :)

rodrigorm commented 7 years ago

I'm having the same problem using vim inside a screen session. I have found in developer tools the same problem with stripped white spaces.

denschub commented 7 years ago

Hm, vim works fine for me. That's surprising and makes debugging this even harder.. :(

Dog commented 6 years ago

FWIW I still experience this in weechat.

From what I can tell in the dev tools, spaces are being dropped between the first blue pipe character used for the border and whatever the next pipe character is. I can manually fix lines by inserting the spaces.

However, the number of spaces between empty lines doesn't seem to be always consistent. (One row may be missing 10 spaces, and the next row is missing 13). The number of spaces on a row may change by switching to another buffer, and then back to the original buffer.

I've noticed it most often for two cases:

  1. We want to render no text in the chat for an x-row, but we want to render a username after a pipe for user list.
  2. A user submits two messages to the chat sequentially and the username and timestamp are not printed for the second line (Weechat-slack does this by default)

I'm still not sure why this happening in hyper though.

Dog commented 6 years ago

This issue appears to be fixed with the canary build.

I took the current build from master and made my own binary. The spacing issue appears to be fixed both in weechat-slack and my other channels so far.

Dog commented 6 years ago

I wanted to come back and report that the issue still exists in the canary build. It didn't seem to appear until a couple days later.