diff --git a/packages/desktop/src/renderer/src/components/sideBar/toc.vue b/packages/desktop/src/renderer/src/components/sideBar/toc.vue index bb6726155d..28601706cf 100644 --- a/packages/desktop/src/renderer/src/components/sideBar/toc.vue +++ b/packages/desktop/src/renderer/src/components/sideBar/toc.vue @@ -26,6 +26,7 @@ import { computed, ref } from 'vue' import { useEditorStore } from '@/store/editor' import { usePreferencesStore } from '@/store/preferences' +import { deriveKeyedToc, type KeyedTocNode } from '@/util/tocKeys' import bus from '../../bus' import { storeToRefs } from 'pinia' import { useI18n } from 'vue-i18n' @@ -44,33 +45,9 @@ const defaultProps = { const { toc } = storeToRefs(editorStore) const { wordWrapInToc } = storeToRefs(preferencesStore) -interface KeyedTocNode { - key: string - label: unknown - slug: unknown - children: KeyedTocNode[] -} - -// The TOC nodes carry no stable id, so el-tree (without a node-key) discarded -// the user's expand/collapse state on every content edit (#3028). Derive a -// stable key per node — its slug, deduplicated in document order so duplicate -// headings stay unique — and let el-tree preserve state by that key. -const keyedToc = computed(() => { - const seen = new Map() - const assign = (nodes: Array>): KeyedTocNode[] => - nodes.map((node) => { - const base = typeof node.slug === 'string' && node.slug ? node.slug : 'heading' - const count = seen.get(base) ?? 0 - seen.set(base, count + 1) - return { - key: count === 0 ? base : `${base}-${count}`, - label: node.label, - slug: node.slug, - children: assign((node.children as Array>) ?? []) - } - }) - return assign(toc.value as unknown as Array>) -}) +// Stable per-node key so el-tree preserves the user's expand/collapse state +// across content edits (#3028) and tab switches (#3791). See deriveKeyedToc. +const keyedToc = computed(() => deriveKeyedToc(toc.value)) // Track which headings the user collapsed, by stable key (#3028). Headings are // expanded by default; a collapse is remembered here. diff --git a/packages/desktop/src/renderer/src/store/editor.ts b/packages/desktop/src/renderer/src/store/editor.ts index ec7b4a2cf2..d51a026f01 100644 --- a/packages/desktop/src/renderer/src/store/editor.ts +++ b/packages/desktop/src/renderer/src/store/editor.ts @@ -500,8 +500,18 @@ export const useEditorStore = defineStore('editor', { } }, + // Flush any edit still queued in the engine's rAF batch into the active + // tab's `currentFile` before its markdown is read to persist — otherwise an + // edit made in the same frame as the read is silently dropped from the + // written file (#3803), the way tab switching already guards (#2938). Safe + // no-op when nothing is pending. + flushActiveEditor(): void { + bus.emit('flush-active-editor') + }, + FILE_SAVE(): void { if (!this.currentFile) return + this.flushActiveEditor() const projectStore = useProjectStore() const { id, filename, pathname, markdown } = this.currentFile const options = getOptionsFromState(this.currentFile) @@ -531,6 +541,7 @@ export const useEditorStore = defineStore('editor', { FILE_SAVE_AS(): void { if (!this.currentFile) return + this.flushActiveEditor() const projectStore = useProjectStore() const { id, filename, pathname, markdown } = this.currentFile const options = getOptionsFromState(this.currentFile) @@ -705,6 +716,7 @@ export const useEditorStore = defineStore('editor', { MOVE_FILE_TO(): void { if (!this.currentFile) return + this.flushActiveEditor() const projectStore = useProjectStore() const { id, filename, pathname, markdown } = this.currentFile const options = getOptionsFromState(this.currentFile) @@ -747,6 +759,7 @@ export const useEditorStore = defineStore('editor', { RESPONSE_FOR_RENAME(): void { if (!this.currentFile) return + this.flushActiveEditor() const projectStore = useProjectStore() const { id, filename, pathname, markdown } = this.currentFile const options = getOptionsFromState(this.currentFile) @@ -812,7 +825,7 @@ export const useEditorStore = defineStore('editor', { // Must run while `currentFile` still points at the outgoing tab, so its // flushed edit is attributed to that tab and not lost on switch (#2938). if (oldCurrentFile) { - bus.emit('flush-active-editor') + this.flushActiveEditor() } window.DIRNAME = pathname ? window.path.dirname(pathname) : '' this.currentFile = currentFile diff --git a/packages/desktop/src/renderer/src/util/listToTree.ts b/packages/desktop/src/renderer/src/util/listToTree.ts index debf24e1ec..3e316df85c 100644 --- a/packages/desktop/src/renderer/src/util/listToTree.ts +++ b/packages/desktop/src/renderer/src/util/listToTree.ts @@ -15,6 +15,7 @@ export interface TreeNode { lvl: number | null label: unknown slug: unknown + githubSlug: unknown children: Array> } @@ -23,6 +24,7 @@ class Node implements TreeNode { lvl: number | null label: unknown slug: unknown + githubSlug: unknown children: Array> constructor(item: { @@ -30,12 +32,16 @@ class Node implements TreeNode { lvl: number | null content?: unknown slug?: unknown + githubSlug?: unknown }) { - const { parent, lvl, content, slug } = item + const { parent, lvl, content, slug, githubSlug } = item this.parent = parent this.lvl = lvl this.label = content this.slug = slug + // Carried through for the TOC: a content-derived id that, unlike `slug` + // (a per-render object id), survives a document reload / tab switch (#3791). + this.githubSlug = githubSlug this.children = [] } diff --git a/packages/desktop/src/renderer/src/util/tocKeys.ts b/packages/desktop/src/renderer/src/util/tocKeys.ts new file mode 100644 index 0000000000..3f7830eea9 --- /dev/null +++ b/packages/desktop/src/renderer/src/util/tocKeys.ts @@ -0,0 +1,37 @@ +export interface KeyedTocNode { + key: string + label: unknown + slug: unknown + children: KeyedTocNode[] +} + +interface TocLike { + label?: unknown + slug?: unknown + githubSlug?: unknown + children?: TocLike[] +} + +// Give each TOC node a stable key so el-tree can preserve the user's +// expand/collapse state. The key is the heading's content-derived `githubSlug`, +// deduplicated in document order so duplicate headings stay distinct. This is +// stable across content edits (#3028) AND across a document reload / tab switch +// (#3791) — unlike the per-render object id `slug`, which every switch rebuilds. +// `slug` is still carried for the click-to-scroll anchor payload. +export function deriveKeyedToc(nodes: TocLike[]): KeyedTocNode[] { + const seen = new Map() + const assign = (list: TocLike[]): KeyedTocNode[] => + list.map((node) => { + const base = + typeof node.githubSlug === 'string' && node.githubSlug ? node.githubSlug : 'heading' + const count = seen.get(base) ?? 0 + seen.set(base, count + 1) + return { + key: count === 0 ? base : `${base}-${count}`, + label: node.label, + slug: node.slug, + children: assign(node.children ?? []) + } + }) + return assign(nodes) +} diff --git a/packages/desktop/test/unit/specs/flush-before-save.spec.ts b/packages/desktop/test/unit/specs/flush-before-save.spec.ts new file mode 100644 index 0000000000..c9f0d71a4f --- /dev/null +++ b/packages/desktop/test/unit/specs/flush-before-save.spec.ts @@ -0,0 +1,169 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' +import { createPinia, setActivePinia } from 'pinia' + +// `@/store/editor` reads `window.path` at module load and `window.electron` +// at runtime; stub those surfaces before the hoisted imports run. +vi.hoisted(() => { + const w = globalThis as unknown as { + window?: { + path?: { sep: string; dirname: (p: string) => string } + electron?: { + clipboard: { writeText: (s: string) => void } + ipcRenderer: { send: (...a: unknown[]) => void; on: (...a: unknown[]) => void } + } + } + } + w.window ??= {} + w.window.path ??= { sep: '/', dirname: (p: string) => p } + w.window.electron ??= { + clipboard: { writeText: () => {} }, + ipcRenderer: { send: () => {}, on: () => {} } + } +}) + +vi.mock('@/services/notification', () => ({ + default: { notify: vi.fn(), name: 'notify' } +})) + +import { useEditorStore } from '@/store/editor' +import bus from '@/bus' + +// #3803: the store snapshots `currentFile.markdown` (refreshed only on the +// engine's deferred rAF `json-change`) to send to the main process. A keystroke +// typed in the same frame as Cmd+S was therefore dropped from the saved file. +// The save/move/rename paths now emit `flush-active-editor` first, which the +// editor synchronously commits into `currentFile.markdown` before it is read. +// +// The bug lives at the `const { …, markdown } = this.currentFile` READ, which +// sits between the flush and the send — so an emit-order assertion (flush < send) +// alone would still pass if a regression moved the flush past the read. These +// tests instead wire a real `flush-active-editor` listener that commits the +// pending keystroke (mirroring editor.vue → `editor.flush()` → `json-change` → +// LISTEN_FOR_CONTENT_CHANGE) and assert the SENT PAYLOAD carries it: a flush +// moved after the read would send the stale snapshot and fail here. + +const STALE = 'hello' // what the pre-flush snapshot holds +const FLUSHED = 'hello world!' // the last keystroke the editor commits on flush +const MARKDOWN_ARG = 4 // send(channel, id, filename, pathname, markdown, …) + +function seedCurrentFile( + store: ReturnType, + overrides: Record = {} +) { + store.currentFile = { + id: 'tab-1', + filename: 'note.md', + pathname: '/tmp/note.md', + markdown: STALE, + isSaved: false, + encoding: { encoding: 'utf8', isBom: false }, + lineEnding: 'lf', + adjustLineEndingOnSave: false, + trimTrailingNewline: 2, + ...overrides + // eslint-disable-next-line @typescript-eslint/no-explicit-any + } as any +} + +// Mirror editor.vue's listener: commit the pending keystroke into the store on +// flush. Returns a detach fn (the bus is a module singleton — listeners leak +// across tests otherwise). +function onFlushCommit(store: ReturnType) { + const handler = () => { + if (store.currentFile) store.currentFile.markdown = FLUSHED + } + bus.on('flush-active-editor', handler) + return () => bus.off('flush-active-editor', handler) +} + +// Global invocation order of a given emitted event, located by event name (not +// array position) so an unrelated earlier emit can't mask a moved flush. +function emitOrderOf(emitSpy: ReturnType, event: string): number | undefined { + const i = emitSpy.mock.calls.findIndex((c: unknown[]) => c[0] === event) + return i === -1 ? undefined : emitSpy.mock.invocationCallOrder[i] +} + +describe('editor store — flush pending edits before saving (#3803)', () => { + let detach: (() => void) | undefined + + beforeEach(() => { + setActivePinia(createPinia()) + vi.clearAllMocks() + }) + + afterEach(() => { + detach?.() + detach = undefined + }) + + it('FILE_SAVE sends the flushed markdown, not the stale pre-flush snapshot', () => { + const store = useEditorStore() + seedCurrentFile(store) + detach = onFlushCommit(store) + const sendSpy = vi.spyOn(window.electron.ipcRenderer, 'send') + + store.FILE_SAVE() + + const call = sendSpy.mock.calls.find((c) => c[0] === 'mt::response-file-save') + expect(call).toBeDefined() + expect(call?.[MARKDOWN_ARG]).toBe(FLUSHED) + }) + + it('FILE_SAVE_AS sends the flushed markdown, not the stale pre-flush snapshot', () => { + const store = useEditorStore() + seedCurrentFile(store) + detach = onFlushCommit(store) + const sendSpy = vi.spyOn(window.electron.ipcRenderer, 'send') + + store.FILE_SAVE_AS() + + const call = sendSpy.mock.calls.find((c) => c[0] === 'mt::response-file-save-as') + expect(call).toBeDefined() + expect(call?.[MARKDOWN_ARG]).toBe(FLUSHED) + }) + + // MOVE_FILE_TO / RESPONSE_FOR_RENAME only transmit `markdown` in their untitled + // (no-pathname) branch, which reuses `mt::response-file-save` — that is where + // the flush actually matters, so assert the payload there too. + it('MOVE_FILE_TO (untitled) sends the flushed markdown', () => { + const store = useEditorStore() + seedCurrentFile(store, { pathname: '' }) + detach = onFlushCommit(store) + const sendSpy = vi.spyOn(window.electron.ipcRenderer, 'send') + + store.MOVE_FILE_TO() + + const call = sendSpy.mock.calls.find((c) => c[0] === 'mt::response-file-save') + expect(call).toBeDefined() + expect(call?.[MARKDOWN_ARG]).toBe(FLUSHED) + }) + + it('RESPONSE_FOR_RENAME (untitled) sends the flushed markdown', () => { + const store = useEditorStore() + seedCurrentFile(store, { pathname: '' }) + detach = onFlushCommit(store) + const sendSpy = vi.spyOn(window.electron.ipcRenderer, 'send') + + store.RESPONSE_FOR_RENAME() + + const call = sendSpy.mock.calls.find((c) => c[0] === 'mt::response-file-save') + expect(call).toBeDefined() + expect(call?.[MARKDOWN_ARG]).toBe(FLUSHED) + }) + + // The existing-file rename branch emits 'rename' (no markdown payload); guard + // that the flush still precedes it so it can't be silently dropped later. + it('RESPONSE_FOR_RENAME (existing file) flushes before emitting rename', () => { + const store = useEditorStore() + seedCurrentFile(store, { pathname: '/tmp/note.md' }) + const emitSpy = vi.spyOn(bus, 'emit') + + store.RESPONSE_FOR_RENAME() + + const flushOrder = emitOrderOf(emitSpy, 'flush-active-editor') + const renameOrder = emitOrderOf(emitSpy, 'rename') + expect(flushOrder).toBeDefined() + expect(renameOrder).toBeDefined() + expect(flushOrder as number).toBeLessThan(renameOrder as number) + }) +}) diff --git a/packages/desktop/test/unit/specs/toc-keys.spec.ts b/packages/desktop/test/unit/specs/toc-keys.spec.ts new file mode 100644 index 0000000000..e357775e00 --- /dev/null +++ b/packages/desktop/test/unit/specs/toc-keys.spec.ts @@ -0,0 +1,58 @@ +import { describe, it, expect } from 'vitest' +import { deriveKeyedToc } from '@/util/tocKeys' + +// #3791: the TOC's collapse state is remembered by these keys. They must be +// derived from the content-based `githubSlug` so they survive a tab switch +// (which rebuilds every heading block with a fresh object-identity `slug`), +// not only same-tab edits (#3028). + +const flatKeys = (nodes: ReturnType): string[] => + nodes.flatMap((n) => [n.key, ...flatKeys(n.children)]) + +describe('deriveKeyedToc', () => { + it('keys nodes by githubSlug, deduplicated in document order', () => { + const keyed = deriveKeyedToc([ + { label: 'Intro', slug: 'uid-1', githubSlug: 'intro', children: [] }, + { label: 'Intro', slug: 'uid-2', githubSlug: 'intro', children: [] } + ]) + expect(keyed.map((n) => n.key)).toEqual(['intro', 'intro-1']) + }) + + it('produces identical keys when the same document is rebuilt with fresh slugs (tab switch)', () => { + // Same headings/content, different per-render object-identity slugs — this + // is exactly what Editor.setContent does when you switch back to a tab. + const doc = (p: string) => [ + { + label: 'A', + slug: `${p}-1`, + githubSlug: 'a', + children: [{ label: 'B', slug: `${p}-2`, githubSlug: 'b', children: [] }] + }, + { label: 'C', slug: `${p}-3`, githubSlug: 'c', children: [] } + ] + + const before = deriveKeyedToc(doc('old')) + const after = deriveKeyedToc(doc('new')) + + // Keys are stable across the rebuild, so collapse state keyed by them + // survives the switch (they would differ if keyed by `slug`). + expect(flatKeys(after)).toEqual(flatKeys(before)) + expect(flatKeys(after)).toEqual(['a', 'b', 'c']) + }) + + it('falls back to a stable placeholder for unsluggable headings', () => { + const keyed = deriveKeyedToc([ + { label: '🎉', slug: 'uid-1', githubSlug: '', children: [] }, + { label: '🎊', slug: 'uid-2', githubSlug: '', children: [] } + ]) + expect(keyed.map((n) => n.key)).toEqual(['heading', 'heading-1']) + }) + + it('preserves slug (for the scroll-to-heading payload) while keying by githubSlug', () => { + const keyed = deriveKeyedToc([ + { label: 'A', slug: 'uid-42', githubSlug: 'a', children: [] } + ]) + expect(keyed[0].slug).toBe('uid-42') + expect(keyed[0].key).toBe('a') + }) +}) diff --git a/packages/muya/src/assets/styles/inlineSyntax.css b/packages/muya/src/assets/styles/inlineSyntax.css index c1d60ad71c..825e2e5617 100644 --- a/packages/muya/src/assets/styles/inlineSyntax.css +++ b/packages/muya/src/assets/styles/inlineSyntax.css @@ -393,9 +393,9 @@ blockquote .mu-hide.mu-math > .mu-math-render { background: transparent; } -.mu-inline-image a.mu-image-icon-success, -.mu-inline-image a.mu-image-icon-fail, -.mu-inline-image a.mu-image-icon-close { +.mu-inline-image .mu-image-icon-success, +.mu-inline-image .mu-image-icon-fail, +.mu-inline-image .mu-image-icon-close { position: absolute; top: 15px; @@ -404,8 +404,8 @@ blockquote .mu-hide.mu-math > .mu-math-render { height: 20px; } -.mu-inline-image a.mu-image-icon-success, -.mu-inline-image a.mu-image-icon-fail { +.mu-inline-image .mu-image-icon-success, +.mu-inline-image .mu-image-icon-fail { left: 15px; } @@ -478,17 +478,17 @@ blockquote .mu-hide.mu-math > .mu-math-render { content: attr(fail-text); } -.mu-inline-image.mu-image-loading a.mu-image-icon-success, -.mu-inline-image.mu-empty-image a.mu-image-icon-success { +.mu-inline-image.mu-image-loading .mu-image-icon-success, +.mu-inline-image.mu-empty-image .mu-image-icon-success { display: block; } -.mu-inline-image.mu-image-fail a.mu-image-icon-fail { +.mu-inline-image.mu-image-fail .mu-image-icon-fail { display: block; } -.mu-inline-image.mu-empty-image:hover a.mu-image-icon-close, -.mu-inline-image.mu-image-fail:hover a.mu-image-icon-close { +.mu-inline-image.mu-empty-image:hover .mu-image-icon-close, +.mu-inline-image.mu-image-fail:hover .mu-image-icon-close { right: 15px; z-index: 1; diff --git a/packages/muya/src/editor/linkMouseEvents.ts b/packages/muya/src/editor/linkMouseEvents.ts index 34f9b02b87..f47f36cff8 100644 --- a/packages/muya/src/editor/linkMouseEvents.ts +++ b/packages/muya/src/editor/linkMouseEvents.ts @@ -36,7 +36,7 @@ import { getLinkInfo } from '../utils/getLinkInfo'; // `mu-raw-html` is added to every inline HTML tag (``, ``, // ``, ``, `` …), so we can't match it loosely — narrow // each entry by the actual tag the renderer emits. -const LINK_SELECTOR = [ +export const LINK_SELECTOR = [ `span.${CLASS_NAMES.MU_LINK}`, `a.${CLASS_NAMES.MU_REFERENCE_LINK}`, `a.${CLASS_NAMES.MU_RAW_HTML}`, diff --git a/packages/muya/src/inlineRenderer/__tests__/referenceLinkImageAnchor.spec.ts b/packages/muya/src/inlineRenderer/__tests__/referenceLinkImageAnchor.spec.ts new file mode 100644 index 0000000000..4829aa07e2 --- /dev/null +++ b/packages/muya/src/inlineRenderer/__tests__/referenceLinkImageAnchor.spec.ts @@ -0,0 +1,51 @@ +import type { ReferenceLinkToken, Token } from '../types'; +import { describe, expect, it } from 'vitest'; +import { tokenizer } from '../lexer'; + +// #4865: a full reference link whose text is an image — `[![alt](img)][ref]`, +// the standard README-badge pattern — must tokenize as ONE reference_link +// carrying the image as its child. The anchor group used `[^\]]+?`, which +// stopped at the image's inner `]`, so the input fragmented into a bare image +// plus a separate empty reference link and the image never nested in the link. + +function toks(src: string): Token[] { + const labels = new Map([['ref', { href: 'https://example.com/dst', title: '' }]]); + return tokenizer(src, { labels } as Parameters[1]); +} + +describe('reference link with an image anchor (#4865)', () => { + it('tokenizes `[![alt](img)][ref]` as a single reference_link wrapping the image', () => { + const result = toks('[![alt](https://example.com/badge.svg)][ref]'); + + expect(result).toHaveLength(1); + const link = result[0] as ReferenceLinkToken; + expect(link.type).toBe('reference_link'); + expect(link.isFullLink).toBe(true); + expect(link.label).toBe('ref'); + + expect(link.children).toHaveLength(1); + const image = link.children[0] as Token & { attrs?: { src: string; alt: string } }; + expect(image.type).toBe('image'); + expect(image.attrs?.src).toBe('https://example.com/badge.svg'); + expect(image.attrs?.alt).toBe('alt'); + }); + + it('still tokenizes a plain-text reference link `[text][ref]` unchanged', () => { + const result = toks('[text][ref]'); + + expect(result).toHaveLength(1); + const link = result[0] as ReferenceLinkToken; + expect(link.type).toBe('reference_link'); + expect(link.children).toHaveLength(1); + expect(link.children[0].type).toBe('text'); + }); + + it('does not treat `[![alt](img)][ref]` as a link when the ref is undefined', () => { + // No matching definition ⇒ not a reference link (CommonMark); the image + // stays a standalone image, never a fabricated link. + const result = tokenizer('[![alt](https://example.com/badge.svg)][missing]'); + + expect(result.some(t => t.type === 'reference_link')).toBe(false); + expect(result.some(t => t.type === 'image')).toBe(true); + }); +}); diff --git a/packages/muya/src/inlineRenderer/renderer/image.ts b/packages/muya/src/inlineRenderer/renderer/image.ts index 9286df11d7..1bdcc9327a 100644 --- a/packages/muya/src/inlineRenderer/renderer/image.ts +++ b/packages/muya/src/inlineRenderer/renderer/image.ts @@ -10,7 +10,12 @@ import { CLASS_NAMES } from '../../config'; import { getImageSrc } from '../../utils/image'; function renderIcon(h: H, className: string, icon: string) { - const selector = `a.${className}`; + // A ``, not an ``: these hover controls carry no href, and an `` + // here nests illegally when the image sits inside a real anchor (e.g. a + // reference-linked image `[![alt](img)][ref]`). The HTML parser closes the + // outer anchor early on the nested ``, hoisting the image out of the link + // (#4865). + const selector = `span.${className}`; const iconVnode = h( 'i.icon', h( diff --git a/packages/muya/src/inlineRenderer/rules.ts b/packages/muya/src/inlineRenderer/rules.ts index 0a1f6cdbe2..eee16b1c62 100644 --- a/packages/muya/src/inlineRenderer/rules.ts +++ b/packages/muya/src/inlineRenderer/rules.ts @@ -31,8 +31,12 @@ export const commonMarkRules = { image: /^(!\[)(.*?)(\\*)\]\((.*)(\\*)\)/, // eslint-disable-next-line regexp/no-super-linear-backtracking, regexp/optimal-quantifier-concatenation, regexp/no-misleading-capturing-group link: /^(\[)((?:\[[^\]]*\]|[^[\]]|\](?=[^[]*\]))*?)(\\*)\]\((.*)(\\*)\)/, // can nest + // Link text can hold balanced brackets — notably an image `![alt](src)` — + // so mirror `link`'s nesting-capable anchor group instead of the bracket- + // free `[^\]]+?`, which stopped at the image's inner `]` and broke + // `[![alt](img)][ref]` (#4865). // eslint-disable-next-line regexp/no-super-linear-backtracking - reference_link: /^\[([^\]]+?)(\\*)\](?:\[([^\]]*?)(\\*)\])?/, + reference_link: /^\[((?:\[[^\]]*\]|[^[\]]|\](?=[^[]*\]))*?)(\\*)\](?:\[([^\]]*?)(\\*)\])?/, // eslint-disable-next-line regexp/no-super-linear-backtracking reference_image: /^!\[([^\]]+?)(\\*)\](?:\[([^\]]*?)(\\*)\])?/, html_tag: diff --git a/packages/muya/src/selection/ImageSelection.ts b/packages/muya/src/selection/ImageSelection.ts index cc13b50aff..bca4b62770 100644 --- a/packages/muya/src/selection/ImageSelection.ts +++ b/packages/muya/src/selection/ImageSelection.ts @@ -3,6 +3,7 @@ import type { Muya } from '../muya'; import type Selection from './index'; import type { IImageSelectionData } from './types'; import { BLOCK_DOM_PROPERTY, CLASS_NAMES } from '../config'; +import { LINK_SELECTOR } from '../editor/linkMouseEvents'; import { isHTMLElement, isKeyboardEvent } from '../utils'; import { getImageInfo, getImageSrc } from '../utils/image'; import { findContentDOM } from './dom'; @@ -102,7 +103,17 @@ class ImageSelection { } if (isHTMLElement(target) && target.tagName === 'IMG') { - if (event instanceof MouseEvent && (event.metaKey || event.ctrlKey)) { + // A linked image (e.g. `[![alt](src)](href)`) renders its image + // wrapper inside a link element. On modifier-click the link handler + // (linkMouseEvents) opens the URL; don't also emit the image preview, + // which would pop a viewer over the navigation (#3835). Reuse + // linkMouseEvents' selector so every link variant is covered (plain, + // reference, autolink, raw-HTML anchor), not just `mu-link`. + if ( + event instanceof MouseEvent + && (event.metaKey || event.ctrlKey) + && !imageWrapper.closest(LINK_SELECTOR) + ) { const tokenSrc = imageInfo.token.src || imageInfo.token.attrs.src || ''; const src = getImageSrc(tokenSrc).src || target.getAttribute('src') || ''; if (src) { diff --git a/packages/muya/src/selection/__tests__/linkedImageClick.spec.ts b/packages/muya/src/selection/__tests__/linkedImageClick.spec.ts new file mode 100644 index 0000000000..53c7abbbdc --- /dev/null +++ b/packages/muya/src/selection/__tests__/linkedImageClick.spec.ts @@ -0,0 +1,119 @@ +// @vitest-environment happy-dom + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { CLASS_NAMES } from '../../config'; +import { Muya } from '../../muya'; + +// #3835: Ctrl/Cmd-clicking a linked image `[![alt](src)](href)` popped the +// image preview (ImageSelection's 'image' format-click) on top of the link +// navigation (linkMouseEvents' 'link' format-click), so the link appeared not +// to open. ImageSelection now skips emitting the image preview for an image +// that lives inside a link when the click carries a modifier. + +const bootedMuyas: Muya[] = []; + +beforeEach(() => { + window.MUYA_VERSION = 'test'; +}); + +afterEach(() => { + while (bootedMuyas.length) + bootedMuyas.pop()!.destroy(); + delete (window as Partial).MUYA_VERSION; +}); + +function boot(markdown: string): Muya { + const host = document.createElement('div'); + document.body.appendChild(host); + const muya = new Muya(host, { markdown } as ConstructorParameters[1]); + muya.init(); + bootedMuyas.push(muya); + return muya; +} + +// The async image load never resolves under happy-dom, so inject the the +// loaded path would have produced. +function injectImg(muya: Muya, src: string): HTMLImageElement { + const wrapper = muya.domNode.querySelector( + `span.${CLASS_NAMES.MU_INLINE_IMAGE}`, + )!; + const container = wrapper.querySelector( + `.${CLASS_NAMES.MU_IMAGE_CONTAINER}`, + )!; + const img = document.createElement('img'); + img.setAttribute('src', src); + container.appendChild(img); + return img; +} + +function ctrlClick(img: HTMLImageElement): void { + img.dispatchEvent( + new MouseEvent('click', { bubbles: true, cancelable: true, ctrlKey: true }), + ); +} + +function captureFormatClickTypes(muya: Muya): string[] { + const types: string[] = []; + muya.eventCenter.on('format-click', (payload: { formatType?: string }) => { + if (payload && payload.formatType) + types.push(payload.formatType); + }); + return types; +} + +describe('linked image modifier-click does not pop the image preview (#3835)', () => { + it('a linked image does not emit an image format-click on Ctrl-click', () => { + const src = 'https://example.com/pic.png'; + const muya = boot(`[![alt](${src})](https://link.example.com)`); + + // The image wrapper renders inside the link span; the fix keys off this. + const wrapper = muya.domNode.querySelector( + `span.${CLASS_NAMES.MU_INLINE_IMAGE}`, + )!; + expect(wrapper.closest(`.${CLASS_NAMES.MU_LINK}`)).not.toBeNull(); + + const img = injectImg(muya, src); + const types = captureFormatClickTypes(muya); + + ctrlClick(img); + + expect(types).not.toContain('image'); + }); + + it('a plain (non-linked) image still emits an image format-click on Ctrl-click', () => { + const src = 'https://example.com/pic.png'; + const muya = boot(`![alt](${src})`); + + const img = injectImg(muya, src); + const types = captureFormatClickTypes(muya); + + ctrlClick(img); + + expect(types).toContain('image'); + }); +}); + +// #4865: a reference-linked image `[![alt](src)][ref]` must render the image +// inside `a.mu-reference-link`, so the #3835 guard applies and Ctrl/Cmd-click +// follows the link. The inline tokenizer previously fragmented it into a bare +// image plus a separate empty reference link (its anchor group stopped at the +// image's inner `]`), leaving the image unwrapped. +describe('reference-linked image modifier-click follows the link (#4865)', () => { + it('renders the image inside a.mu-reference-link and suppresses the image format-click', () => { + const src = 'https://example.com/pic.png'; + const muya = boot(`[![alt](${src})][ref]\n\n[ref]: https://link.example.com`); + + const wrapper = muya.domNode.querySelector( + `span.${CLASS_NAMES.MU_INLINE_IMAGE}`, + )!; + expect(wrapper).not.toBeNull(); + expect(wrapper.closest(`a.${CLASS_NAMES.MU_REFERENCE_LINK}`)).not.toBeNull(); + + const img = injectImg(muya, src); + const types = captureFormatClickTypes(muya); + + ctrlClick(img); + + expect(types).not.toContain('image'); + }); +}); diff --git a/packages/muya/src/selection/__tests__/rawHtmlLinkedImageClick.spec.ts b/packages/muya/src/selection/__tests__/rawHtmlLinkedImageClick.spec.ts new file mode 100644 index 0000000000..30d2c0c2a2 --- /dev/null +++ b/packages/muya/src/selection/__tests__/rawHtmlLinkedImageClick.spec.ts @@ -0,0 +1,83 @@ +// @vitest-environment jsdom + +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { CLASS_NAMES } from '../../config'; +import { Muya } from '../../muya'; + +// #4865 (raw-HTML case): a raw-HTML linked image `` renders +// its `` as a real anchor (`a.mu-raw-html`) — an image's hover icons used to +// be `` elements, so nesting them produced invalid ``-inside-`` and the +// HTML parser hoisted the image out of the link. Icons are now ``, so the +// image stays inside the anchor and Ctrl/Cmd-click follows the link. +// +// This runs under jsdom, NOT happy-dom: DOMPurify strips the `` under +// happy-dom (the raw-HTML tag would render as a ``, masking the anchor +// nesting the fix addresses). jsdom keeps it, matching the real Electron app. + +const bootedMuyas: Muya[] = []; + +beforeEach(() => { + (window as unknown as { MUYA_VERSION?: string }).MUYA_VERSION = 'test'; +}); + +afterEach(() => { + while (bootedMuyas.length) + bootedMuyas.pop()!.destroy(); + delete (window as unknown as { MUYA_VERSION?: string }).MUYA_VERSION; +}); + +function boot(markdown: string): Muya { + const host = document.createElement('div'); + document.body.appendChild(host); + const muya = new Muya(host, { markdown } as ConstructorParameters[1]); + muya.init(); + bootedMuyas.push(muya); + return muya; +} + +function injectImg(muya: Muya, src: string): HTMLImageElement { + const wrapper = muya.domNode.querySelector( + `span.${CLASS_NAMES.MU_INLINE_IMAGE}`, + )!; + const container = wrapper.querySelector( + `.${CLASS_NAMES.MU_IMAGE_CONTAINER}`, + )!; + const img = document.createElement('img'); + img.setAttribute('src', src); + container.appendChild(img); + return img; +} + +function captureFormatClickTypes(muya: Muya): string[] { + const types: string[] = []; + muya.eventCenter.on('format-click', (payload: { formatType?: string }) => { + if (payload && payload.formatType) + types.push(payload.formatType); + }); + return types; +} + +describe('raw-HTML linked image modifier-click follows the link (#4865)', () => { + it('renders the image inside a.mu-raw-html and suppresses the image format-click', () => { + const src = 'https://example.com/pic.png'; + const muya = boot(``); + + const wrapper = muya.domNode.querySelector( + `span.${CLASS_NAMES.MU_INLINE_IMAGE}`, + )!; + expect(wrapper).not.toBeNull(); + // The is a real anchor; the image (and its container) must stay + // inside it rather than being hoisted out by an invalid nested . + expect(wrapper.closest(`a.${CLASS_NAMES.MU_RAW_HTML}`)).not.toBeNull(); + expect(wrapper.querySelector(`.${CLASS_NAMES.MU_IMAGE_CONTAINER}`)).not.toBeNull(); + + const img = injectImg(muya, src); + const types = captureFormatClickTypes(muya); + + img.dispatchEvent( + new MouseEvent('click', { bubbles: true, cancelable: true, ctrlKey: true }), + ); + + expect(types).not.toContain('image'); + }); +}); diff --git a/packages/muya/src/utils/__tests__/image.spec.ts b/packages/muya/src/utils/__tests__/image.spec.ts index 87fdc0f9bc..b6e422c255 100644 --- a/packages/muya/src/utils/__tests__/image.spec.ts +++ b/packages/muya/src/utils/__tests__/image.spec.ts @@ -1,7 +1,7 @@ // @vitest-environment happy-dom -import { afterEach, describe, expect, it } from 'vitest'; -import { getImageSrc } from '../image'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { checkImageContentType, getImageSrc, loadImage } from '../image'; // Regression tests for the Phase G "G1" blocker: relative-path images stopped // rendering after the @muyajs/core migration because `getImageSrc` returned a @@ -27,6 +27,110 @@ afterEach(() => { window.DIRNAME = undefined; }); +describe('checkImageContentType (#3837)', () => { + // Same-origin URL — the only kind whose HEAD the renderer can actually read. + const sameOrigin = (p: string) => new URL(p, window.location.href).href; + const CROSS_ORIGIN = 'https://img.shields.io/badge/x-blue'; + + function mockFetch(status: number, contentType: string | null) { + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ + status, + headers: { + get: (h: string) => + h.toLowerCase() === 'content-type' ? contentType : null, + }, + }), + ); + return globalThis.fetch as unknown as ReturnType; + } + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('accepts a same-origin image type carrying a charset parameter', async () => { + mockFetch(200, 'image/svg+xml;charset=utf-8'); + expect(await checkImageContentType(sameOrigin('/badge'))).toBe(true); + }); + + it('accepts a bare same-origin image content type', async () => { + mockFetch(200, 'image/png'); + expect(await checkImageContentType(sameOrigin('/badge'))).toBe(true); + }); + + it('reports a same-origin non-image content type as false', async () => { + mockFetch(200, 'text/html;charset=utf-8'); + expect(await checkImageContentType(sameOrigin('/page'))).toBe(false); + }); + + it('returns null (undetermined) on a non-200 response', async () => { + mockFetch(404, 'image/png'); + expect(await checkImageContentType(sameOrigin('/missing'))).toBeNull(); + }); + + it('returns null when the same-origin HEAD fails (network)', async () => { + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('network'))); + expect(await checkImageContentType(sameOrigin('/x'))).toBeNull(); + }); + + it('skips the HEAD entirely for a cross-origin URL (CSP/CORS can never read it)', async () => { + const fetchSpy = mockFetch(200, 'image/png'); + expect(await checkImageContentType(CROSS_ORIGIN)).toBeNull(); + // No wasted, guaranteed-to-fail request (and no CSP console error). + expect(fetchSpy).not.toHaveBeenCalled(); + }); +}); + +describe('loadImage — undetermined content-type still attempts the load (#3837)', () => { + // Drive the load deterministically: setting `src` fires onload/onerror. + function stubImage(succeeds: boolean) { + class FakeImage { + width = 10; + height = 10; + onload: (() => void) | null = null; + onerror: ((err: unknown) => void) | null = null; + private _src = ''; + get src(): string { + return this._src; + } + + set src(v: string) { + this._src = v; + queueMicrotask(() => + succeeds ? this.onload?.() : this.onerror?.(new Error('load failed')), + ); + } + } + vi.stubGlobal('Image', FakeImage); + } + + afterEach(() => { + vi.unstubAllGlobals(); + }); + + const sameOrigin = (p: string) => new URL(p, window.location.href).href; + + it('loads a cross-origin extensionless image (its HEAD check is skipped)', async () => { + // The shields.io badge's content-type can't be read (CSP/CORS), so the + // check is skipped and the badge must still load via the permissive img-src. + stubImage(true); + await expect( + loadImage('https://img.shields.io/badge/example-blue', true), + ).resolves.toMatchObject({ width: 10, height: 10 }); + }); + + it('still rejects when a same-origin HEAD positively reports a non-image type', async () => { + vi.stubGlobal( + 'fetch', + vi.fn().mockResolvedValue({ status: 200, headers: { get: () => 'text/html' } }), + ); + stubImage(true); + await expect(loadImage(sameOrigin('/page'), true)).rejects.toBe('not an image.'); + }); +}); + describe('getImageSrc — relative local image paths anchored to window.DIRNAME', () => { it('resolves a relative path against the document directory', () => { withDirname(DIRNAME, () => { diff --git a/packages/muya/src/utils/image.ts b/packages/muya/src/utils/image.ts index 28ae86fa3f..977042b39d 100644 --- a/packages/muya/src/utils/image.ts +++ b/packages/muya/src/utils/image.ts @@ -135,7 +135,10 @@ export async function loadImage(url: string, detectContentType = false): Promise }> { if (detectContentType) { const isImage = await checkImageContentType(url); - if (!isImage) + // Only bail out when we positively know it is NOT an image. `null` + // means we couldn't check (e.g. a cross-origin HEAD blocked by CSP); + // fall through to the actual load, which `img-src` permits (#3837). + if (isImage === false) // eslint-disable-next-line prefer-promise-reject-errors return Promise.reject('not an image.'); } @@ -157,23 +160,47 @@ export async function loadImage(url: string, detectContentType = false): Promise }); } -export async function checkImageContentType(url: string) { +// Only a same-origin URL can have its Content-Type read from the renderer: a +// cross-origin response has its headers stripped by CORS, and the app's CSP +// (no `connect-src`, so it falls back to `default-src 'self'`) refuses the +// request outright. Relative/opaque URLs are treated as same-origin so the +// check is still attempted. +function isSameOrigin(url: string): boolean { + try { + return new URL(url, window.location.href).origin === window.location.origin; + } + catch { + return true; + } +} + +// Returns `true`/`false` when a HEAD response positively identifies the URL as +// an image (or not), or `null` when that can't be determined. A `null` must NOT +// be read as "not an image": the actual load is governed by the far more +// permissive `img-src`, so callers should still attempt it (#3837 — shields.io +// badges and other extensionless remote images). +export async function checkImageContentType(url: string): Promise { + // Don't fire a HEAD we could never read: a cross-origin request is refused + // by the CSP (logging a console error) and unreadable under CORS anyway. + // Report "undetermined" and let the caller fall through to the load. + if (!isSameOrigin(url)) + return null; + try { const res = await fetch(url, { method: 'HEAD' }); - const contentType = res.headers.get('content-type'); - - if ( - contentType - && res.status === 200 - && /^image\/(?:jpeg|png|gif|svg\+xml|webp)$/.test(contentType) - ) { - return true; - } + if (res.status !== 200) + return null; + + // Content-Type can carry parameters (e.g. `image/svg+xml;charset=utf-8`); + // match only the MIME type. + const contentType = res.headers.get('content-type')?.split(';')[0].trim(); + if (!contentType) + return null; - return false; + return /^image\/(?:jpeg|png|gif|svg\+xml|webp)$/.test(contentType); } catch { - return false; + return null; } }