From 2c9bf9f03a2ddcdfd000bc256b436de78abb53ba Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Fri, 17 Nov 2023 16:47:05 +0000 Subject: [PATCH] Desktop: Fixed copying and pasting an image from Chrome in RTE --- .eslintignore | 2 + .gitignore | 2 + .../NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx | 11 ++--- .../utils/shouldPasteResources.test.ts | 47 ++++++++++++++++++ .../TinyMCE/utils/shouldPasteResources.ts | 49 +++++++++++++++++++ packages/renderer/htmlUtils.test.ts | 37 +++++++++++++- packages/renderer/htmlUtils.ts | 29 +++++++++++ 7 files changed, 168 insertions(+), 9 deletions(-) create mode 100644 packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.test.ts create mode 100644 packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.ts diff --git a/.eslintignore b/.eslintignore index 170ff2c62..b73dcce14 100644 --- a/.eslintignore +++ b/.eslintignore @@ -249,6 +249,8 @@ packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/styles/index.js packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/joplinCommandToTinyMceCommands.js packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/openEditDialog.js packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/setupToolbarButtons.js +packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.test.js +packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.js packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/types.js packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useContextMenu.js packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useScroll.js diff --git a/.gitignore b/.gitignore index 24e029020..b61c9fb41 100644 --- a/.gitignore +++ b/.gitignore @@ -231,6 +231,8 @@ packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/styles/index.js packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/joplinCommandToTinyMceCommands.js packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/openEditDialog.js packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/setupToolbarButtons.js +packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.test.js +packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.js packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/types.js packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useContextMenu.js packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/useScroll.js diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx b/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx index 7fa4055a6..c9f557964 100644 --- a/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx +++ b/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx @@ -27,6 +27,7 @@ import bridge from '../../../../services/bridge'; import { TinyMceEditorEvents } from './utils/types'; import type { Editor } from 'tinymce'; import { joplinCommandToTinyMceCommands, TinyMceCommand } from './utils/joplinCommandToTinyMceCommands'; +import shouldPasteResources from './utils/shouldPasteResources'; const { clipboard } = require('electron'); const supportedLocales = require('./supportedLocales'); @@ -1085,15 +1086,9 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => { // formatted text. const pastedHtml = event.clipboardData.getData('text/html') ? clipboard.readHTML() : ''; - // We should only process the images if there is no plain text or - // HTML text in the clipboard. This is because certain applications, - // such as Word, are going to add multiple versions of the copied - // data to the clipboard - one with the text formatted as HTML, and - // one with the text as an image. In that case, we need to ignore - // the image and only process the HTML. + const resourceMds = await getResourcesFromPasteEvent(event); - if (!pastedText && !pastedHtml) { - const resourceMds = await getResourcesFromPasteEvent(event); + if (shouldPasteResources(pastedText, pastedHtml, resourceMds)) { if (resourceMds.length) { const result = await markupToHtml.current(MarkupToHtml.MARKUP_LANGUAGE_MARKDOWN, resourceMds.join('\n'), markupRenderOptions({ bodyOnly: true })); editor.insertContent(result.html); diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.test.ts b/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.test.ts new file mode 100644 index 000000000..539d732ff --- /dev/null +++ b/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.test.ts @@ -0,0 +1,47 @@ +import shouldPasteResources from './shouldPasteResources'; + +describe('shouldPasteResources', () => { + + test.each([ + [ + '', + '', + [], + true, + ], + [ + 'some text', + '', + [], + false, + ], + [ + '', + 'some html', + [], + false, + ], + [ + '', + '', + [], + false, + ], + [ + 'some text', + '', + [], + false, + ], + [ + '', + '

Some text

', + [], + false, + ], + ])('should tell if clipboard content should be processed as resources', (pastedText, pastedHtml, resourceMds, expected) => { + const actual = shouldPasteResources(pastedText, pastedHtml, resourceMds); + expect(actual).toBe(expected); + }); + +}); diff --git a/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.ts b/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.ts new file mode 100644 index 000000000..7bc22f9f9 --- /dev/null +++ b/packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/utils/shouldPasteResources.ts @@ -0,0 +1,49 @@ +import { htmlDocIsImageOnly } from '@joplin/renderer/htmlUtils'; +import Logger from '@joplin/utils/Logger'; + +const logger = Logger.create('shouldPasteResources'); + +// We should only process the images if there is no plain text or HTML text in +// the clipboard. This is because certain applications, such as Word, are going +// to add multiple versions of the copied data to the clipboard - one with the +// text formatted as HTML, and one with the text as an image. In that case, we +// need to ignore the image and only process the HTML. +// +// Additional source of troubles is that when copying an image from Chrome, the +// clipboard will contain two elements: The actual image (type=image), and an +// HTML fragment with a link to the image. Most of the time getting the image +// from the HTML will work... except if some authentication is required to +// access the image. In that case we'll end up with dead link in the RTE. For +// that reason, when there's only an image in the HTML document, we process +// instead the clipboard resources, which will contain the actual image. +// +// We have a lot of log statements so that if someone reports a bug we can ask +// them to check the console and give us the messages they have. +export default (pastedText: string, pastedHtml: string, resourceMds: string[]) => { + logger.info('Pasted text:', pastedText); + logger.info('Pasted HTML:', pastedHtml); + logger.info('Resources:', resourceMds); + + if (pastedText) { + logger.info('Not pasting resources because the clipboard contains plain text'); + return false; + } + + if (pastedHtml) { + if (!htmlDocIsImageOnly(pastedHtml)) { + logger.info('Not pasting resources because the clipboard contains HTML, which contains more than just one image'); + return false; + } else { + logger.info('Not pasting HTML because it only contains one image.'); + } + + if (!resourceMds.length) { + logger.info('Not pasting resources because there isn\'t any'); + return false; + } + } + + logger.info('Pasting resources'); + + return true; +}; diff --git a/packages/renderer/htmlUtils.test.ts b/packages/renderer/htmlUtils.test.ts index 51732c780..a91873596 100644 --- a/packages/renderer/htmlUtils.test.ts +++ b/packages/renderer/htmlUtils.test.ts @@ -1,4 +1,4 @@ -import htmlUtils, { extractHtmlBody } from './htmlUtils'; +import htmlUtils, { extractHtmlBody, htmlDocIsImageOnly } from './htmlUtils'; describe('htmlUtils', () => { @@ -51,4 +51,39 @@ describe('htmlUtils', () => { } }); + test('should tell if an HTML document is an image only', () => { + const testCases: [string, boolean][] = [ + [ + // This is the kind of HTML that's pasted when copying an image from Chrome + '\n', + true, + ], + [ + '', + false, + ], + [ + '', + true, + ], + [ + '', + false, + ], + [ + '

Some text

', + false, + ], + [ + ' Some text', + false, + ], + ]; + + for (const [input, expected] of testCases) { + const actual = htmlDocIsImageOnly(input); + expect(actual).toBe(expected); + } + }); + }); diff --git a/packages/renderer/htmlUtils.ts b/packages/renderer/htmlUtils.ts index e32b5312f..73cb5feaa 100644 --- a/packages/renderer/htmlUtils.ts +++ b/packages/renderer/htmlUtils.ts @@ -404,4 +404,33 @@ export const extractHtmlBody = (html: string) => { return bodyFound ? output.join('') : html; }; +export const htmlDocIsImageOnly = (html: string) => { + let imageCount = 0; + let nonImageFound = false; + let textFound = false; + + const parser = new htmlparser2.Parser({ + + onopentag: (name: string) => { + if (name === 'img') { + imageCount++; + } else if (['meta'].includes(name)) { + // We allow these tags since they don't print anything + } else { + nonImageFound = true; + } + }, + + ontext: (text: string) => { + if (text.trim()) textFound = true; + }, + + }); + + parser.write(html); + parser.end(); + + return imageCount === 1 && !nonImageFound && !textFound; +}; + export default new HtmlUtils();