From 20ea98ad4149680b6d3e8c44b40bdeb05cd364a2 Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 5 Sep 2017 18:02:48 +0200 Subject: [PATCH] Don't render block decorations located outside the visible range Previously, when trying to use block decorations on non-empty markers, Atom could sometimes throw an error if such markers ended or started at a position that was not currently rendered. In fact, even if we already restricted the decoration query to markers that intersected the visible row range, markers that were only partially visible would still be considered for rendering. If, depending on the `reversed` property, we decided to render the tail or head of the marker in question and this was outside the viewport, Atom would throw the aforementioned exception. This commit addresses the above issue by explicitly ignoring block decorations that are located on rows that are not yet rendered. --- spec/text-editor-component-spec.js | 25 +++++++++++++++++++++++++ src/text-editor-component.js | 8 +++++--- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/spec/text-editor-component-spec.js b/spec/text-editor-component-spec.js index 311759bac..b34d3d766 100644 --- a/spec/text-editor-component-spec.js +++ b/spec/text-editor-component-spec.js @@ -2388,6 +2388,31 @@ describe('TextEditorComponent', () => { ]) }) + it('does not attempt to render block decorations located outside the visible range', async () => { + const {editor, component} = buildComponent({autoHeight: false, rowsPerTile: 2}) + await setEditorHeightInLines(component, 2) + expect(component.getRenderedStartRow()).toBe(0) + expect(component.getRenderedEndRow()).toBe(4) + + const marker1 = editor.markScreenRange([[3, 0], [5, 0]], {reversed: false}) + const item1 = document.createElement('div') + editor.decorateMarker(marker1, {type: 'block', item: item1}) + + const marker2 = editor.markScreenRange([[3, 0], [5, 0]], {reversed: true}) + const item2 = document.createElement('div') + editor.decorateMarker(marker2, {type: 'block', item: item2}) + + await component.getNextUpdatePromise() + expect(item1.parentElement).toBeNull() + expect(item2.nextSibling).toBe(lineNodeForScreenRow(component, 3)) + + await setScrollTop(component, 4 * component.getLineHeight()) + expect(component.getRenderedStartRow()).toBe(4) + expect(component.getRenderedEndRow()).toBe(8) + expect(item1.nextSibling).toBe(lineNodeForScreenRow(component, 5)) + expect(item2.parentElement).toBeNull() + }) + it('measures block decorations correctly when they are added before the component width has been updated', async () => { { const {editor, component, element} = buildComponent({autoHeight: false, width: 500, attach: false}) diff --git a/src/text-editor-component.js b/src/text-editor-component.js index 61868228b..941bffc8d 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -1153,9 +1153,11 @@ class TextEditorComponent { } addBlockDecorationToRender (decoration, screenRange, reversed) { - const screenPosition = reversed ? screenRange.start : screenRange.end - const tileStartRow = this.tileStartRowForRow(screenPosition.row) - const screenLine = this.renderedScreenLines[screenPosition.row - this.getRenderedStartRow()] + const {row} = reversed ? screenRange.start : screenRange.end + if (row < this.getRenderedStartRow() || row >= this.getRenderedEndRow()) return + + const tileStartRow = this.tileStartRowForRow(row) + const screenLine = this.renderedScreenLines[row - this.getRenderedStartRow()] let decorationsByScreenLine = this.decorationsToRender.blocks.get(tileStartRow) if (!decorationsByScreenLine) {