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

Renderer crash when resizing terminal with text-wrap #1926

Closed
zxlin opened this issue Feb 2, 2019 · 5 comments
Closed

Renderer crash when resizing terminal with text-wrap #1926

zxlin opened this issue Feb 2, 2019 · 5 comments
Assignees
Labels
type/bug Something is misbehaving
Milestone

Comments

@zxlin
Copy link

zxlin commented Feb 2, 2019

Details

  • Browser and browser version: Chrome 71.0.3578.98, Safari 12.0.2 (14606.3.4), Firefox 65.0
  • OS version: Windows 10 & macOS Mojave
  • xterm.js version: v3.11.0

Steps to reproduce

  1. Visit: https://4jnl8qxxn4.codesandbox.io/
  2. Open browser dev console
  3. Click "Play Example" button
  4. Observe TextRenderLayer crash

Error trace copied below:

Uncaught TypeError: Cannot read property 'get' of undefined
    at TextRenderLayer._forEachCell (TextRenderLayer.js:51)
    at TextRenderLayer._drawBackground (TextRenderLayer.js:101)
    at TextRenderLayer.onGridChanged (TextRenderLayer.js:164)
    at eval (Renderer.js:166)
    at Array.forEach (<anonymous>)
    at Renderer._renderRows (Renderer.js:166)
    at RenderDebouncer._innerRefresh (RenderDebouncer.js:31)
    at eval (RenderDebouncer.js:26)

This only started with v3.11.0, this did not happen in v3.10.1 (I've gone back and tested the same sample on v3.10.1).

Something of note is that this is only an issue when you have a long text gets printed to the terminal and then you resize the terminal smaller, and then resize it bigger. Another interesting behavior is that this ONLY occurs if you resize both rows AND columns, if you simply resize columns, without resizing rows, it does not crash the render (simply resizing rows also does not crash).

I would imagine it's probably something introduced by text-wrap feature, but I'm not too sure.

Thanks in advance to anyone looking into this!

Code snippet to play with: https://codesandbox.io/s/4jnl8qxxn4

@Tyriar
Copy link
Member

Tyriar commented Feb 2, 2019

My guess is that ybase is being set based on _rows, instead of newRows which will cause it to have an invalid value when newRows is larger.

@thenengah
Copy link

thenengah commented Feb 5, 2019

Also running into this issue. [UPDATED] na, actually mine is a little different. But it is on resize.

screen shot 2019-02-05 at 10 05 19 am

I downgraded to 3.10.0 and that fixed the issue. Will revisit this issue after this bug is addressed as it might affect the issue I was facing.

@Tyriar
Copy link
Member

Tyriar commented Feb 5, 2019

Reduced repro in demo:

  1. Run in console: term.resize(40,40)
  2. Paste console core fd full mqueue null ptmx pts random stderr stdin stdout tty urandom zero and press enter
  3. Run in console: term.resize(100,100)
  4. Run in console: term.resize(200,200)

This throws because lines.length === 198 (not 200)

Tyriar added a commit to Tyriar/xterm.js that referenced this issue Feb 5, 2019
@Tyriar Tyriar added this to the 3.11.1 milestone Feb 5, 2019
@Tyriar Tyriar self-assigned this Feb 5, 2019
@Tyriar Tyriar added the type/bug Something is misbehaving label Feb 5, 2019
Tyriar added a commit to microsoft/xterm.js that referenced this issue Feb 5, 2019
@pfitzseb
Copy link
Contributor

pfitzseb commented Feb 7, 2019

Somewhat similar error which I didn't yet have success reproducing:

Uncaught TypeError: Cannot read property 'copyCellsFrom' of undefined
    at Buffer._reflowSmaller (D:\Dokumente\Projekte\Juno\atom-ink\node_modules\xterm\lib\Buffer.js:250:45)
    at Buffer._reflow (D:\Dokumente\Projekte\Juno\atom-ink\node_modules\xterm\lib\Buffer.js:170:18)
    at Buffer.resize (D:\Dokumente\Projekte\Juno\atom-ink\node_modules\xterm\lib\Buffer.js:152:18)
    at BufferSet.resize (D:\Dokumente\Projekte\Juno\atom-ink\node_modules\xterm\lib\BufferSet.js:78:22)
    at Terminal.resize (D:\Dokumente\Projekte\Juno\atom-ink\node_modules\xterm\lib\Terminal.js:1096:22)
    at Terminal.resize (D:\Dokumente\Projekte\Juno\atom-ink\node_modules\xterm\lib\public\Terminal.js:53:20)

Tyriar added a commit to microsoft/xterm.js that referenced this issue Feb 7, 2019
@zxlin
Copy link
Author

zxlin commented Feb 12, 2019

@Tyriar Thank you for all your hard work!
I tested microsoft@88d424f and it seems to have resolved the bug. I know #1930 needs to be merged in first before this fix can go in, any thoughts on if that could possibly happen this week?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug Something is misbehaving
Projects
None yet
Development

No branches or pull requests

4 participants