From f5695149387ddd68a30b53b229340de885c556ea Mon Sep 17 00:00:00 2001 From: Antonio Scandurra Date: Tue, 15 Aug 2017 15:24:32 +0200 Subject: [PATCH] Ensure extra document updates are not scheduled during `updateSync` When changing the editor styles, we force the component to remeasure character dimensions. If they change, each line's height could change too, causing the current scroll top position to not match the viewport the user was observing. Thus, when detecting a line height change, we try to show users the area of the screen they were looking prior to tweaking the font size. In trying to maintain the aforementioned logical position, however, we were mistakenly scheduling a new update before actually finishing the current one. This was problematic because if the first update detected that the longest screen line changed and such line was off-screen, it would try to render it. Before having the chance to measure it, though, the new update would kick in and delete the new longest screen line node, because it assumed it had already been measured. Finally, when `measureContentDuringUpdateSync` fired, it would notice that the longest screen line node did not exist and throw an exception as a result. This commit changes the `updateSync` method to set the `updateScheduled` flag only before returning control to the caller, as opposed to doing so at the beginning. This prevents calls to `scheduleUpdate` made in `updateSync` from scheduling new unwanted updates. --- spec/text-editor-component-spec.js | 17 +++++++++++++++++ src/text-editor-component.js | 11 ++++++++--- 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/spec/text-editor-component-spec.js b/spec/text-editor-component-spec.js index 4c0108b33..bd4f37b7d 100644 --- a/spec/text-editor-component-spec.js +++ b/spec/text-editor-component-spec.js @@ -3610,6 +3610,23 @@ describe('TextEditorComponent', () => { element.style.display = 'none' await component.getNextUpdatePromise() }) + + it('does not throw an exception when the editor is soft-wrapped and changing the font size changes also the longest screen line', async () => { + const {component, element, editor} = buildComponent({rowsPerTile: 3, autoHeight: false}) + editor.setText( + 'Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do\n' + + 'eiusmod tempor incididunt ut labore et dolore magna' + + 'aliqua. Ut enim ad minim veniam, quis nostrud exercitation' + ) + editor.setSoftWrapped(true) + await setEditorHeightInLines(component, 2) + await setEditorWidthInCharacters(component, 56) + await setScrollTop(component, 3 * component.getLineHeight()) + + element.style.fontSize = '20px' + TextEditor.didUpdateStyles() + await component.getNextUpdatePromise() + }) }) describe('synchronous updates', () => { diff --git a/src/text-editor-component.js b/src/text-editor-component.js index 2370774e3..3fd6aa9d3 100644 --- a/src/text-editor-component.js +++ b/src/text-editor-component.js @@ -213,15 +213,17 @@ class TextEditorComponent { } updateSync (useScheduler = false) { - this.updateScheduled = false - // Don't proceed if we know we are not visible - if (!this.visible) return + if (!this.visible) { + this.updateScheduled = false + return + } // Don't proceed if we have to pay for a measurement anyway and detect // that we are no longer visible. if ((this.remeasureCharacterDimensions || this.remeasureAllBlockDecorations) && !this.isVisible()) { if (this.resolveNextUpdatePromise) this.resolveNextUpdatePromise() + this.updateScheduled = false return } @@ -230,6 +232,7 @@ class TextEditorComponent { if (useScheduler && onlyBlinkingCursors) { this.refs.cursorsAndInput.updateCursorBlinkSync(this.cursorsBlinkedOff) if (this.resolveNextUpdatePromise) this.resolveNextUpdatePromise() + this.updateScheduled = false return } @@ -266,6 +269,8 @@ class TextEditorComponent { this.measureContentDuringUpdateSync() this.updateSyncAfterMeasuringContent() } + + this.updateScheduled = false } measureBlockDecorations () {