Desktop: Fixes #9511: HTML notes are not readable in dark mode

packages/app-desktop/gui/NoteEditor/NoteBody/CodeMirror/v5/CodeMirror.tsx

@@ -621,7 +621,6 @@ function CodeMirror(props: NoteBodyEditorProps, ref: ForwardedRef<NoteBodyEditor
 				resourceInfos: props.resourceInfos,
 				contentMaxWidth: props.contentMaxWidth,
 				mapsToLine: true,
-				// Always using useCustomPdfViewer for now, we can add a new setting for it in future if we need to.
 				useCustomPdfViewer: props.useCustomPdfViewer,
 				noteId: props.noteId,
 				vendorDir: bridge().vendorDir(),

packages/app-desktop/gui/NoteEditor/NoteBody/TinyMCE/TinyMCE.tsx

@@ -14,7 +14,7 @@ import { _, closestSupportedLocale } from '@joplin/lib/locale';
 import useContextMenu from './utils/useContextMenu';
 import { copyHtmlToClipboard } from '../../utils/clipboardUtils';
 import shim from '@joplin/lib/shim';
-import { MarkupToHtml } from '@joplin/renderer';
+import { MarkupLanguage, MarkupToHtml } from '@joplin/renderer';
 import { reg } from '@joplin/lib/registry';
 import BaseItem from '@joplin/lib/models/BaseItem';
 import setupToolbarButtons from './utils/setupToolbarButtons';
@@ -28,6 +28,9 @@ import { TinyMceEditorEvents } from './utils/types';
 import type { Editor } from 'tinymce';
 import { joplinCommandToTinyMceCommands, TinyMceCommand } from './utils/joplinCommandToTinyMceCommands';
 import shouldPasteResources from './utils/shouldPasteResources';
+import lightTheme from '@joplin/lib/themes/light';
+import { Options as NoteStyleOptions } from '@joplin/renderer/noteStyle';
+const md5 = require('md5');
 const { clipboard } = require('electron');
 const supportedLocales = require('./supportedLocales');
 
@@ -86,8 +89,6 @@ interface LastOnChangeEventInfo {
 	contentKey: string;
 }
 
-let loadedCssFiles_: string[] = [];
-let loadedJsFiles_: string[] = [];
 let dispatchDidUpdateIID_: any = null;
 let changeId_ = 1;
 
@@ -354,6 +355,7 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => {
 
 	useEffect(() => {
 		const theme = themeStyle(props.themeId);
+		const backgroundColor = props.whiteBackgroundNoteRendering ? lightTheme.backgroundColor : theme.backgroundColor;
 
 		const element = document.createElement('style');
 		element.setAttribute('id', 'tinyMceStyle');
@@ -503,7 +505,7 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => {
 			}
 
 			.joplin-tinymce .tox .tox-edit-area__iframe {
-				background-color: ${theme.backgroundColor} !important;
+				background-color: ${backgroundColor} !important;
 			}
 
 			.joplin-tinymce .tox .tox-toolbar__primary {
@@ -524,7 +526,7 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => {
 		//
 		// tl;dr: editorReady is used here because the css needs to be re-applied after TinyMCE init
 		// eslint-disable-next-line @seiyab/react-hooks/exhaustive-deps -- Old code before rule was applied
-	}, [editorReady, props.themeId]);
+	}, [editorReady, props.themeId, lightTheme, props.whiteBackgroundNoteRendering]);
 
 	// -----------------------------------------------------------------------------------------
 	// Enable or disable the editor
@@ -542,9 +544,6 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => {
 	useEffect(() => {
 		if (!scriptLoaded) return;
 
-		loadedCssFiles_ = [];
-		loadedJsFiles_ = [];
-
 		const loadEditor = async () => {
 			const language = closestSupportedLocale(props.locale, true, supportedLocales);
 
@@ -739,13 +738,10 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => {
 	// Set the initial content and load the plugin CSS and JS files
 	// -----------------------------------------------------------------------------------------
 
-	const documentCssElements: Record<string, HTMLLinkElement> = {};
+	const loadDocumentAssets = (themeId: number, editor: any, pluginAssets: any[]) => {
-	const documentScriptElements: Record<string, HTMLScriptElement> = {};
+		const theme = themeStyle(themeId);
 
-	const loadDocumentAssets = (editor: any, pluginAssets: any[]) => {
+		let docHead_: HTMLHeadElement = null;
-		const theme = themeStyle(props.themeId);
-
-		let docHead_: any = null;
 
 		function docHead() {
 			if (docHead_) return docHead_;
@@ -768,58 +764,55 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => {
 				.map((a: any) => a.path),
 		);
 
+		const filePathToElementId = (path: string) => {
+			return `jop-tiny-mce-${md5(escape(path))}`;
+		};
+
+		const existingElements = Array.from(docHead().getElementsByClassName('jop-tinymce-css')).concat(Array.from(docHead().getElementsByClassName('jop-tinymce-js')));
+
+		const existingIds: string[] = [];
+		for (const e of existingElements) existingIds.push(e.getAttribute('id'));
+
+		const processedIds: string[] = [];
+
+		for (const cssFile of allCssFiles) {
+			const elementId = filePathToElementId(cssFile);
+			processedIds.push(elementId);
+			if (existingIds.includes(elementId)) continue;
+
+			const style = editor.dom.create('link', {
+				id: elementId,
+				rel: 'stylesheet',
+				type: 'text/css',
+				href: cssFile,
+				class: 'jop-tinymce-css',
+			});
+
+			docHead().appendChild(style);
+		}
+
+		for (const jsFile of allJsFiles) {
+			const elementId = filePathToElementId(jsFile);
+			processedIds.push(elementId);
+			if (existingIds.includes(elementId)) continue;
+
+			const script = editor.dom.create('script', {
+				id: filePathToElementId(jsFile),
+				type: 'text/javascript',
+				class: 'jop-tinymce-js',
+				src: jsFile,
+			});
+
+			docHead().appendChild(script);
+		}
 
 		// Remove all previously loaded files that aren't in the assets this time.
 		// Note: This is important to ensure that we properly change themes.
 		// See https://github.com/laurent22/joplin/issues/8520
-		for (const cssFile of loadedCssFiles_) {
+		for (const existingId of existingIds) {
-			if (!allCssFiles.includes(cssFile)) {
+			if (!processedIds.includes(existingId)) {
-				documentCssElements[cssFile]?.remove();
+				const element = existingElements.find(e => e.getAttribute('id') === existingId);
-				delete documentCssElements[cssFile];
+				if (element) docHead().removeChild(element);
-			}
-		}
-
-		for (const jsFile of loadedJsFiles_) {
-			if (!allJsFiles.includes(jsFile)) {
-				documentScriptElements[jsFile]?.remove();
-				delete documentScriptElements[jsFile];
-			}
-		}
-
-		const newCssFiles = allCssFiles.filter((path: string) => !loadedCssFiles_.includes(path));
-		const newJsFiles = allJsFiles.filter((path: string) => !loadedJsFiles_.includes(path));
-
-		loadedCssFiles_ = allCssFiles;
-		loadedJsFiles_ = allJsFiles;
-
-		// console.info('loadDocumentAssets: files to load', cssFiles, jsFiles);
-
-		if (newCssFiles.length) {
-			for (const cssFile of newCssFiles) {
-				const style = editor.dom.create('link', {
-					rel: 'stylesheet',
-					type: 'text/css',
-					href: cssFile,
-					class: 'jop-tinymce-css',
-				});
-
-				documentCssElements[cssFile] = style;
-				docHead().appendChild(style);
-			}
-		}
-
-		if (newJsFiles.length) {
-			const editorElementId = editor.dom.uniqueId();
-
-			for (const jsFile of newJsFiles) {
-				const script = editor.dom.create('script', {
-					id: editorElementId,
-					type: 'text/javascript',
-					src: jsFile,
-				});
-
-				documentScriptElements[jsFile] = script;
-				docHead().appendChild(script);
 			}
 		}
 	};
@@ -898,7 +891,14 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => {
 				};
 			}
 
-			await loadDocumentAssets(editor, await props.allAssets(props.contentMarkupLanguage, { contentMaxWidthTarget: '.mce-content-body' }));
+			const allAssetsOptions: NoteStyleOptions = {
+				contentMaxWidthTarget: '.mce-content-body',
+				themeId: props.contentMarkupLanguage === MarkupLanguage.Html ? 1 : null,
+				whiteBackgroundNoteRendering: props.whiteBackgroundNoteRendering,
+			};
+
+			const allAssets = await props.allAssets(props.contentMarkupLanguage, allAssetsOptions);
+			await loadDocumentAssets(props.themeId, editor, allAssets);
 
 			dispatchDidUpdate(editor);
 		};
@@ -909,7 +909,7 @@ const TinyMCE = (props: NoteBodyEditorProps, ref: any) => {
 			cancelled = true;
 		};
 		// eslint-disable-next-line @seiyab/react-hooks/exhaustive-deps -- Old code before rule was applied
-	}, [editor, props.markupToHtml, props.allAssets, props.content, props.resourceInfos, props.contentKey]);
+	}, [editor, props.themeId, props.markupToHtml, props.allAssets, props.content, props.resourceInfos, props.contentKey, props.contentMarkupLanguage, props.whiteBackgroundNoteRendering]);
 
 	useEffect(() => {
 		if (!editor) return () => {};

packages/app-desktop/gui/NoteEditor/NoteEditor.tsx

@@ -50,6 +50,7 @@ import CodeMirror6 from './NoteBody/CodeMirror/v6/CodeMirror';
 import CodeMirror5 from './NoteBody/CodeMirror/v5/CodeMirror';
 import { openItemById } from './utils/contextMenu';
 import { namespacedKey } from '@joplin/lib/services/plugins/api/JoplinSettings';
+import { MarkupLanguage } from '@joplin/renderer';
 
 const commands = [
 	require('./commands/showRevisions'),
@@ -165,8 +166,11 @@ function NoteEditor(props: NoteEditorProps) {
 		return Setting.value(namespacedKey(pluginId, key));
 	}, []);
 
+	const whiteBackgroundNoteRendering = formNote.markup_language === MarkupLanguage.Html;
+
 	const markupToHtml = useMarkupToHtml({
 		themeId: props.themeId,
+		whiteBackgroundNoteRendering,
 		customCss: props.customCss,
 		plugins: props.plugins,
 		settingValue,
@@ -178,7 +182,7 @@ function NoteEditor(props: NoteEditorProps) {
 			...options,
 		};
 
-		const theme = themeStyle(props.themeId);
+		const theme = themeStyle(options.themeId ? options.themeId : props.themeId);
 
 		const markupToHtml = markupLanguageUtils.newMarkupToHtml({}, {
 			resourceBaseUrl: `file://${Setting.value('resourceDir')}/`,
@@ -188,6 +192,7 @@ function NoteEditor(props: NoteEditorProps) {
 		return markupToHtml.allAssets(markupLanguage, theme, {
 			contentMaxWidth: props.contentMaxWidth,
 			contentMaxWidthTarget: options.contentMaxWidthTarget,
+			whiteBackgroundNoteRendering: options.whiteBackgroundNoteRendering,
 		});
 	}, [props.themeId, props.customCss, props.contentMaxWidth]);
 
@@ -433,6 +438,7 @@ function NoteEditor(props: NoteEditorProps) {
 		ref: editorRef,
 		contentKey: formNote.id,
 		style: styles.tinyMCE,
+		whiteBackgroundNoteRendering,
 		onChange: onBodyChange,
 		onWillChange: onBodyWillChange,
 		onMessage: onMessage,

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

@@ -9,6 +9,8 @@ import { ProcessResultsRow } from '@joplin/lib/services/searchengine/SearchEngin
 
 export interface AllAssetsOptions {
 	contentMaxWidthTarget?: string;
+	themeId?: number;
+	whiteBackgroundNoteRendering?: boolean;
 }
 
 export interface ToolbarButtonInfos {
@@ -63,6 +65,14 @@ export interface NoteBodyEditorProps {
 	style: any;
 	ref: any;
 	themeId: number;
+
+	// When this is true it means the note must always be rendered using a white
+	// background theme. This applies to the Markdown editor note view, and to
+	// the RTE. It does not apply to other elements such as the toolbar, dialogs
+	// or the CodeMirror editor. This is used to correctly render HTML notes and
+	// avoid cases where black text is rendered over a dark background.
+	whiteBackgroundNoteRendering: boolean;
+
 	content: string;
 	contentKey: string;
 	contentMarkupLanguage: number;

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

@@ -13,6 +13,7 @@ interface HookDependencies {
 	customCss: string;
 	plugins: PluginStates;
 	settingValue: (pluginId: string, key: string)=> any;
+	whiteBackgroundNoteRendering: boolean;
 }
 
 export interface MarkupToHtmlOptions {
@@ -27,13 +28,14 @@ export interface MarkupToHtmlOptions {
 	vendorDir?: string;
 	platformName?: string;
 	allowedFilePrefixes?: string[];
+	whiteBackgroundNoteRendering?: boolean;
 }
 
 export default function useMarkupToHtml(deps: HookDependencies) {
-	const { themeId, customCss, plugins } = deps;
+	const { themeId, customCss, plugins, whiteBackgroundNoteRendering } = deps;
 
 	const markupToHtml = useMemo(() => {
-		return markupLanguageUtils.newMarkupToHtml(deps.plugins, {
+		return markupLanguageUtils.newMarkupToHtml(plugins, {
 			resourceBaseUrl: `file://${Setting.value('resourceDir')}/`,
 			customCss: customCss || '',
 		});
@@ -69,10 +71,11 @@ export default function useMarkupToHtml(deps: HookDependencies) {
 			externalAssetsOnly: true,
 			codeHighlightCacheKey: 'useMarkupToHtml',
 			settingValue: deps.settingValue,
+			whiteBackgroundNoteRendering,
 			...options,
 		});
 
 		return result;
 		// eslint-disable-next-line @seiyab/react-hooks/exhaustive-deps -- Old code before rule was applied
-	}, [themeId, customCss, markupToHtml]);
+	}, [themeId, customCss, markupToHtml, whiteBackgroundNoteRendering]);
 }

packages/renderer/HtmlToHtml.ts

@@ -3,6 +3,8 @@ import linkReplacement from './MdToHtml/linkReplacement';
 import utils, { ItemIdToUrlHandler } from './utils';
 import InMemoryCache from './InMemoryCache';
 import { RenderResult } from './MarkupToHtml';
+import noteStyle, { whiteBackgroundNoteStyle } from './noteStyle';
+import { Options as NoteStyleOptions } from './noteStyle';
 const md5 = require('md5');
 
 // Renderered notes can potentially be quite large (for example
@@ -39,6 +41,7 @@ interface RenderOptions {
 	enableLongPress: boolean;
 	itemIdToUrl?: ItemIdToUrlHandler;
 	allowedFilePrefixes?: string[];
+	whiteBackgroundNoteRendering?: boolean;
 }
 
 // https://github.com/es-shims/String.prototype.trimStart/blob/main/implementation.js
@@ -81,14 +84,22 @@ export default class HtmlToHtml {
 		return this.fsDriver_;
 	}
 
-	public async allAssets(/* theme*/): Promise<any[]> {
+	public async allAssets(theme: any, noteStyleOptions: NoteStyleOptions = null) {
-		return []; // TODO
+		let cssStrings: string[] = [];
+
+		if (noteStyleOptions.whiteBackgroundNoteRendering) {
+			cssStrings = [whiteBackgroundNoteStyle()];
+		} else {
+			cssStrings = [noteStyle(theme, noteStyleOptions).join('\n')];
+		}
+
+		return [await this.fsDriver().cacheCssToFile(cssStrings)];
 	}
 
 	// Note: the "theme" variable is ignored and instead the light theme is
 	// always used for HTML notes.
 	// See: https://github.com/laurent22/joplin/issues/3698
-	public async render(markup: string, _theme: any, options: RenderOptions): Promise<RenderResult> {
+	public async render(markup: string, theme: any, options: RenderOptions): Promise<RenderResult> {
 		options = {
 			splitted: false,
 			postMessageSyntax: 'postMessage',
@@ -152,9 +163,7 @@ export default class HtmlToHtml {
 			};
 		}
 
-		// const lightTheme = themeStyle(Setting.THEME_LIGHT);
+		let cssStrings = options.whiteBackgroundNoteRendering ? [whiteBackgroundNoteStyle()] : noteStyle(theme);
-		// let cssStrings = noteStyle(lightTheme);
-		let cssStrings: string[] = [];
 
 		if (options.splitted) {
 			const splitted = splitHtml(html);

packages/renderer/noteStyle.ts

@@ -10,8 +10,44 @@ function formatCssSize(v: any): string {
 export interface Options {
 	contentMaxWidth?: number;
 	contentMaxWidthTarget?: string;
+	themeId?: number;
+	whiteBackgroundNoteRendering?: boolean;
 }
 
+// If we are viewing an HTML note, it means it comes from the web clipper or
+// emil-to-note, in which case we don't apply any specific theme. We just need
+// to ensure the background is white so that we don't end up with a dark theme
+// and dark font for example. https://github.com/laurent22/joplin/issues/9511
+export const whiteBackgroundNoteStyle = () => {
+	return `
+		body {
+			background-color: #ffffff;
+		}
+
+		/* TinyMCE adds a dashed border for tables that have no borders
+		to make it easier to view where the cells are and edit them.
+		However HTML notes may contain many nested tables used for
+		layout and we also consider that these notes are more or less
+		read-only. Because of this, we remove the dashed lines in this
+		case as it makes the note more readable. */
+
+		.mce-item-table:not([border]),
+		.mce-item-table:not([border]) caption,
+		.mce-item-table:not([border]) td,
+		.mce-item-table:not([border]) th,
+		.mce-item-table[border="0"],
+		.mce-item-table[border="0"] caption,
+		.mce-item-table[border="0"] td,
+		.mce-item-table[border="0"] th,
+		table[style*="border-width: 0px"],
+		table[style*="border-width: 0px"] caption,
+		table[style*="border-width: 0px"] td,
+		table[style*="border-width: 0px"] th {
+			border: none !important;
+		}
+	`;
+};
+
 export default function(theme: any, options: Options = null) {
 	options = {
 		contentMaxWidth: 0,