From 24c5388e0c603bd0a5beabe56af3b2826943fc9c Mon Sep 17 00:00:00 2001 From: zadam Date: Tue, 7 Jan 2020 19:48:26 +0100 Subject: [PATCH 1/3] protection against text note initialization race conditions --- .../javascripts/services/note_detail.js | 8 +- .../javascripts/services/note_detail_text.js | 77 ++++++++++--------- 2 files changed, 43 insertions(+), 42 deletions(-) diff --git a/src/public/javascripts/services/note_detail.js b/src/public/javascripts/services/note_detail.js index b17bb7d61..ed7a03474 100644 --- a/src/public/javascripts/services/note_detail.js +++ b/src/public/javascripts/services/note_detail.js @@ -201,17 +201,13 @@ async function loadNoteDetail(origNotePath, options = {}) { const newTab = !!options.newTab; const activate = !!options.activate; - const notePath = await treeService.resolveNotePath(origNotePath); + let notePath = await treeService.resolveNotePath(origNotePath); if (!notePath) { console.error(`Cannot resolve note path ${origNotePath}`); // fallback to display something - if (tabContexts.length === 0) { - await openEmptyTab(); - } - - return; + notePath = 'root'; } const noteId = treeUtils.getNoteIdFromNotePath(notePath); diff --git a/src/public/javascripts/services/note_detail_text.js b/src/public/javascripts/services/note_detail_text.js index d81d5aed0..ab48e7e1a 100644 --- a/src/public/javascripts/services/note_detail_text.js +++ b/src/public/javascripts/services/note_detail_text.js @@ -47,6 +47,7 @@ class NoteDetailText { this.ctx = ctx; this.$component = ctx.$tabContent.find('.note-detail-text'); this.$editorEl = this.$component.find('.note-detail-text-editor'); + this.textEditorPromise = null; this.textEditor = null; this.$component.on("dblclick", "img", e => { @@ -67,44 +68,12 @@ class NoteDetailText { } async render() { - if (!this.textEditor) { - await libraryLoader.requireLibrary(libraryLoader.CKEDITOR); - - const codeBlockLanguages = - (await mimeTypesService.getMimeTypes()) - .filter(mt => mt.enabled) - .map(mt => { - return { - language: mt.mime.toLowerCase().replace(/[\W_]+/g,"-"), - label: mt.title - } - }); - - // CKEditor since version 12 needs the element to be visible before initialization. At the same time - // we want to avoid flicker - i.e. show editor only once everything is ready. That's why we have separate - // display of $component in both branches. - this.$component.show(); - - // textEditor might have been initialized during previous await so checking again - // looks like double initialization can freeze CKEditor pretty badly - if (!this.textEditor) { - this.textEditor = await BalloonEditor.create(this.$editorEl[0], { - placeholder: "Type the content of your note here ...", - mention: mentionSetup, - codeBlock: { - languages: codeBlockLanguages - } - }); - - if (glob.isDev && ENABLE_INSPECTOR) { - await import('../../libraries/ckeditor/inspector.js'); - CKEditorInspector.attach(this.textEditor); - } - - this.onNoteChange(() => this.ctx.noteChanged()); - } + if (!this.textEditorPromise) { + this.textEditorPromise = this.initEditor(); } + await this.textEditorPromise; + // lazy loading above can take time and tab might have been already switched to another note if (this.ctx.note && this.ctx.note.type === 'text') { this.textEditor.isReadOnly = await this.isReadOnly(); @@ -115,6 +84,42 @@ class NoteDetailText { } } + async initEditor() { + await libraryLoader.requireLibrary(libraryLoader.CKEDITOR); + + const codeBlockLanguages = + (await mimeTypesService.getMimeTypes()) + .filter(mt => mt.enabled) + .map(mt => { + return { + language: mt.mime.toLowerCase().replace(/[\W_]+/g,"-"), + label: mt.title + } + }); + + // CKEditor since version 12 needs the element to be visible before initialization. At the same time + // we want to avoid flicker - i.e. show editor only once everything is ready. That's why we have separate + // display of $component in both branches. + this.$component.show(); + + const textEditorInstance = await BalloonEditor.create(this.$editorEl[0], { + placeholder: "Type the content of your note here ...", + mention: mentionSetup, + codeBlock: { + languages: codeBlockLanguages + } + }); + + if (glob.isDev && ENABLE_INSPECTOR) { + await import('../../libraries/ckeditor/inspector.js'); + CKEditorInspector.attach(textEditorInstance); + } + + this.textEditor = textEditorInstance; + + this.onNoteChange(() => this.ctx.noteChanged()); + } + getContent() { const content = this.textEditor.getData(); From ccaa9eae3ab2079caa2a8636318d423802ccd6ab Mon Sep 17 00:00:00 2001 From: zadam Date: Tue, 7 Jan 2020 20:53:41 +0100 Subject: [PATCH 2/3] fix context submenus, closes #810 --- src/public/javascripts/services/context_menu.js | 3 +-- src/public/stylesheets/desktop.css | 2 +- src/services/notes.js | 2 ++ 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/public/javascripts/services/context_menu.js b/src/public/javascripts/services/context_menu.js index 713490a55..a0baa028e 100644 --- a/src/public/javascripts/services/context_menu.js +++ b/src/public/javascripts/services/context_menu.js @@ -76,8 +76,7 @@ async function initContextMenu(event, contextMenu) { // in such case we'll position it above click coordinates so it will fit into client const clickPosition = event.pageY; const clientHeight = document.documentElement.clientHeight; - const contextMenuHeight = $contextMenuContainer.height(); - + const contextMenuHeight = $contextMenuContainer.outerHeight() + 30; let top; if (clickPosition + contextMenuHeight > clientHeight) { diff --git a/src/public/stylesheets/desktop.css b/src/public/stylesheets/desktop.css index affa5d15a..078e6b7e0 100644 --- a/src/public/stylesheets/desktop.css +++ b/src/public/stylesheets/desktop.css @@ -110,7 +110,7 @@ body { #context-menu-container { max-height: 100vh; - overflow: auto; /* make it scrollable when exceeding total height of the window */ + /* !!! Cannot set overflow: auto, submenus will break !!! */ } #context-menu-container, #context-menu-container .dropdown-menu { diff --git a/src/services/notes.js b/src/services/notes.js index e654a249d..79b7c193c 100644 --- a/src/services/notes.js +++ b/src/services/notes.js @@ -490,6 +490,8 @@ async function eraseDeletedNotes() { SET isErased = 1, title = NULL WHERE isErased = 0 AND noteId IN (???)`, noteIdsToErase); + + log.info(`Erased notes: ${JSON.stringify(noteIdsToErase)}`); } async function duplicateNote(noteId, parentNoteId) { From f782d2bef9260ca36f89adcf2cb933dc5ea9d01d Mon Sep 17 00:00:00 2001 From: zadam Date: Tue, 7 Jan 2020 22:29:15 +0100 Subject: [PATCH 3/3] don't empty script area on save --- src/public/javascripts/services/tab_context.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/public/javascripts/services/tab_context.js b/src/public/javascripts/services/tab_context.js index 6d732724a..74c885c17 100644 --- a/src/public/javascripts/services/tab_context.js +++ b/src/public/javascripts/services/tab_context.js @@ -350,8 +350,6 @@ class TabContext { this.$savedIndicator.fadeIn(); - this.$scriptArea.empty(); - // run async bundleService.executeRelationBundles(this.note, 'runOnNoteChange', this);