All: Resolves #3839: Change Markdown rendering to align with CommonMark spec

CliClient/tests/MdToHtml.ts

@@ -1,8 +1,8 @@
+import MdToHtml from 'lib/joplin-renderer/MdToHtml';
 const os = require('os');
 const { filename } = require('lib/path-utils');
 const { asyncTest, setupDatabaseAndSynchronizer, switchClient } = require('test-utils.js');
 const shim = require('lib/shim').default;
-const MdToHtml = require('lib/joplin-renderer/MdToHtml').default;
 const { themeStyle } = require('lib/theme');
 
 function newTestMdToHtml(options:any = null) {
@@ -120,23 +120,54 @@ describe('MdToHtml', function() {
 	it('should return the rendered body only', asyncTest(async () => {
 		const mdToHtml = newTestMdToHtml();
 
-		// In this case, the HTML contains only the rendered markdown,
-		// with no wrapper and no style.
-		// The style is instead in the cssStrings property.
-		const result = await mdToHtml.render('just **testing**', null, { bodyOnly: true });
-		expect(result.cssStrings.length).toBeGreaterThan(0);
-		expect(result.html.trim()).toBe('just <strong>testing</strong>');
+		// In this case, the HTML contains only the rendered markdown, with
+		// no wrapper and no style. The style is instead in the cssStrings
+		// property.
+		{
+			const result = await mdToHtml.render('just **testing**', null, { bodyOnly: true });
+			expect(result.cssStrings.length).toBeGreaterThan(0);
+			expect(result.html.trim()).toBe('just <strong>testing</strong>');
+		}
+
+		// But it should not remove the wrapping <p> tags if there's more
+		// than one line
+		{
+			const result = await mdToHtml.render('one\n\ntwo', null, { bodyOnly: true });
+			expect(result.html.trim()).toBe('<p>one</p>\n<p>two</p>');
+		}
 	}));
 
 	it('should split HTML and CSS', asyncTest(async () => {
 		const mdToHtml = newTestMdToHtml();
 
-		// It is similar to the bodyOnly option, excepts that
-		// the rendered Markdown is wrapped in a DIV
+		// It is similar to the bodyOnly option, excepts that the rendered
+		// Markdown is wrapped in a DIV
 		const result = await mdToHtml.render('just **testing**', null, { splitted: true });
 		expect(result.cssStrings.length).toBeGreaterThan(0);
 		expect(result.html.trim()).toBe('<div id="rendered-md"><p>just <strong>testing</strong></p>\n</div>');
 	}));
 
+	it('should render links correctly', asyncTest(async () => {
+		const mdToHtml = newTestMdToHtml();
+
+		const testCases = [
+			// None of these should result in a link
+			['https://example.com', 'https://example.com'],
+			['file://C:\\AUTOEXEC.BAT', 'file://C:\\AUTOEXEC.BAT'],
+			['example.com', 'example.com'],
+			['oo.ps', 'oo.ps'],
+			['test@example.com', 'test@example.com'],
+
+			// Those should be converted to links
+			['<https://example.com>', '<a data-from-md title=\'https://example.com\' href=\'https://example.com\'>https://example.com</a>'],
+			['[ok](https://example.com)', '<a data-from-md title=\'https://example.com\' href=\'https://example.com\'>ok</a>'],
+		];
+
+		for (const testCase of testCases) {
+			const [input, expected] = testCase;
+			const actual = await mdToHtml.render(input, null, { bodyOnly: true, plainResourceRendering: true });
+			expect(actual.html).toBe(expected);
+		}
+	}));
 
 });

ReactNativeClient/lib/joplin-renderer/MdToHtml.ts

@@ -318,9 +318,14 @@ export default class MdToHtml {
 		return output;
 	}
 
+	// The string we are looking for is: <p></p>\n
 	private removeMarkdownItWrappingParagraph_(html:string) {
-		// <p></p>\n
 		if (html.length < 8) return html;
+
+		// If there are multiple <p> tags, we keep them because it's multiple lines
+		// and removing the first and last tag will result in invalid HTML.
+		if ((html.match(/<\/p>/g) || []).length > 1) return html;
+
 		if (html.substr(0, 3) !== '<p>') return html;
 		if (html.slice(-5) !== '</p>\n') return html;
 		return html.substring(3, html.length - 5);
@@ -376,7 +381,7 @@ export default class MdToHtml {
 		const markdownIt = new MarkdownIt({
 			breaks: !this.pluginEnabled('softbreaks'),
 			typographer: this.pluginEnabled('typographer'),
-			linkify: true,
+			// linkify: true,
 			html: true,
 			highlight: (str:string, lang:string) => {
 				let outputCodeHtml = '';

ReactNativeClient/lib/joplin-renderer/MdToHtml/rules/link_open.ts

@@ -72,11 +72,24 @@ function plugin(markdownIt:any, ruleOptions:RuleOptions) {
 
 		if (hrefAttr.indexOf('#') === 0 && href.indexOf('#') === 0) js = ''; // If it's an internal anchor, don't add any JS since the webview is going to handle navigating to the right place
 
+		const attrHtml = [];
+		attrHtml.push('data-from-md');
+		if (resourceIdAttr) attrHtml.push(resourceIdAttr);
+		if (title) attrHtml.push(`title='${htmlentities(title)}'`);
+		if (mime) attrHtml.push(`type='${htmlentities(mime)}'`);
+
 		if (ruleOptions.plainResourceRendering || ruleOptions.linkRenderingType === 2) {
-			return `<a data-from-md ${resourceIdAttr} title='${htmlentities(title)}' href='${htmlentities(href)}' type='${htmlentities(mime)}'>`;
+			icon = '';
+			attrHtml.push(`href='${htmlentities(href)}'`);
+
+			// return `<a data-from-md ${resourceIdAttr} title='${htmlentities(title)}' href='${htmlentities(href)}' type='${htmlentities(mime)}'>`;
 		} else {
-			return `<a data-from-md ${resourceIdAttr} title='${htmlentities(title)}' href='${hrefAttr}' ${js} type='${htmlentities(mime)}'>${icon}`;
+			attrHtml.push(`href='${hrefAttr}'`);
+			if (js) attrHtml.push(js);
+			// return `<a data-from-md ${resourceIdAttr} title='${htmlentities(title)}' href='${hrefAttr}' ${js} type='${htmlentities(mime)}'>${icon}`;
 		}
+
+		return `<a ${attrHtml.join(' ')}>${icon}`;
 	};
 }
 

ReactNativeClient/lib/models/Setting.ts

@@ -565,11 +565,11 @@ class Setting extends BaseModel {
 			},
 
 			// Deprecated - use markdown.plugin.*
-			'markdown.softbreaks': { value: false, type: SettingItemType.Bool, public: false, appTypes: ['mobile', 'desktop'] },
+			'markdown.softbreaks': { value: true, type: SettingItemType.Bool, public: false, appTypes: ['mobile', 'desktop'] },
 			'markdown.typographer': { value: false, type: SettingItemType.Bool, public: false, appTypes: ['mobile', 'desktop'] },
 			// Deprecated
 
-			'markdown.plugin.softbreaks': { value: false, type: SettingItemType.Bool, section: 'markdownPlugins', public: true, appTypes: ['mobile', 'desktop'], label: () => `${_('Enable soft breaks')}${wysiwygYes}` },
+			'markdown.plugin.softbreaks': { value: true, type: SettingItemType.Bool, section: 'markdownPlugins', public: true, appTypes: ['mobile', 'desktop'], label: () => `${_('Enable soft breaks')}${wysiwygYes}` },
 			'markdown.plugin.typographer': { value: false, type: SettingItemType.Bool, section: 'markdownPlugins', public: true, appTypes: ['mobile', 'desktop'], label: () => `${_('Enable typographer support')}${wysiwygYes}` },
 			'markdown.plugin.katex': { value: true, type: SettingItemType.Bool, section: 'markdownPlugins', public: true, appTypes: ['mobile', 'desktop'], label: () => `${_('Enable math expressions')}${wysiwygYes}` },
 			'markdown.plugin.fountain': { value: false, type: SettingItemType.Bool, section: 'markdownPlugins', public: true, appTypes: ['mobile', 'desktop'], label: () => `${_('Enable Fountain syntax support')}${wysiwygYes}` },

tsconfig.dev.json

@@ -12,6 +12,6 @@
 		"**/node_modules",
 		"ElectronClient/dist/**/*",
 		"CliClient/tests/support/**/*",
-		"CliClient/tests-build/support/**/*",
+		"CliClient/tests-build/**/*",
 	],
 }

tsconfig.json

@@ -31,6 +31,6 @@
 		"**/node_modules",
 		"ElectronClient/dist/**/*",
 		"CliClient/tests/support/**/*",
-		"CliClient/tests-build/support/**/*",
+		"CliClient/tests-build/**/*",
 	],
 }