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

Bug in resize/fit #875

Closed
ioquatix opened this issue Aug 9, 2017 · 6 comments
Closed

Bug in resize/fit #875

ioquatix opened this issue Aug 9, 2017 · 6 comments

Comments

@ioquatix
Copy link
Contributor

ioquatix commented Aug 9, 2017

In some cases xterm fails because an off by n error in the render function.

Details

  • Browser and browser version: Atom 1.19.0 / Electron
  • OS version: Mac OS X 10.12.6
  • xterm.js version: 2.9.1

Steps to reproduce

  1. Create the terminal of size rows: 40 cols: 80
  2. Immediately call fit which makes it bigger, e.g. {terminal: Terminal, cols: 82, rows: 45}
  3. Crash in Render.
      for (let i = 0; i < width; i++) { // i = 80, width = 82, lines has 80 things in it.
        // TODO: Could data be a more specific type?
        let data: any = line[i][0]; // lines[80] is undefined.
@ioquatix
Copy link
Contributor Author

ioquatix commented Aug 9, 2017

A quick hack to work around this is to make sure the initial size is always bigger than the first resize:

    @term = new Terminal {
      rows: 400
      cols: 800
      scrollback: atom.config.get('script-runner.scrollback'),
      useStyle: no
      screenKeys: no
      handler: (data) =>
        @emitter.emit('data', data)
      cursorBlink: yes
    }

Seems like a massive hack though.

@ioquatix
Copy link
Contributor Author

ioquatix commented Aug 9, 2017

The problem is reproducible with this branch:

ioquatix/script-runner@4943992#diff-ff2e3d1abc5ef3094441fd3d980abbe4R95

@ioquatix
Copy link
Contributor Author

ioquatix commented Aug 9, 2017

(but you need to change cols: 80 * 4 to cols: 1 to crash every time).

Once the terminal is working/displaying/refreshing correctly, calling resize works as expected.

@Tyriar
Copy link
Member

Tyriar commented Aug 9, 2017

Duplicate of #860, it's already fixed in master. Hopefully a release should come soon 😃

@Tyriar Tyriar closed this as completed Aug 9, 2017
@ioquatix
Copy link
Contributor Author

ioquatix commented Aug 9, 2017

Cool can you let me know when it's released? Thanks :)

@parisk
Copy link
Contributor

parisk commented Aug 9, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants