From 71e53042989ccacb07bd27641b6d7a375992d911 Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Thu, 23 Jul 2020 19:56:53 +0000 Subject: [PATCH] Desktop: Fixes #3534: Undoing changes multiple time on an existing note could result in a blank note --- .../NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx | 43 +++++++++++-------- .../gui/NoteEditor/utils/useMessageHandler.ts | 2 +- ElectronClient/gui/NoteRevisionViewer.jsx | 4 +- ReactNativeClient/lib/BaseApplication.js | 2 +- 4 files changed, 28 insertions(+), 23 deletions(-) diff --git a/ElectronClient/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx b/ElectronClient/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx index 604eaa074..a8fea13d7 100644 --- a/ElectronClient/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx +++ b/ElectronClient/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx @@ -162,6 +162,7 @@ const TinyMCE = (props:NoteBodyEditorProps, ref:any) => { const lastOnChangeEventInfo = useRef({ content: null, resourceInfos: null, + contentKey: null, }); const rootIdRef = useRef(`tinymce-${Date.now()}${Math.round(Math.random() * 10000)}`); @@ -722,7 +723,7 @@ const TinyMCE = (props:NoteBodyEditorProps, ref:any) => { for (const cssFile of cssFiles) loadedCssFiles_.push(cssFile); for (const jsFile of jsFiles) loadedJsFiles_.push(jsFile); - console.info('loadDocumentAssets: files to load', cssFiles, jsFiles); + // console.info('loadDocumentAssets: files to load', cssFiles, jsFiles); if (cssFiles.length) { for (const cssFile of cssFiles) { @@ -767,12 +768,31 @@ const TinyMCE = (props:NoteBodyEditorProps, ref:any) => { const result = await props.markupToHtml(props.contentMarkupLanguage, props.content, markupRenderOptions({ resourceInfos: props.resourceInfos })); if (cancelled) return; + editor.setContent(result.html); + + if (lastOnChangeEventInfo.current.contentKey !== props.contentKey) { + // Need to clear UndoManager to avoid this problem: + // - Load note 1 + // - Make a change + // - Load note 2 + // - Undo => content is that of note 1 + // + // The doc is not very clear what's the different between + // clear() and reset() but it seems reset() works best, in + // particular for the onPaste bug. + // + // It seems the undo manager must be reset after having + // set the initial content (not before). Otherwise undoing multiple + // times would result in an empty note. + // https://github.com/laurent22/joplin/issues/3534 + editor.undoManager.reset(); + } + lastOnChangeEventInfo.current = { content: props.content, resourceInfos: props.resourceInfos, + contentKey: props.contentKey, }; - - editor.setContent(result.html); } await loadDocumentAssets(editor, await props.allAssets(props.contentMarkupLanguage)); @@ -785,22 +805,7 @@ const TinyMCE = (props:NoteBodyEditorProps, ref:any) => { return () => { cancelled = true; }; - }, [editor, props.markupToHtml, props.allAssets, props.content, props.resourceInfos]); - - useEffect(() => { - if (!editor) return; - - // Need to clear UndoManager to avoid this problem: - // - Load note 1 - // - Make a change - // - Load note 2 - // - Undo => content is that of note 1 - - // The doc is not very clear what's the different between - // clear() and reset() but it seems reset() works best, in - // particular for the onPaste bug. - editor.undoManager.reset(); - }, [editor, props.contentKey]); + }, [editor, props.markupToHtml, props.allAssets, props.content, props.resourceInfos, props.contentKey]); useEffect(() => { if (!editor) return () => {}; diff --git a/ElectronClient/gui/NoteEditor/utils/useMessageHandler.ts b/ElectronClient/gui/NoteEditor/utils/useMessageHandler.ts index e5aeb8c94..f1100e155 100644 --- a/ElectronClient/gui/NoteEditor/utils/useMessageHandler.ts +++ b/ElectronClient/gui/NoteEditor/utils/useMessageHandler.ts @@ -18,7 +18,7 @@ export default function useMessageHandler(scrollWhenReady:any, setScrollWhenRead const args = event.args; const arg0 = args && args.length >= 1 ? args[0] : null; - if (msg !== 'percentScroll') console.info(`Got ipc-message: ${msg}`, arg0); + // if (msg !== 'percentScroll') console.info(`Got ipc-message: ${msg}`, arg0); if (msg.indexOf('error:') === 0) { const s = msg.split(':'); diff --git a/ElectronClient/gui/NoteRevisionViewer.jsx b/ElectronClient/gui/NoteRevisionViewer.jsx index b2d153feb..b129f66f8 100644 --- a/ElectronClient/gui/NoteRevisionViewer.jsx +++ b/ElectronClient/gui/NoteRevisionViewer.jsx @@ -139,9 +139,9 @@ class NoteRevisionViewerComponent extends React.PureComponent { // We try to get most links work though, except for internal (joplin://) links. const msg = event.channel ? event.channel : ''; - const args = event.args; + // const args = event.args; - if (msg !== 'percentScroll') console.info(`Got ipc-message: ${msg}`, args); + // if (msg !== 'percentScroll') console.info(`Got ipc-message: ${msg}`, args); try { if (msg.indexOf('joplin://') === 0) { diff --git a/ReactNativeClient/lib/BaseApplication.js b/ReactNativeClient/lib/BaseApplication.js index 752334306..6dfc377be 100644 --- a/ReactNativeClient/lib/BaseApplication.js +++ b/ReactNativeClient/lib/BaseApplication.js @@ -662,7 +662,7 @@ class BaseApplication { this.dbLogger_.setLevel(initArgs.logLevel); if (Setting.value('env') === 'dev' && Setting.value('appType') === 'desktop') { - this.logger_.addTarget('console', { level: Logger.LEVEL_DEBUG }); + // this.logger_.addTarget('console', { level: Logger.LEVEL_DEBUG }); this.dbLogger_.addTarget('console', { level: Logger.LEVEL_WARN }); }