Desktop: Fixes #11020: Fix clicking on most non-media resource links opens them inline (#11022)

packages/app-cli/tests/MdToHtml.ts

@@ -1,4 +1,4 @@
-import MdToHtml from '@joplin/renderer/MdToHtml';
+import MdToHtml, { LinkRenderingType } from '@joplin/renderer/MdToHtml';
 const { filename } = require('@joplin/lib/path-utils');
 import { setupDatabaseAndSynchronizer, switchClient } from '@joplin/lib/testing/test-utils';
 import shim from '@joplin/lib/shim';
@@ -218,6 +218,9 @@ describe('MdToHtml', () => {
 		const mdToHtmlLinkifyOn = newTestMdToHtml({
 			pluginOptions: {
 				linkify: { enabled: true },
+				link_open: {
+					linkRenderingType: LinkRenderingType.HrefHandler,
+				},
 			},
 		});
 
@@ -227,29 +230,52 @@ describe('MdToHtml', () => {
 			},
 		});
 
+		const renderOptions = {
+			bodyOnly: true,
+			plainResourceRendering: true,
+			linkRenderingType: LinkRenderingType.HrefHandler,
+		};
+
 		for (const testCase of testCases) {
 			const [input, expectedLinkifyOff, expectedLinkifyOn] = testCase;
 
 			{
-				const actual = await mdToHtmlLinkifyOn.render(input, null, {
-					bodyOnly: true,
-					plainResourceRendering: true,
-				});
+				const actual = await mdToHtmlLinkifyOn.render(input, null, renderOptions);
 
 				expect(actual.html).toBe(expectedLinkifyOn);
 			}
 
 			{
-				const actual = await mdToHtmlLinkifyOff.render(input, null, {
-					bodyOnly: true,
-					plainResourceRendering: true,
-				});
+				const actual = await mdToHtmlLinkifyOff.render(input, null, renderOptions);
 
 				expect(actual.html).toBe(expectedLinkifyOff);
 			}
 		}
 	}));
 
+	it.each([
+		'[test](http://example.com/)',
+		'[test](mailto:test@example.com)',
+	])('should add onclick handlers to links when linkRenderingType is JavaScriptHandler (%j)', async (markdown) => {
+		const mdToHtml = newTestMdToHtml();
+
+		const renderWithoutOnClickOptions = {
+			bodyOnly: true,
+			linkRenderingType: LinkRenderingType.HrefHandler,
+		};
+		expect(
+			(await mdToHtml.render(markdown, undefined, renderWithoutOnClickOptions)).html,
+		).not.toContain('onclick');
+
+		const renderWithOnClickOptions = {
+			bodyOnly: true,
+			linkRenderingType: LinkRenderingType.JavaScriptHandler,
+		};
+		expect(
+			(await mdToHtml.render(markdown, undefined, renderWithOnClickOptions)).html,
+		).toMatch(/<a data-from-md .*onclick=['"].*['"].*>/);
+	});
+
 	it('should return attributes of line numbers', (async () => {
 		const mdToHtml = newTestMdToHtml();
 

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

@@ -1,3 +1,4 @@
+import { LinkRenderingType } from '@joplin/renderer/MdToHtml';
 import { MarkupToHtmlOptions } from './types';
 
 export default (override: MarkupToHtmlOptions = null): MarkupToHtmlOptions => {
@@ -7,7 +8,7 @@ export default (override: MarkupToHtmlOptions = null): MarkupToHtmlOptions => {
 				checkboxRenderingType: 2,
 			},
 			link_open: {
-				linkRenderingType: 2,
+				linkRenderingType: LinkRenderingType.HrefHandler,
 			},
 		},
 		replaceResourceInternalToExternalLinks: true,

packages/app-desktop/integration-tests/markdownEditor.spec.ts

@@ -77,6 +77,24 @@ test.describe('markdownEditor', () => {
 		await mainScreen.noteEditor.toggleEditorsButton.click();
 
 		await expectToBeRendered();
+
+		// Clicking on the PDF link should attempt to open it in a viewer
+		await expect(pdfLink).toBeVisible();
+
+		const nextOpenFilePromise = electronApp.evaluate(({ shell }) => {
+			return new Promise<string>(resolve => {
+				const openPath = async (url: string) => {
+					resolve(url);
+					return '';
+				};
+				shell.openPath = openPath;
+			});
+		});
+		await pdfLink.click();
+		expect(await nextOpenFilePromise).toMatch(/\.pdf$/);
+
+		// Should not have rendered something else in the viewer frame
+		await expectToBeRendered();
 	});
 
 	test('preview pane should render video attachments', async ({ mainWindow, electronApp }) => {

packages/app-mobile/commands/util/showResource.ts

@@ -14,7 +14,7 @@ const showResource = async (item: ResourceEntity) => {
 	if (shim.mobilePlatform() === 'web') {
 		const url = URL.createObjectURL(await shim.fsDriver().fileAtPath(resourcePath));
 		const w = window.open(url, '_blank');
-		w.addEventListener('close', () => {
+		w?.addEventListener('close', () => {
 			URL.revokeObjectURL(url);
 		}, { once: true });
 	} else {

packages/lib/services/interop/InteropService_Exporter_Html.ts

@@ -14,6 +14,7 @@ const { themeStyle } = require('../../theme');
 const { escapeHtml } = require('../../string-utils.js');
 import { assetsToHeaders } from '@joplin/renderer';
 import getPluginSettingValue from '../plugins/utils/getPluginSettingValue';
+import { LinkRenderingType } from '@joplin/renderer/MdToHtml';
 
 export default class InteropService_Exporter_Html extends InteropService_Exporter_Base {
 
@@ -115,8 +116,14 @@ export default class InteropService_Exporter_Html extends InteropService_Exporte
 			const bodyMd = await this.processNoteResources_(item);
 			const result = await this.markupToHtml_.render(item.markup_language, bodyMd, this.style_, {
 				resources: this.resources_,
-				plainResourceRendering: true,
 				settingValue: getPluginSettingValue,
+
+				plainResourceRendering: true,
+				plugins: {
+					link_open: {
+						linkRenderingType: LinkRenderingType.HrefHandler,
+					},
+				},
 			});
 			const noteContent = [];
 			if (item.title) noteContent.push(`<div class="exported-note-title">${escapeHtml(item.title)}</div>`);

packages/renderer/MdToHtml.ts

@@ -146,6 +146,14 @@ interface PluginContext {
 	};
 }
 
+export enum LinkRenderingType {
+	// linkRenderingType = 1 is the regular rendering and clicking on it is handled via embedded JS (in onclick attribute)
+	JavaScriptHandler = 1,
+
+	// linkRenderingType = 2 gives a plain link with no JS. Caller needs to handle clicking on the link.
+	HrefHandler = 2,
+}
+
 export interface RuleOptions {
 	context: PluginContext;
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
@@ -174,9 +182,7 @@ export interface RuleOptions {
 	enableLongPress?: boolean;
 
 	// Use by `link_open` rule.
-	// linkRenderingType = 1 is the regular rendering and clicking on it is handled via embedded JS (in onclick attribute)
-	// linkRenderingType = 2 gives a plain link with no JS. Caller needs to handle clicking on the link.
-	linkRenderingType?: number;
+	linkRenderingType?: LinkRenderingType;
 
 	// A list of MIME types for which an edit button appears on tap/hover.
 	// Used by the image editor in the mobile app.

packages/renderer/MdToHtml/linkReplacement.ts

@@ -1,3 +1,4 @@
+import { LinkRenderingType } from '../MdToHtml';
 import { ItemIdToUrlHandler, OptionsResourceModel } from '../types';
 import * as utils from '../utils';
 import createEventHandlingAttrs from './createEventHandlingAttrs';
@@ -11,7 +12,7 @@ export interface Options {
 	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 	resources?: any;
 	ResourceModel?: OptionsResourceModel;
-	linkRenderingType?: number;
+	linkRenderingType?: LinkRenderingType;
 	plainResourceRendering?: boolean;
 	postMessageSyntax?: string;
 	enableLongPress?: boolean;
@@ -27,16 +28,14 @@ export interface LinkReplacementResult {
 }
 
 export default function(href: string, options: Options = null): LinkReplacementResult {
-	options = {
-		title: '',
-		resources: {},
-		ResourceModel: null,
-		linkRenderingType: 1,
-		plainResourceRendering: false,
-		postMessageSyntax: 'postMessage',
-		enableLongPress: false,
-		...options,
-	};
+	options = { ...options };
+	options.title ??= '';
+	options.resources ??= {};
+	options.ResourceModel ??= null;
+	options.linkRenderingType ??= LinkRenderingType.JavaScriptHandler;
+	options.plainResourceRendering ??= false;
+	options.postMessageSyntax ??= 'postMessage';
+	options.enableLongPress ??= false;
 
 	const resourceHrefInfo = urlUtils.parseResourceUrl(href);
 	const isResourceUrl = options.resources && !!resourceHrefInfo;
@@ -129,12 +128,15 @@ export default function(href: string, options: Options = null): LinkReplacementR
 
 	if (addedHrefAttr) {
 		// Done -- the HREF has already bee set.
-	} else if (options.plainResourceRendering || options.linkRenderingType === 2) {
+	} else if (options.plainResourceRendering || options.linkRenderingType === LinkRenderingType.HrefHandler) {
 		icon = '';
 		attrHtml.push(`href='${htmlentities(href)}'`);
 	} else {
 		attrHtml.push(`href='${htmlentities(hrefAttr)}'`);
-		if (js) attrHtml.push(js);
+	}
+
+	if (js && options.linkRenderingType === LinkRenderingType.JavaScriptHandler) {
+		attrHtml.push(js);
 	}
 
 	return {

packages/renderer/MdToHtml/rules/link_close.ts

@@ -1,7 +1,7 @@
 // This rule is used to add a media player for certain resource types below
 // the link.
 
-import { RuleOptions } from '../../MdToHtml';
+import { LinkRenderingType, RuleOptions } from '../../MdToHtml';
 import renderMedia, { Options as RenderMediaOptions } from '../renderMedia';
 
 
@@ -23,7 +23,7 @@ function plugin(markdownIt: any, ruleOptions: RuleOptions) {
 		const defaultOutput = defaultRender(tokens, idx, options, env, self);
 		const link = ruleOptions.context.currentLinks.pop();
 
-		if (!link || ruleOptions.linkRenderingType === 2 || ruleOptions.plainResourceRendering) return defaultOutput;
+		if (!link || ruleOptions.linkRenderingType === LinkRenderingType.HrefHandler || ruleOptions.plainResourceRendering) return defaultOutput;
 
 		return [defaultOutput, renderMedia(link, ruleOptions as RenderMediaOptions, linkIndexes)].join('');
 	};