New issue
Advanced search Search tips

Issue 727957 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: May 2017
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug-Regression



Sign in to add a comment

hterm: wide char display calculation broken (wc-node.width)

Project Member Reported by vapier@chromium.org, May 30 2017

Issue description

while changing the CSS rules in hterm.Terminal.prototype.decorate, the wc-node link was inadvertently broken.
  https://chromium-review.googlesource.com/472106

rather than update the fragile indexing logic to pull out the wc-node again, we should switch to CSS vars.  this will let us simplify the logic in a few places by not having to reset a bunch of hardcoded CSS rules ... they all refer to this one variable, so as soon as we tweak it, all the other CSS rules update automatically.
 
Project Member

Comment 1 by bugdroid1@chromium.org, May 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/apps/libapps/+/66beb0bba799380e161b6f5281f863ebd1c0c22f

commit 66beb0bba799380e161b6f5281f863ebd1c0c22f
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed May 31 21:22:56 2017

hterm: add CSS vars for char width & height

Rather than update a bunch of specific CSS rules everytime the font size
changes, switch to using CSS variables.  Now we only have to update one
place in CSS and the rest is handled by the browser.  This makes it more
resilient to future changes, and lets us delete more JS logic.

BUG= chromium:727957 

Change-Id: I8c076423c8cff249aa13b2e2825400c4ec7aa292
Reviewed-on: https://chromium-review.googlesource.com/517890
Reviewed-by: Brandon Gilmore <varz@google.com>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/66beb0bba799380e161b6f5281f863ebd1c0c22f/hterm/js/hterm_terminal.js
[modify] https://crrev.com/66beb0bba799380e161b6f5281f863ebd1c0c22f/hterm/js/hterm_scrollport.js

Project Member

Comment 2 by bugdroid1@chromium.org, May 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/apps/libapps/+/d680492dff4ec2dbc4e5351b8fa4be3e416df72a

commit d680492dff4ec2dbc4e5351b8fa4be3e416df72a
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed May 31 21:22:57 2017

hterm: simplify char size measuring

By using the ES6 .repeat() helper, we can collapse a good amount of
code here to the initialization code path.  This should functionally
be the same.

BUG= chromium:727957 

Change-Id: Id2f4476d6b65e5f6c7e6a56885883a965d185a0e
Reviewed-on: https://chromium-review.googlesource.com/519188
Reviewed-by: Brandon Gilmore <varz@google.com>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/d680492dff4ec2dbc4e5351b8fa4be3e416df72a/hterm/js/hterm_scrollport.js

Project Member

Comment 3 by bugdroid1@chromium.org, May 31 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/apps/libapps/+/4036f6e258c8f998fb43f0051997f002b2842b28

commit 4036f6e258c8f998fb43f0051997f002b2842b28
Author: Mike Frysinger <vapier@chromium.org>
Date: Wed May 31 21:23:29 2017

hterm: set line-height of wide-characters to exact height

Currently if you display a wide character, the font will often cause the
line height to be a bit bigger than normal.  This leads to weird jank as
the lines shift up/down based on what characters happen to be in them.
By setting the line-height of the wide characters span, we force them to
render in the same space as other "normal" content.

Depending on the glyph, this might overlap with previous lines, but since
the current behavior overlaps with successive lines and looks ugly, this
should hopefully provide a more consistent experience.

BUG= chromium:727957 

Change-Id: I95cdc1e068724754bca09f53bb3c71690ead1054
Reviewed-on: https://chromium-review.googlesource.com/519762
Reviewed-by: Brandon Gilmore <varz@google.com>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/4036f6e258c8f998fb43f0051997f002b2842b28/hterm/js/hterm_terminal.js

Comment 4 by vapier@chromium.org, May 31 2017

Status: Fixed (was: Available)
this will be fixed in hterm-1.66+ and nassh-0.8.36.6+
Project Member

Comment 5 by bugdroid1@chromium.org, Jun 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/apps/libapps/+/d7d69e976d9de665df79b38834a8a6da6f88bc07

commit d7d69e976d9de665df79b38834a8a6da6f88bc07
Author: Mike Frysinger <vapier@chromium.org>
Date: Thu Jun 01 20:26:58 2017

hterm: set line-height of x-row to exact height

Just like wide character glyphs can be too tall, narrow characters can
be as well.  Set the line height for all the rows to be the same.  An
example is U+0F3C (༼) which throws the whole line off.

BUG= chromium:727957 

Change-Id: I9970a45bba28f0ac2af91ace8ea2ba160d075ef6
Reviewed-on: https://chromium-review.googlesource.com/521582
Reviewed-by: Brandon Gilmore <varz@google.com>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/d7d69e976d9de665df79b38834a8a6da6f88bc07/hterm/js/hterm_scrollport.js

Project Member

Comment 6 by bugdroid1@chromium.org, Jun 1 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/apps/libapps/+/d7d69e976d9de665df79b38834a8a6da6f88bc07

commit d7d69e976d9de665df79b38834a8a6da6f88bc07
Author: Mike Frysinger <vapier@chromium.org>
Date: Thu Jun 01 20:26:58 2017

hterm: set line-height of x-row to exact height

Just like wide character glyphs can be too tall, narrow characters can
be as well.  Set the line height for all the rows to be the same.  An
example is U+0F3C (༼) which throws the whole line off.

BUG= chromium:727957 

Change-Id: I9970a45bba28f0ac2af91ace8ea2ba160d075ef6
Reviewed-on: https://chromium-review.googlesource.com/521582
Reviewed-by: Brandon Gilmore <varz@google.com>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/d7d69e976d9de665df79b38834a8a6da6f88bc07/hterm/js/hterm_scrollport.js

Sign in to add a comment