Desktop: Fixes #9597: Fix image rotation not preserved when resizing an attached image (#9824)

.eslintignore

@@ -415,6 +415,7 @@ packages/app-desktop/integration-tests/util/activateMainMenuItem.js
 packages/app-desktop/integration-tests/util/createStartupArgs.js
 packages/app-desktop/integration-tests/util/firstNonDevToolsWindow.js
 packages/app-desktop/integration-tests/util/setFilePickerResponse.js
+packages/app-desktop/integration-tests/util/setMessageBoxResponse.js
 packages/app-desktop/integration-tests/util/test.js
 packages/app-desktop/playwright.config.js
 packages/app-desktop/plugins/GotoAnything.js

.gitignore

@@ -395,6 +395,7 @@ packages/app-desktop/integration-tests/util/activateMainMenuItem.js
 packages/app-desktop/integration-tests/util/createStartupArgs.js
 packages/app-desktop/integration-tests/util/firstNonDevToolsWindow.js
 packages/app-desktop/integration-tests/util/setFilePickerResponse.js
+packages/app-desktop/integration-tests/util/setMessageBoxResponse.js
 packages/app-desktop/integration-tests/util/test.js
 packages/app-desktop/playwright.config.js
 packages/app-desktop/plugins/GotoAnything.js

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

@@ -7,6 +7,7 @@ import { join } from 'path';
 import createStartupArgs from './util/createStartupArgs';
 import firstNonDevToolsWindow from './util/firstNonDevToolsWindow';
 import setFilePickerResponse from './util/setFilePickerResponse';
+import setMessageBoxResponse from './util/setMessageBoxResponse';
 
 
 test.describe('main', () => {
@@ -44,7 +45,7 @@ test.describe('main', () => {
 		const editor = await mainScreen.createNewNote('🚧 Test 🚧');
 
 		const testCommitId = 'bf59b2';
-		await editor.codeMirrorEditor.click();
+		await editor.focusCodeMirrorEditor();
 		const noteText = [
 			'```mermaid',
 			'gitGraph',
@@ -94,7 +95,7 @@ test.describe('main', () => {
 		const editor = mainScreen.noteEditor;
 
 		// Set the note's content
-		await editor.codeMirrorEditor.click();
+		await editor.focusCodeMirrorEditor();
 
 		// Attach this file to the note (create a resource ID)
 		await setFilePickerResponse(electronApp, [__filename]);
@@ -132,6 +133,61 @@ test.describe('main', () => {
 		expect(finalCodeMirrorContent).toContain(`:/${resourceId}`);
 	});
 
+	test('should correctly resize large images', async ({ electronApp, mainWindow }) => {
+		const mainScreen = new MainScreen(mainWindow);
+		await mainScreen.createNewNote('Image resize test (part 1)');
+		const editor = mainScreen.noteEditor;
+
+		await editor.focusCodeMirrorEditor();
+
+		const filename = 'large-jpg-image-with-exif-rotation.jpg';
+		await setFilePickerResponse(electronApp, [join(__dirname, 'resources', filename)]);
+
+		// Should be possible to cancel attaching for large images
+		await setMessageBoxResponse(electronApp, /^Cancel/i);
+		await editor.attachFileButton.click();
+		await expect(editor.codeMirrorEditor).toHaveText('', { useInnerText: true });
+
+		// Clicking "No" should not resize
+		await setMessageBoxResponse(electronApp, /^No/i);
+		await editor.attachFileButton.click();
+
+		const getImageSize = async () => {
+			// Wait for it to render
+			const viewerFrame = editor.getNoteViewerIframe();
+			const renderedImage = viewerFrame.getByAltText(filename);
+			await renderedImage.waitFor();
+
+			// We load a copy of the image to avoid returning an overriden width set with
+			//    .width = some_number
+			return await renderedImage.evaluate((originalImage: HTMLImageElement) => {
+				return new Promise<[number, number]>(resolve => {
+					const testImage = new Image();
+					testImage.onload = () => {
+						resolve([testImage.width, testImage.height]);
+					};
+					testImage.src = originalImage.src;
+				});
+			});
+		};
+		const fullSize = await getImageSize();
+
+		// To make it easier to find the image (one image per note), we switch to a new, empty note.
+		await mainScreen.createNewNote('Image resize test (part 2)');
+		await editor.focusCodeMirrorEditor();
+
+		// Clicking "Yes" should resize
+		await setMessageBoxResponse(electronApp, /^Yes/i);
+		await editor.attachFileButton.click();
+
+		const resizedSize = await getImageSize();
+		expect(resizedSize[0]).toBeLessThan(fullSize[0]);
+		expect(resizedSize[1]).toBeLessThan(fullSize[1]);
+
+		// Should keep aspect ratio (regression test for #9597)
+		expect(fullSize[0] / resizedSize[0]).toBeCloseTo(fullSize[1] / resizedSize[1]);
+	});
+
 	test('should be possible to remove sort order buttons in settings', async ({ electronApp, mainWindow }) => {
 		const mainScreen = new MainScreen(mainWindow);
 		await mainScreen.waitFor();

packages/app-desktop/integration-tests/models/NoteEditorScreen.ts

@@ -32,6 +32,10 @@ export default class NoteEditorPage {
 		return this.richTextEditor.frameLocator(':scope');
 	}
 
+	public focusCodeMirrorEditor() {
+		return this.codeMirrorEditor.click();
+	}
+
 	public async waitFor() {
 		await this.noteTitleInput.waitFor();
 		await this.toggleEditorsButton.waitFor();

packages/app-desktop/integration-tests/util/setMessageBoxResponse.ts

@@ -0,0 +1,37 @@
+import { ElectronApplication } from '@playwright/test';
+import { BrowserWindow, MessageBoxOptions } from 'electron';
+
+const setMessageBoxResponse = (electronApp: ElectronApplication, responseMatch: RegExp) => {
+	return electronApp.evaluate(async ({ dialog }, responseMatch) => {
+		type DialogArgsType = [ BrowserWindow, MessageBoxOptions ]|[MessageBoxOptions];
+
+		const getMatchingButton = (dialogArgs: DialogArgsType) => {
+			const matchingButton = (options: MessageBoxOptions) => {
+				const buttons = options.buttons ?? ['OK'];
+
+				for (let i = 0; i < buttons.length; i++) {
+					if (buttons[i].match(responseMatch)) {
+						return i;
+					}
+				}
+
+				throw new Error('No matching button found');
+			};
+
+			if (dialogArgs.length === 1) {
+				return matchingButton(dialogArgs[0]);
+			} else {
+				return matchingButton(dialogArgs[1]);
+			}
+		};
+		dialog.showMessageBoxSync = (...args: DialogArgsType) => getMatchingButton(args);
+		dialog.showMessageBox = async (...args: DialogArgsType) => ({
+			response: getMatchingButton(args),
+			checkboxChecked: false,
+		// We're mocking, so include "as any" to prevent this from breaking when we upgrade
+		// Electron.
+		} as any);
+	}, responseMatch);
+};
+
+export default setMessageBoxResponse;

packages/lib/shim-init-node.ts

@@ -195,33 +195,53 @@ function shimInit(options: ShimInitOptions = null) {
 		const maxDim = Resource.IMAGE_MAX_DIMENSION;
 
 		if (shim.isElectron()) {
-			// For Electron
+			// For Electron/renderer process
-			const nativeImage = require('electron').nativeImage;
+			// Note that we avoid nativeImage because it loses rotation metadata.
-			const image = nativeImage.createFromPath(filePath);
+			// See https://github.com/electron/electron/issues/41189
-			if (image.isEmpty()) throw new Error(`Image is invalid or does not exist: ${filePath}`);
+			//
-			const size = image.getSize();
+			// After the upstream bug has been fixed, this should be reverted to using
+			// nativeImage (see commit 99e8818ba093a931b1a0cbccbee0b94a4fd37a54 for the
+			// original code).
+
+			const image = new Image();
+			image.src = filePath;
+			await new Promise<void>((resolve, reject) => {
+				image.onload = () => resolve();
+				image.onerror = () => reject(`Image at ${filePath} failed to load.`);
+				image.onabort = () => reject(`Loading stopped for image at ${filePath}.`);
+			});
+			if (!image.complete || (image.width === 0 && image.height === 0)) {
+				throw new Error(`Image is invalid or does not exist: ${filePath}`);
+			}
 
 			const saveOriginalImage = async () => {
 				await shim.fsDriver().copy(filePath, targetPath);
 				return true;
 			};
 			const saveResizedImage = async () => {
-				const options: any = {};
+				let newWidth, newHeight;
-				if (size.width > size.height) {
+				if (image.width > image.height) {
-					options.width = maxDim;
+					newWidth = maxDim;
+					newHeight = image.height * maxDim / image.width;
 				} else {
-					options.height = maxDim;
+					newWidth = image.width * maxDim / image.height;
+					newHeight = maxDim;
 				}
-				const resizedImage = image.resize(options);
+
-				await shim.writeImageToFile(resizedImage, mime, targetPath);
+				const canvas = new OffscreenCanvas(newWidth, newHeight);
+				const ctx = canvas.getContext('2d');
+				ctx.drawImage(image, 0, 0, newWidth, newHeight);
+
+				const resizedImage = await canvas.convertToBlob({ type: mime });
+				await fs.writeFile(targetPath, Buffer.from(await resizedImage.arrayBuffer()));
 				return true;
 			};
 
-			const canResize = size.width > maxDim || size.height > maxDim;
+			const canResize = image.width > maxDim || image.height > maxDim;
 			if (canResize) {
 				if (resizeLargeImages === 'alwaysAsk') {
 					const Yes = 0, No = 1, Cancel = 2;
-					const userAnswer = shim.showMessageBox(`${_('You are about to attach a large image (%dx%d pixels). Would you like to resize it down to %d pixels before attaching it?', size.width, size.height, maxDim)}\n\n${_('(You may disable this prompt in the options)')}`, {
+					const userAnswer = shim.showMessageBox(`${_('You are about to attach a large image (%dx%d pixels). Would you like to resize it down to %d pixels before attaching it?', image.width, image.height, maxDim)}\n\n${_('(You may disable this prompt in the options)')}`, {
 						buttons: [_('Yes'), _('No'), _('Cancel')],
 					});
 					if (userAnswer === Yes) return await saveResizedImage();