From 64945abbcc527161a6eff8a47922d690677d4c08 Mon Sep 17 00:00:00 2001 From: Mo Bitar Date: Sat, 17 Apr 2021 19:27:16 -0500 Subject: [PATCH] fix: race condition when editor values read could belong to newly selected note as opposed to note for which save was triggered --- .../javascripts/views/editor/editor_view.ts | 125 ++++++++++++------ 1 file changed, 88 insertions(+), 37 deletions(-) diff --git a/app/assets/javascripts/views/editor/editor_view.ts b/app/assets/javascripts/views/editor/editor_view.ts index 96aa4fc04..c70f6f1bd 100644 --- a/app/assets/javascripts/views/editor/editor_view.ts +++ b/app/assets/javascripts/views/editor/editor_view.ts @@ -88,11 +88,15 @@ type EditorState = { }; type EditorValues = { - title?: string; - text?: string; + title: string; + text: string; tagsInputValue?: string; }; +function copyEditorValues(values: EditorValues) { + return Object.assign({}, values); +} + function sortAlphabetically(array: SNComponent[]): SNComponent[] { return array.sort((a, b) => a.name.toLowerCase() < b.name.toLowerCase() ? -1 : 1 @@ -110,7 +114,7 @@ class EditorViewCtrl extends PureViewCtrl { private saveTimeout?: ng.IPromise; private statusTimeout?: ng.IPromise; private lastEditorFocusEventSource?: EventSource; - public editorValues: EditorValues = {}; + public editorValues: EditorValues = { title: '', text: '' }; onEditorLoad?: () => void; private tags: SNTag[] = []; @@ -464,26 +468,30 @@ class EditorViewCtrl extends PureViewCtrl { * close the editor (if we closed the editor before sync began, we'd get an exception, * since the debouncer will be triggered on a non-existent editor) */ - async saveNote( + async save( + note: SNNote, + editorValues: EditorValues, bypassDebouncer = false, isUserModified = false, dontUpdatePreviews = false, customMutate?: (mutator: NoteMutator) => void, closeAfterSync = false ) { + const title = editorValues.title; + const text = editorValues.text; + const isTemplate = this.editor.isTemplateNote; + const selectedTag = this.appState.selectedTag; if (document.hidden) { - this.application.alertService!.alert(STRING_SAVING_WHILE_DOCUMENT_HIDDEN); + this.application.alertService.alert(STRING_SAVING_WHILE_DOCUMENT_HIDDEN); return; } - const note = this.note; if (note.deleted) { - this.application.alertService!.alert(STRING_DELETED_NOTE); + this.application.alertService.alert(STRING_DELETED_NOTE); return; } - if (this.editor.isTemplateNote) { + if (isTemplate) { await this.editor.insertTemplatedNote(); } - const selectedTag = this.appState.selectedTag; if ( !selectedTag?.isSmartTag && !selectedTag?.hasRelationshipWithItem(note) @@ -493,7 +501,7 @@ class EditorViewCtrl extends PureViewCtrl { }); } if (!this.application.findItem(note.uuid)) { - this.application.alertService!.alert(STRING_INVALID_NOTE); + this.application.alertService.alert(STRING_INVALID_NOTE); return; } await this.application.changeItem( @@ -503,12 +511,12 @@ class EditorViewCtrl extends PureViewCtrl { if (customMutate) { customMutate(noteMutator); } - noteMutator.title = this.editorValues.title!; - noteMutator.text = this.editorValues.text!; + noteMutator.title = title; + noteMutator.text = text; if (!dontUpdatePreviews) { - const text = this.editorValues.text || ''; - const truncate = text.length > NOTE_PREVIEW_CHAR_LIMIT; - const substring = text.substring(0, NOTE_PREVIEW_CHAR_LIMIT); + const noteText = text || ''; + const truncate = noteText.length > NOTE_PREVIEW_CHAR_LIMIT; + const substring = noteText.substring(0, NOTE_PREVIEW_CHAR_LIMIT); const previewPlain = substring + (truncate ? STRING_ELLIPSES : ''); noteMutator.preview_plain = previewPlain; noteMutator.preview_html = undefined; @@ -541,7 +549,8 @@ class EditorViewCtrl extends PureViewCtrl { syncTakingTooLong: false, }); this.setStatus({ - message: 'All changes saved' + (this.application.noAccount() ? ' offline' : ''), + message: + 'All changes saved' + (this.application.noAccount() ? ' offline' : ''), }); } @@ -583,7 +592,7 @@ class EditorViewCtrl extends PureViewCtrl { } contentChanged() { - this.saveNote(false, true); + this.save(this.note, copyEditorValues(this.editorValues), false, true); } onTitleEnter($event: Event) { @@ -593,7 +602,13 @@ class EditorViewCtrl extends PureViewCtrl { } onTitleChange() { - this.saveNote(false, true, true); + this.save( + this.note, + copyEditorValues(this.editorValues), + false, + true, + true + ); } focusEditor() { @@ -653,9 +668,16 @@ class EditorViewCtrl extends PureViewCtrl { if (permanently) { this.performNoteDeletion(this.note); } else { - this.saveNote(true, false, true, (mutator) => { - mutator.trashed = true; - }); + this.save( + this.note, + copyEditorValues(this.editorValues), + true, + false, + true, + (mutator) => { + mutator.trashed = true; + } + ); } } } @@ -665,7 +687,9 @@ class EditorViewCtrl extends PureViewCtrl { } restoreTrashedNote() { - this.saveNote( + this.save( + this.note, + copyEditorValues(this.editorValues), true, false, true, @@ -698,15 +722,31 @@ class EditorViewCtrl extends PureViewCtrl { } togglePin() { - this.saveNote(true, false, true, (mutator) => { - mutator.pinned = !this.note.pinned; - }); + const note = this.note; + this.save( + note, + copyEditorValues(this.editorValues), + true, + false, + true, + (mutator) => { + mutator.pinned = !note.pinned; + } + ); } toggleLockNote() { - this.saveNote(true, false, true, (mutator) => { - mutator.locked = !this.note.locked; - }); + const note = this.note; + this.save( + note, + copyEditorValues(this.editorValues), + true, + false, + true, + (mutator) => { + mutator.locked = !note.locked; + } + ); } async toggleProtectNote() { @@ -723,29 +763,40 @@ class EditorViewCtrl extends PureViewCtrl { } toggleNotePreview() { - this.saveNote(true, false, true, (mutator) => { - mutator.hidePreview = !this.note.hidePreview; - }); + const note = this.note; + this.save( + note, + copyEditorValues(this.editorValues), + true, + false, + true, + (mutator) => { + mutator.hidePreview = !note.hidePreview; + } + ); } toggleArchiveNote() { - if (this.note.locked) { + const note = this.note; + if (note.locked) { alertDialog({ - text: this.note.archived + text: note.archived ? STRING_UNARCHIVE_LOCKED_ATTEMPT : STRING_ARCHIVE_LOCKED_ATTEMPT, }); return; } - this.saveNote( + this.save( + note, + copyEditorValues(this.editorValues), true, false, true, (mutator) => { - mutator.archived = !this.note.archived; + mutator.archived = !note.archived; }, /** If we are unarchiving, and we are in the archived tag, close the editor */ - this.note.archived && this.appState.selectedTag?.isArchiveTag + note.archived && this.appState.selectedTag?.isArchiveTag ); } @@ -1176,7 +1227,7 @@ class EditorViewCtrl extends PureViewCtrl { editor.selectionStart = editor.selectionEnd = start + 4; } this.editorValues.text = editor.value; - this.saveNote(true); + this.save(this.note, copyEditorValues(this.editorValues), true); }, });