Desktop: Fixes #4669: Copying code block from Rich Text editor results in two copies of the text

Also improved copying plain text from Rich Text editor - in that case the HTML is converted to Markdown

.eslintignore

@@ -826,6 +826,9 @@ packages/lib/BaseModel.js.map
 packages/lib/BaseSyncTarget.d.ts
 packages/lib/BaseSyncTarget.js
 packages/lib/BaseSyncTarget.js.map
+packages/lib/HtmlToMd.d.ts
+packages/lib/HtmlToMd.js
+packages/lib/HtmlToMd.js.map
 packages/lib/InMemoryCache.d.ts
 packages/lib/InMemoryCache.js
 packages/lib/InMemoryCache.js.map

.gitignore

@@ -813,6 +813,9 @@ packages/lib/BaseModel.js.map
 packages/lib/BaseSyncTarget.d.ts
 packages/lib/BaseSyncTarget.js
 packages/lib/BaseSyncTarget.js.map
+packages/lib/HtmlToMd.d.ts
+packages/lib/HtmlToMd.js
+packages/lib/HtmlToMd.js.map
 packages/lib/InMemoryCache.d.ts
 packages/lib/InMemoryCache.js
 packages/lib/InMemoryCache.js.map

packages/app-cli/tests/HtmlToMd.ts

@@ -1,7 +1,7 @@
 import shim from '@joplin/lib/shim';
 const os = require('os');
 const { filename } = require('@joplin/lib/path-utils');
-const HtmlToMd = require('@joplin/lib/HtmlToMd');
+import HtmlToMd from '@joplin/lib/HtmlToMd';
 
 describe('HtmlToMd', function() {
 

packages/app-desktop/gui/NoteEditor/utils/clipboardUtils.test.ts

@@ -1,13 +1,41 @@
-import { getCopyableContent } from './clipboardUtils';
+import { htmlToClipboardData } from './clipboardUtils';
+
+describe('clipboardUtils', () => {
+
+	test('should convert HTML to the right format', () => {
+		const testCases = [
+			[
+				'<h1>Header</h1>',
+				'# Header',
+				'<h1>Header</h1>',
+			],
+			[
+				'<p>One line</p><p>Two line</p>',
+				'One line\n\nTwo line',
+				'<p>One line</p><p>Two line</p>',
+			],
+			[
+				'<div id="rendered-md"><p>aaa</p><div class="joplin-editable" contenteditable="false"><pre class="joplin-source" data-joplin-language="javascript" data-joplin-source-open="```javascript\n" data-joplin-source-close="\n```">var a = 123;</pre><pre class="hljs"><code><span class="hljs-keyword">var</span> a = <span class="hljs-number">123</span>;</code></pre></div><ul class="joplin-checklist"><li class="checked">A checkbox</li></ul></div>',
+				'aaa\n\n```javascript\nvar a = 123;\n```\n\n- [x] A checkbox',
+				'<div id="rendered-md"><p>aaa</p><div class="joplin-editable" contenteditable="false"><pre class="hljs"><code><span class="hljs-keyword">var</span> a = <span class="hljs-number">123</span>;</code></pre></div><ul class="joplin-checklist"><li class="checked">A checkbox</li></ul></div>',
+			],
+		];
+
+		for (const testCase of testCases) {
+			const [inputHtml, expectedText, expectedHtml] = testCase;
+			const result = htmlToClipboardData(inputHtml);
+			expect(result.html).toBe(expectedHtml);
+			expect(result.text).toBe(expectedText);
+		}
+	});
 
-describe('getCopyableContent', () => {
 	test('should remove parameters from local images', () => {
 		const localImage = 'file:///home/some/path/test.jpg';
 
 		const content = `<div><img src="${localImage}?a=1&b=2"></div>`;
-		const copyableContent = getCopyableContent(content);
+		const copyableContent = htmlToClipboardData(content);
 
-		expect(copyableContent).toEqual(`<div><img src="${localImage}"></div>`);
+		expect(copyableContent.html).toEqual(`<div><img src="${localImage}"></div>`);
 	});
 
 	test('should be able to process mutiple images', () => {
@@ -22,7 +50,7 @@ describe('getCopyableContent', () => {
         <img src="${localImage3}?t=1">
       </div>`;
 
-		const copyableContent = getCopyableContent(content);
+		const copyableContent = htmlToClipboardData(content);
 		const expectedContent = `
       <div>
         <img src="${localImage1}">
@@ -30,7 +58,7 @@ describe('getCopyableContent', () => {
         <img src="${localImage3}">
       </div>`;
 
-		expect(copyableContent).toEqual(expectedContent);
+		expect(copyableContent.html).toEqual(expectedContent);
 	});
 
 	test('should not change parameters for images on the internet', () => {
@@ -40,11 +68,12 @@ describe('getCopyableContent', () => {
 		const content = `
       <div>
         <img src="${image1}">
-        <img src="${image2}?h=12&w=15">
+        <img src="${image2}?h=12&amp;w=15">
       </div>`;
 
-		const copyableContent = getCopyableContent(content);
+		const copyableContent = htmlToClipboardData(content);
 
-		expect(copyableContent).toEqual(content);
+		expect(copyableContent.html).toEqual(content);
 	});
+
 });

packages/app-desktop/gui/NoteEditor/utils/clipboardUtils.ts

@@ -1,7 +1,22 @@
+import HtmlToMd from '@joplin/lib/HtmlToMd';
 import HtmlUtils from '@joplin/lib/htmlUtils';
 const { clipboard } = require('electron');
 
-export function getCopyableContent(htmlContent: string): string {
+interface ClipboardData {
+	text: string;
+	html: string;
+}
+
+let htmlToMd_: HtmlToMd = null;
+
+function htmlToMd(): HtmlToMd {
+	if (!htmlToMd_) {
+		htmlToMd_ = new HtmlToMd();
+	}
+	return htmlToMd_;
+}
+
+function removeImageUrlAttributes(htmlContent: string): string {
 	// We need to remove extra url params from the image URLs while copying
 	// because some offline edtors do not show the image if there is
 	// an extra parameter in it's path.
@@ -20,14 +35,46 @@ export function getCopyableContent(htmlContent: string): string {
 	return HtmlUtils.replaceImageUrls(htmlContent, removeParametersFromUrl);
 }
 
-export function copyHtmlToClipboard(copiedHtml: string): void {
-	const copyableContent = getCopyableContent(copiedHtml);
+// Code blocks are rendered like so:
+//
+//     <div class="joplin-editable" contenteditable="false" data-mce-selected="1">
+//         <pre class="joplin-source" data-joplin-language="javascript" data-joplin-source-open="```javascript" data-joplin-source-close="```">var a = 123;</pre>
+//         <pre class="hljs"><code><span class="hljs-keyword">var</span> a = <span class="hljs-number">123</span>;</code></pre>
+//     </div>
+//
+// One part is hidden and contains the raw source code, while the second part
+// contains the rendered code. When setting the HTML clipboard, we want to get
+// rid of the raw part, otherwise the code will show up twice when pasting.
+//
+// When setting the plain text part of the clipboard, we simply process it with
+// HtmlToMd, which already supports Joplin code blocks (it will keep the raw
+// part, and discard the rendered part), so we don't need to do any additional
+// processing.
+
+function cleanUpCodeBlocks(html: string): string {
+	const element = document.createElement('div');
+	element.innerHTML = html;
+
+	const sourceElements = element.querySelectorAll('.joplin-editable .joplin-source');
+	for (const sourceElement of sourceElements) {
+		sourceElement.remove();
+	}
+
+	return element.innerHTML;
+}
+
+export function htmlToClipboardData(html: string): ClipboardData {
+	const copyableContent = removeImageUrlAttributes(html);
 
 	// In that case we need to set both HTML and Text context, otherwise it
 	// won't be possible to paste the text in, for example, a text editor.
 	// https://github.com/laurent22/joplin/issues/4788
-	clipboard.write({
-		text: HtmlUtils.stripHtml(copyableContent),
-		html: copyableContent,
-	});
+	return {
+		text: htmlToMd().parse(copyableContent),
+		html: cleanUpCodeBlocks(copyableContent),
+	};
+}
+
+export function copyHtmlToClipboard(copiedHtml: string): void {
+	clipboard.write(htmlToClipboardData(copiedHtml));
 }

packages/app-desktop/gui/NoteEditor/utils/index.ts

@@ -1,6 +1,6 @@
 import { FormNote } from './types';
 
-const HtmlToMd = require('@joplin/lib/HtmlToMd');
+import HtmlToMd from '@joplin/lib/HtmlToMd';
 import Note from '@joplin/lib/models/Note';
 const { MarkupToHtml } = require('@joplin/renderer');
 

packages/app-desktop/jest.config.js

@@ -135,7 +135,7 @@ module.exports = {
 	// snapshotSerializers: [],
 
 	// The test environment that will be used for testing
-	testEnvironment: 'node',
+	testEnvironment: 'jsdom',
 
 	// Options that will be passed to the testEnvironment
 	// testEnvironmentOptions: {},

packages/lib/HtmlToMd.ts

@@ -1,9 +1,16 @@
 const TurndownService = require('@joplin/turndown');
 const turndownPluginGfm = require('@joplin/turndown-plugin-gfm').gfm;
-const markdownUtils = require('./markdownUtils').default;
+import markdownUtils from './markdownUtils';
 
-class HtmlToMd {
-	parse(html, options = {}) {
+export interface ParseOptions {
+	anchorNames?: string[];
+	preserveImageTagsWithSize?: boolean;
+	baseUrl?: string;
+}
+
+export default class HtmlToMd {
+
+	public parse(html: string, options: ParseOptions = {}) {
 		const turndown = new TurndownService({
 			headingStyle: 'atx',
 			anchorNames: options.anchorNames ? options.anchorNames.map(n => n.trim().toLowerCase()) : [],
@@ -21,6 +28,5 @@ class HtmlToMd {
 		if (options.baseUrl) md = markdownUtils.prependBaseUrl(md, options.baseUrl);
 		return md;
 	}
-}
 
-module.exports = HtmlToMd;
+}

packages/lib/services/rest/routes/notes.ts

@@ -20,7 +20,7 @@ import htmlUtils from '../../../htmlUtils';
 import markupLanguageUtils from '../../../markupLanguageUtils';
 const mimeUtils = require('../../../mime-utils.js').mime;
 const md5 = require('md5');
-const HtmlToMd = require('../../../HtmlToMd');
+import HtmlToMd from '../../../HtmlToMd';
 const urlUtils = require('../../../urlUtils.js');
 const ArrayUtils = require('../../../ArrayUtils.js');
 const { netUtils } = require('../../../net-utils');

packages/tools/fetchPatreonPosts.js

@@ -4,7 +4,7 @@
 const fetch = require('node-fetch');
 const fs = require('fs-extra');
 const { patreonOauthToken } = require('./tool-utils');
-const HtmlToMd = require('@joplin/lib/HtmlToMd');
+const HtmlToMd = require('@joplin/lib/HtmlToMd').default;
 const { dirname, filename, basename } = require('@joplin/lib/path-utils');
 const markdownUtils = require('@joplin/lib/markdownUtils').default;
 const mimeUtils = require('@joplin/lib/mime-utils.js').mime;