From 086b2c04e082d42683277a94b57969ebd281c31c Mon Sep 17 00:00:00 2001 From: Daniel Imms Date: Wed, 20 Jun 2018 23:02:05 +1000 Subject: [PATCH] Make sure DOM renderer doesn't render cells beyond cols Fixes #1523 --- src/renderer/dom/DomRenderer.ts | 2 +- .../dom/DomRendererRowFactory.test.ts | 31 ++++++++++++------- src/renderer/dom/DomRendererRowFactory.ts | 10 +++++- 3 files changed, 30 insertions(+), 13 deletions(-) diff --git a/src/renderer/dom/DomRenderer.ts b/src/renderer/dom/DomRenderer.ts index 5b1813b5..8c335dca 100644 --- a/src/renderer/dom/DomRenderer.ts +++ b/src/renderer/dom/DomRenderer.ts @@ -303,7 +303,7 @@ export class DomRenderer extends EventEmitter implements IRenderer { const row = y + terminal.buffer.ydisp; const lineData = terminal.buffer.lines.get(row); - rowElement.appendChild(this._rowFactory.createRow(lineData, row === cursorAbsoluteY, cursorX, terminal.charMeasure.width)); + rowElement.appendChild(this._rowFactory.createRow(lineData, row === cursorAbsoluteY, cursorX, terminal.charMeasure.width, terminal.cols)); } this._terminal.emit('refresh', {start, end}); diff --git a/src/renderer/dom/DomRendererRowFactory.test.ts b/src/renderer/dom/DomRendererRowFactory.test.ts index f28830ce..e91c71fe 100644 --- a/src/renderer/dom/DomRendererRowFactory.test.ts +++ b/src/renderer/dom/DomRendererRowFactory.test.ts @@ -23,7 +23,7 @@ describe('DomRendererRowFactory', () => { describe('createRow', () => { it('should create an element for every character in the row', () => { - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), ' ' + ' ' @@ -34,24 +34,33 @@ describe('DomRendererRowFactory', () => { lineData[0] = [DEFAULT_ATTR, '語', 2, '語'.charCodeAt(0)]; // There should be no element for the following "empty" cell lineData[1] = [DEFAULT_ATTR, '', 0, undefined]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), '語' ); }); it('should add class for cursor', () => { - const fragment = rowFactory.createRow(lineData, true, 0, 5); + const fragment = rowFactory.createRow(lineData, true, 0, 5, 20); assert.equal(getFragmentHtml(fragment), ' ' + ' ' ); }); + it('should not render cells that go beyond the terminal\'s columns', () => { + lineData[0] = [DEFAULT_ATTR, 'a', 1, 'a'.charCodeAt(0)]; + lineData[1] = [DEFAULT_ATTR, 'b', 1, 'b'.charCodeAt(0)]; + const fragment = rowFactory.createRow(lineData, false, 0, 5, 1); + assert.equal(getFragmentHtml(fragment), + 'a' + ); + }); + describe('attributes', () => { it('should add class for bold', () => { lineData[0] = [DEFAULT_ATTR | (FLAGS.BOLD << 18), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -60,7 +69,7 @@ describe('DomRendererRowFactory', () => { it('should add class for italic', () => { lineData[0] = [DEFAULT_ATTR | (FLAGS.ITALIC << 18), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -71,7 +80,7 @@ describe('DomRendererRowFactory', () => { const defaultAttrNoFgColor = (0 << 9) | (256 << 0); for (let i = 0; i < 256; i++) { lineData[0] = [defaultAttrNoFgColor | (i << 9), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), `a` + ' ' @@ -83,7 +92,7 @@ describe('DomRendererRowFactory', () => { const defaultAttrNoBgColor = (257 << 9) | (0 << 0); for (let i = 0; i < 256; i++) { lineData[0] = [defaultAttrNoBgColor | (i << 0), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), `a` + ' ' @@ -93,7 +102,7 @@ describe('DomRendererRowFactory', () => { it('should correctly invert colors', () => { lineData[0] = [(FLAGS.INVERSE << 18) | (2 << 9) | (1 << 0), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -102,7 +111,7 @@ describe('DomRendererRowFactory', () => { it('should correctly invert default fg color', () => { lineData[0] = [(FLAGS.INVERSE << 18) | (257 << 9) | (1 << 0), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -111,7 +120,7 @@ describe('DomRendererRowFactory', () => { it('should correctly invert default bg color', () => { lineData[0] = [(FLAGS.INVERSE << 18) | (1 << 9) | (256 << 0), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), 'a' + ' ' @@ -121,7 +130,7 @@ describe('DomRendererRowFactory', () => { it('should turn bold fg text bright', () => { for (let i = 0; i < 8; i++) { lineData[0] = [(FLAGS.BOLD << 18) | (i << 9) | (256 << 0), 'a', 1, 'a'.charCodeAt(0)]; - const fragment = rowFactory.createRow(lineData, false, 0, 5); + const fragment = rowFactory.createRow(lineData, false, 0, 5, 20); assert.equal(getFragmentHtml(fragment), `a` + ' ' diff --git a/src/renderer/dom/DomRendererRowFactory.ts b/src/renderer/dom/DomRendererRowFactory.ts index d72629a3..eedb1d34 100644 --- a/src/renderer/dom/DomRendererRowFactory.ts +++ b/src/renderer/dom/DomRendererRowFactory.ts @@ -17,9 +17,16 @@ export class DomRendererRowFactory { ) { } - public createRow(lineData: LineData, isCursorRow: boolean, cursorX: number, cellWidth: number): DocumentFragment { + public createRow(lineData: LineData, isCursorRow: boolean, cursorX: number, cellWidth: number, cols: number): DocumentFragment { const fragment = this._document.createDocumentFragment(); + let colCount = 0; + for (let x = 0; x < lineData.length; x++) { + // Don't allow any buffer to the right to be displayed + if (colCount >= cols) { + continue; + } + const charData = lineData[x]; const char: string = charData[CHAR_DATA_CHAR_INDEX]; const attr: number = charData[CHAR_DATA_ATTR_INDEX]; @@ -76,6 +83,7 @@ export class DomRendererRowFactory { charElement.classList.add(`xterm-bg-${bg}`); } fragment.appendChild(charElement); + colCount += width; } return fragment; }