diff --git a/.eslintignore b/.eslintignore index 894d5d04eb..9ba90863ee 100644 --- a/.eslintignore +++ b/.eslintignore @@ -455,6 +455,7 @@ packages/app-desktop/integration-tests/models/MainScreen.js packages/app-desktop/integration-tests/models/NoteEditorScreen.js packages/app-desktop/integration-tests/models/SettingsScreen.js packages/app-desktop/integration-tests/models/Sidebar.js +packages/app-desktop/integration-tests/richTextEditor.spec.js packages/app-desktop/integration-tests/sidebar.spec.js packages/app-desktop/integration-tests/simpleBackup.spec.js packages/app-desktop/integration-tests/util/activateMainMenuItem.js @@ -463,6 +464,7 @@ 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/integration-tests/util/waitForNextOpenPath.js packages/app-desktop/playwright.config.js packages/app-desktop/plugins/GotoAnything.js packages/app-desktop/services/bridge.js @@ -1221,6 +1223,7 @@ packages/lib/themes/solarizedLight.js packages/lib/themes/type.js packages/lib/time.js packages/lib/types.js +packages/lib/urlUtils.js packages/lib/utils/ActionLogger.test.js packages/lib/utils/ActionLogger.js packages/lib/utils/credentialFiles.js diff --git a/.gitignore b/.gitignore index f97ad70905..74d14eb3da 100644 --- a/.gitignore +++ b/.gitignore @@ -434,6 +434,7 @@ packages/app-desktop/integration-tests/models/MainScreen.js packages/app-desktop/integration-tests/models/NoteEditorScreen.js packages/app-desktop/integration-tests/models/SettingsScreen.js packages/app-desktop/integration-tests/models/Sidebar.js +packages/app-desktop/integration-tests/richTextEditor.spec.js packages/app-desktop/integration-tests/sidebar.spec.js packages/app-desktop/integration-tests/simpleBackup.spec.js packages/app-desktop/integration-tests/util/activateMainMenuItem.js @@ -442,6 +443,7 @@ 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/integration-tests/util/waitForNextOpenPath.js packages/app-desktop/playwright.config.js packages/app-desktop/plugins/GotoAnything.js packages/app-desktop/services/bridge.js @@ -1200,6 +1202,7 @@ packages/lib/themes/solarizedLight.js packages/lib/themes/type.js packages/lib/time.js packages/lib/types.js +packages/lib/urlUtils.js packages/lib/utils/ActionLogger.test.js packages/lib/utils/ActionLogger.js packages/lib/utils/credentialFiles.js diff --git a/packages/app-desktop/gui/MainScreen/commands/openItem.ts b/packages/app-desktop/gui/MainScreen/commands/openItem.ts index a9411c8bce..f4712c9ebf 100644 --- a/packages/app-desktop/gui/MainScreen/commands/openItem.ts +++ b/packages/app-desktop/gui/MainScreen/commands/openItem.ts @@ -3,9 +3,10 @@ import shim from '@joplin/lib/shim'; import { _ } from '@joplin/lib/locale'; import bridge from '../../../services/bridge'; import { openItemById } from '../../NoteEditor/utils/contextMenu'; -const { parseResourceUrl, urlProtocol } = require('@joplin/lib/urlUtils'); +import { fileUrlToResourceUrl, parseResourceUrl, urlProtocol } from '@joplin/lib/urlUtils'; import { fileUriToPath } from '@joplin/utils/url'; -const { urlDecode } = require('@joplin/lib/string-utils'); +import { urlDecode } from '@joplin/lib/string-utils'; +import Setting from '@joplin/lib/models/Setting'; export const declaration: CommandDeclaration = { name: 'openItem', @@ -16,6 +17,11 @@ export const runtime = (): CommandRuntime => { execute: async (context: CommandContext, link: string) => { if (!link) throw new Error('Link cannot be empty'); + const fromFileUrl = fileUrlToResourceUrl(link, Setting.value('resourceDir')); + if (fromFileUrl) { + link = fromFileUrl; + } + if (link.startsWith('joplin://') || link.startsWith(':/')) { const parsedUrl = parseResourceUrl(link); if (parsedUrl) { diff --git a/packages/app-desktop/integration-tests/main.spec.ts b/packages/app-desktop/integration-tests/main.spec.ts index 0bab7bc705..31ac996c13 100644 --- a/packages/app-desktop/integration-tests/main.spec.ts +++ b/packages/app-desktop/integration-tests/main.spec.ts @@ -89,50 +89,6 @@ test.describe('main', () => { await expect(viewerFrame.locator('.joplin-editable > .katex').first()).toBeAttached(); }); - test('HTML links should be preserved when editing a note in the WYSIWYG editor', async ({ electronApp, mainWindow }) => { - const mainScreen = new MainScreen(mainWindow); - await mainScreen.createNewNote('Testing!'); - const editor = mainScreen.noteEditor; - - // Set the note's content - await editor.focusCodeMirrorEditor(); - - // Attach this file to the note (create a resource ID) - await setFilePickerResponse(electronApp, [__filename]); - await editor.attachFileButton.click(); - - // Wait to render - const viewerFrame = editor.getNoteViewerIframe(); - await viewerFrame.locator('a[data-from-md]').waitFor(); - - // Should have an attached resource - const codeMirrorContent = await editor.codeMirrorEditor.innerText(); - - const resourceUrlExpression = /\[.*\]\(:\/(\w+)\)/; - expect(codeMirrorContent).toMatch(resourceUrlExpression); - const resourceId = codeMirrorContent.match(resourceUrlExpression)[1]; - - // Create a new note with just an HTML link - await mainScreen.createNewNote('Another test'); - await editor.codeMirrorEditor.click(); - await mainWindow.keyboard.type(`HTML Link`); - - // Switch to the RTE - await editor.toggleEditorsButton.click(); - await editor.richTextEditor.waitFor(); - - // Edit the note to cause the original content to update - await editor.getTinyMCEFrameLocator().locator('a').click(); - await mainWindow.keyboard.type('Test...'); - - await editor.toggleEditorsButton.click(); - await editor.codeMirrorEditor.waitFor(); - - // Note should still contain the resource ID and note title - const finalCodeMirrorContent = await editor.codeMirrorEditor.innerText(); - 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)'); diff --git a/packages/app-desktop/integration-tests/richTextEditor.spec.ts b/packages/app-desktop/integration-tests/richTextEditor.spec.ts new file mode 100644 index 0000000000..29f920e916 --- /dev/null +++ b/packages/app-desktop/integration-tests/richTextEditor.spec.ts @@ -0,0 +1,86 @@ +import { test, expect } from './util/test'; +import MainScreen from './models/MainScreen'; +import setFilePickerResponse from './util/setFilePickerResponse'; +import waitForNextOpenPath from './util/waitForNextOpenPath'; +import { basename } from 'path'; + +test.describe('richTextEditor', () => { + test('HTML links should be preserved when editing a note', async ({ electronApp, mainWindow }) => { + const mainScreen = new MainScreen(mainWindow); + await mainScreen.createNewNote('Testing!'); + const editor = mainScreen.noteEditor; + + // Set the note's content + await editor.focusCodeMirrorEditor(); + + // Attach this file to the note (create a resource ID) + await setFilePickerResponse(electronApp, [__filename]); + await editor.attachFileButton.click(); + + // Wait to render + const viewerFrame = editor.getNoteViewerIframe(); + await viewerFrame.locator('a[data-from-md]').waitFor(); + + // Should have an attached resource + const codeMirrorContent = await editor.codeMirrorEditor.innerText(); + + const resourceUrlExpression = /\[.*\]\(:\/(\w+)\)/; + expect(codeMirrorContent).toMatch(resourceUrlExpression); + const resourceId = codeMirrorContent.match(resourceUrlExpression)[1]; + + // Create a new note with just an HTML link + await mainScreen.createNewNote('Another test'); + await editor.codeMirrorEditor.click(); + await mainWindow.keyboard.type(`HTML Link`); + + // Switch to the RTE + await editor.toggleEditorsButton.click(); + await editor.richTextEditor.waitFor(); + + // Edit the note to cause the original content to update + await editor.getTinyMCEFrameLocator().locator('a').click(); + await mainWindow.keyboard.type('Test...'); + + await editor.toggleEditorsButton.click(); + await editor.codeMirrorEditor.waitFor(); + + // Note should still contain the resource ID and note title + const finalCodeMirrorContent = await editor.codeMirrorEditor.innerText(); + expect(finalCodeMirrorContent).toContain(`:/${resourceId}`); + }); + + test('should watch resources for changes when opened with ctrl+click', async ({ electronApp, mainWindow }) => { + const mainScreen = new MainScreen(mainWindow); + await mainScreen.createNewNote('Testing!'); + const editor = mainScreen.noteEditor; + + // Set the note's content + await editor.focusCodeMirrorEditor(); + + // Attach this file to the note (create a resource ID) + const pathToAttach = __filename; + await setFilePickerResponse(electronApp, [pathToAttach]); + await editor.attachFileButton.click(); + + // Switch to the RTE + await editor.toggleEditorsButton.click(); + await editor.richTextEditor.waitFor(); + + await editor.richTextEditor.click(); + + // Click on the attached file URL + const openPathResult = waitForNextOpenPath(electronApp); + const targetLink = editor.getTinyMCEFrameLocator().getByRole('link', { name: basename(pathToAttach) }); + if (process.platform === 'darwin') { + await targetLink.click({ modifiers: ['Meta'] }); + } else { + await targetLink.click({ modifiers: ['Control'] }); + } + + // Should watch the file + await mainWindow.getByText(/^The following attachments are being watched for changes/i).waitFor(); + expect(await openPathResult).toContain(basename(pathToAttach)); + }); + +}); + diff --git a/packages/app-desktop/integration-tests/util/waitForNextOpenPath.ts b/packages/app-desktop/integration-tests/util/waitForNextOpenPath.ts new file mode 100644 index 0000000000..9d347512b9 --- /dev/null +++ b/packages/app-desktop/integration-tests/util/waitForNextOpenPath.ts @@ -0,0 +1,16 @@ +import { ElectronApplication } from '@playwright/test'; + +const waitForNextOpenPath = (electronApp: ElectronApplication) => { + return electronApp.evaluate(async ({ shell }) => { + return new Promise(resolve => { + const originalOpenPath = shell.openPath; + shell.openPath = async (path: string) => { + shell.openPath = originalOpenPath; + resolve(path); + return ''; + }; + }); + }); +}; + +export default waitForNextOpenPath; diff --git a/packages/app-mobile/commands/openItem.ts b/packages/app-mobile/commands/openItem.ts index bd961661cb..a5304ae32b 100644 --- a/packages/app-mobile/commands/openItem.ts +++ b/packages/app-mobile/commands/openItem.ts @@ -1,7 +1,7 @@ import { CommandRuntime, CommandDeclaration, CommandContext } from '@joplin/lib/services/CommandService'; import shim from '@joplin/lib/shim'; import { _ } from '@joplin/lib/locale'; -const { parseResourceUrl, urlProtocol } = require('@joplin/lib/urlUtils'); +import { parseResourceUrl, urlProtocol } from '@joplin/lib/urlUtils'; import Logger from '@joplin/utils/Logger'; import goToNote from './util/goToNote'; diff --git a/packages/app-mobile/components/screens/Note.tsx b/packages/app-mobile/components/screens/Note.tsx index 28f8e1aeb7..66b29f720a 100644 --- a/packages/app-mobile/components/screens/Note.tsx +++ b/packages/app-mobile/components/screens/Note.tsx @@ -63,7 +63,7 @@ import pickDocument from '../../utils/pickDocument'; import debounce from '../../utils/debounce'; import { focus } from '@joplin/lib/utils/focusHandler'; import CommandService from '@joplin/lib/services/CommandService'; -const urlUtils = require('@joplin/lib/urlUtils'); +import * as urlUtils from '@joplin/lib/urlUtils'; // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied const emptyArray: any[] = []; diff --git a/packages/lib/urlUtils.test.js b/packages/lib/urlUtils.test.js index c23936830d..9baa2d0ec6 100644 --- a/packages/lib/urlUtils.test.js +++ b/packages/lib/urlUtils.test.js @@ -52,6 +52,26 @@ describe('urlUtils', () => { } })); + it.each([ + [ + 'file:///home/builder/.config/joplindev-desktop/profile-owmhbsat/resources/4a12670298dd46abbb140ffc8a10b583.md', + '/home/builder/.config/joplindev-desktop/profile-owmhbsat/resources', + { itemId: '4a12670298dd46abbb140ffc8a10b583', hash: '' }, + ], + [ + 'file:///home/builder/.config/joplindev-desktop/profile-owmhbsat/resources/4a12670298dd46abbb140ffc8a10b583.md5#foo', + '/home/builder/.config/joplindev-desktop/profile-owmhbsat/resources', + { itemId: '4a12670298dd46abbb140ffc8a10b583', hash: 'foo' }, + ], + [ + 'file:///home/builder/.config/joplindev-desktop/profile-owmhbsat/resources/4a12670298dd46abbb140ffc8a10b583.png?t=12345', + '/home/builder/.config/joplindev-desktop/profile-owmhbsat/resources', + { itemId: '4a12670298dd46abbb140ffc8a10b583', hash: '' }, + ], + ])('should detect resource file URLs', (url, resourceDir, expected) => { + expect(urlUtils.parseResourceUrl(urlUtils.fileUrlToResourceUrl(url, resourceDir))).toMatchObject(expected); + }); + it('should extract resource URLs', (async () => { const testCases = [ ['Bla [](:/11111111111111111111111111111111) bla [](:/22222222222222222222222222222222) bla', ['11111111111111111111111111111111', '22222222222222222222222222222222']], diff --git a/packages/lib/urlUtils.js b/packages/lib/urlUtils.ts similarity index 60% rename from packages/lib/urlUtils.js rename to packages/lib/urlUtils.ts index d509716aed..6f4165c75e 100644 --- a/packages/lib/urlUtils.js +++ b/packages/lib/urlUtils.ts @@ -1,53 +1,52 @@ -const { rtrimSlashes } = require('./path-utils'); -const { urlDecode } = require('./string-utils'); +import { rtrimSlashes, toFileProtocolPath } from './path-utils'; +import { urlDecode } from './string-utils'; -const urlUtils = {}; - -urlUtils.hash = function(url) { +export const hash = (url: string) => { const s = url.split('#'); if (s.length <= 1) return ''; return s[s.length - 1]; }; -urlUtils.urlWithoutPath = function(url) { +export const urlWithoutPath = (url: string) => { const parsed = require('url').parse(url, true); return `${parsed.protocol}//${parsed.host}`; }; -urlUtils.urlProtocol = function(url) { +export const urlProtocol = (url: string) => { if (!url) return ''; const parsed = require('url').parse(url, true); return parsed.protocol; }; -urlUtils.prependBaseUrl = function(url, baseUrl) { +export const prependBaseUrl = (url: string, baseUrl: string) => { baseUrl = rtrimSlashes(baseUrl).trim(); // All the code below assumes that the baseUrl does not end up with a slash url = url.trim(); if (!url) url = ''; if (!baseUrl) return url; if (url.indexOf('#') === 0) return url; // Don't prepend if it's a local anchor - if (urlUtils.urlProtocol(url)) return url; // Don't prepend the base URL if the URL already has a scheme + if (urlProtocol(url)) return url; // Don't prepend the base URL if the URL already has a scheme if (url.length >= 2 && url.indexOf('//') === 0) { // If it starts with // it's a protcol-relative URL - return urlUtils.urlProtocol(baseUrl) + url; + return urlProtocol(baseUrl) + url; } else if (url && url[0] === '/') { // If it starts with a slash, it's an absolute URL so it should be relative to the domain (and not to the full baseUrl) - return urlUtils.urlWithoutPath(baseUrl) + url; + return urlWithoutPath(baseUrl) + url; } else { return baseUrl + (url ? `/${url}` : ''); } }; + const resourceRegex = /^(joplin:\/\/|:\/)([0-9a-zA-Z]{32})(|#[^\s]*)(|\s".*?")$/; -urlUtils.isResourceUrl = function(url) { +export const isResourceUrl = (url: string) => { return !!url.match(resourceRegex); }; -urlUtils.parseResourceUrl = function(url) { - if (!urlUtils.isResourceUrl(url)) return null; +export const parseResourceUrl = (url: string) => { + if (!isResourceUrl(url)) return null; const match = url.match(resourceRegex); @@ -65,13 +64,35 @@ urlUtils.parseResourceUrl = function(url) { }; }; -urlUtils.extractResourceUrls = function(text) { +export const fileUrlToResourceUrl = (fileUrl: string, resourceDir: string) => { + let resourceDirUrl = toFileProtocolPath(resourceDir); + if (!resourceDirUrl.endsWith('/')) { + resourceDirUrl += '/'; + } + + if (fileUrl.startsWith(resourceDirUrl)) { + let result = fileUrl.substring(resourceDirUrl.length); + // Remove the timestamp parameter, keep the hash. + result = result.replace(/\?t=\d+(#.*)?$/, '$1'); + // Remove the file extension. + result = result.replace(/\.[a-z0-9]+(#.*)?$/, '$1'); + result = `joplin://${result}`; + + if (isResourceUrl(result)) { + return result; + } + } + + return null; +}; + +export const extractResourceUrls = (text: string) => { const markdownLinksRE = /\]\((.*?)\)/g; const output = []; let result = null; while ((result = markdownLinksRE.exec(text)) !== null) { - const resourceUrlInfo = urlUtils.parseResourceUrl(result[1]); + const resourceUrlInfo = parseResourceUrl(result[1]); if (resourceUrlInfo) output.push(resourceUrlInfo); } @@ -91,7 +112,7 @@ urlUtils.extractResourceUrls = function(text) { return output; }; -urlUtils.objectToQueryString = function(query) { +export const objectToQueryString = (query: Record) => { if (!query) return ''; let queryString = ''; @@ -105,4 +126,3 @@ urlUtils.objectToQueryString = function(query) { return queryString; }; -module.exports = urlUtils;