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

Sometimes, repeating spaces are stripped out #1103

Open
2 tasks done
denschub opened this issue Dec 9, 2016 · 12 comments
Open
2 tasks done

Sometimes, repeating spaces are stripped out #1103

denschub opened this issue Dec 9, 2016 · 12 comments
Labels
🐛 Type: Bug Issue pertains to something wrong within Hyper

Comments

@denschub
Copy link

denschub commented Dec 9, 2016

  • I am on the latest Hyper.app version
  • I have searched the issues of this repo and believe that this is not a duplicate
  • OS version and name: macOS, 10.12.1
  • Hyper.app version: 0.8.3
  • Link of a Gist with the contents of your .hyper.js: N/A, it's the default ocnfig.
  • Relevent information from devtools (CMD+SHIFT+I on Mac OS, CTRL+SHIFT+I elsewhere): See below for HTML snippets.
  • The issue is reproducible in vanilla Hyper.app: Yeah!

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:

  • I was not able to reproduce this issue outside of weechat, although, to be fair, that's pretty much the only ncurses based application I actively use.
  • When running this inside a tmux and sending a refresh via tmux refresh-client, the spaces are drawn correctly.

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
Copy link
Contributor

Hey.

Could you try to reproduce this issue on this PR?
#1111

@denschub
Copy link
Author

Still reproducible, unfortunately.

@denschub
Copy link
Author

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
Copy link
Contributor

dotcypress commented Dec 13, 2016

@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
Copy link
Author

denschub commented Dec 13, 2016

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

@dotcypress
Copy link
Contributor

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
Copy link
Author

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
Copy link

rodrigorm commented Jan 9, 2017

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
Copy link
Author

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

@Dog
Copy link

Dog commented Mar 1, 2018

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
Copy link

Dog commented Mar 2, 2018

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
Copy link

Dog commented Mar 6, 2018

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.

@Stanzilla Stanzilla added the 🐛 Type: Bug Issue pertains to something wrong within Hyper label Apr 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Type: Bug Issue pertains to something wrong within Hyper
Projects
None yet
Development

No branches or pull requests

5 participants