fix(editor): live-preview links open again after #302

e2e/links-open.spec.ts

@@ -0,0 +1,95 @@
+import { test, expect, type Page } from '@playwright/test'
+
+// Live-preview links must open on a real click, in a real browser.
+//
+// jsdom cannot catch this class of break: PR #302 moved the open from
+// mousedown to mouseup and its jsdom test kept a stale reference to the link
+// element, so it stayed green while every markdown link on beta went dead.
+// Here Chromium does the press and the release itself.
+
+test.beforeEach(async ({ page }) => {
+  await page.addInitScript(() => {
+    try { window.localStorage.clear() } catch { /* ignore */ }
+    try { for (const n of ['noteser', 'keyval-store']) indexedDB.deleteDatabase(n) } catch { /* ignore */ }
+    try {
+      window.localStorage.setItem('noteser-settings', JSON.stringify({
+        state: {
+          onboardingShown: true,
+          sidebarGroups: [{ id: 'g-files', tabs: ['files'], activeTab: 'files', collapsed: false }],
+        },
+        version: 3,
+      }))
+    } catch { /* ignore */ }
+    // Record window.open instead of actually opening a tab.
+    ;(window as unknown as { __opened: string[] }).__opened = []
+    window.open = ((url?: string | URL) => {
+      ;(window as unknown as { __opened: string[] }).__opened.push(String(url))
+      return null
+    }) as typeof window.open
+  })
+})
+
+async function opened(page: Page): Promise<string[]> {
+  return page.evaluate(() => (window as unknown as { __opened: string[] }).__opened)
+}
+
+test('markdown link, bare URL and wikilink all open on a real click', async ({ page }) => {
+  await page.goto('/')
+  await expect(page.getByTestId('folder-tree')).toBeVisible()
+  await page.waitForFunction(() => !!window.__noteser_test?.stores?.noteStore)
+
+  const id = await page.evaluate(() => {
+    const ns = window.__noteser_test!.stores.noteStore.getState()
+    ns.addNote({ title: 'Target', content: 'target body', folderId: null })
+    return ns.addNote({
+      title: 'Links',
+      content: 'md [Example](https://example.com/md) end\n\nbare https://example.com/bare end\n\nwiki [[Target]] end\n',
+      folderId: null,
+    }).id
+  })
+  await page.evaluate((noteId) => {
+    const el = document.querySelector(`[data-testid="note-row"][data-note-id="${noteId}"]`) as HTMLElement
+    el.click(); el.click()
+  }, id)
+
+  await expect(page.locator('.cm-lp-link').first()).toBeVisible()
+
+  // 1. markdown link
+  await page.locator('.cm-lp-link[data-cm-lp-href="https://example.com/md"]').click()
+  await page.waitForTimeout(150)
+  expect(await opened(page)).toContain('https://example.com/md')
+
+  // 2. bare URL
+  await page.locator('.cm-lp-link[data-cm-lp-href="https://example.com/bare"]').click()
+  await page.waitForTimeout(150)
+  expect(await opened(page)).toContain('https://example.com/bare')
+
+  // 3. wikilink
+  await page.locator('.cm-lp-wikilink').click()
+  await page.waitForTimeout(300)
+  const tab = await page.locator('.border-t-obsidianAccentPurple span.truncate').first().textContent()
+  expect(tab).toContain('Target')
+})
+
+test('drag starting on a markdown link selects, does not navigate (#300)', async ({ page }) => {
+  await page.goto('/')
+  await expect(page.getByTestId('folder-tree')).toBeVisible()
+  await page.waitForFunction(() => !!window.__noteser_test?.stores?.noteStore)
+  const id = await page.evaluate(() => window.__noteser_test!.stores.noteStore.getState()
+    .addNote({ title: 'Links2', content: 'md [Example](https://example.com/md) trailing text here\n', folderId: null }).id)
+  await page.evaluate((noteId) => {
+    const el = document.querySelector(`[data-testid="note-row"][data-note-id="${noteId}"]`) as HTMLElement
+    el.click(); el.click()
+  }, id)
+  const link = page.locator('.cm-lp-link[data-cm-lp-href="https://example.com/md"]')
+  await expect(link).toBeVisible()
+  const box = (await link.boundingBox())!
+  await page.mouse.move(box.x + 2, box.y + box.height / 2)
+  await page.mouse.down()
+  await page.mouse.move(box.x + 120, box.y + box.height / 2, { steps: 8 })
+  await page.mouse.up()
+  await page.waitForTimeout(150)
+  const sel = await page.evaluate(() => window.getSelection()?.toString() ?? '')
+  expect(await opened(page)).toHaveLength(0)
+  expect(sel.length).toBeGreaterThan(0)
+})

src/__tests__/linksLivePreview.test.ts

@@ -32,6 +32,7 @@ import {
   shouldOpenOnRelease,
   type LinksLivePreviewDeps,
 } from '../components/editor/linksLivePreview'
+import type { Note } from '../types'
 
 const deps: LinksLivePreviewDeps = {
   getActiveNotes: () => [],
@@ -219,45 +220,47 @@ describe('linksLivePreview — produced decoration types', () => {
 // so a text selection that starts on a link selects instead of navigating.
 
 describe('shouldOpenOnRelease', () => {
-  const el = {} // stand-in for the link element (identity is all that matters)
-  const other = {}
+  const key = 'https://example.com'
+  const other = 'https://other.example'
 
   test('press then release in place on the same link opens', () => {
     expect(shouldOpenOnRelease(
-      { x: 10, y: 10, el },
-      { x: 11, y: 12, el, button: 0 },
+      { x: 10, y: 10, key },
+      { x: 11, y: 12, key, button: 0 },
     )).toBe(true)
   })
 
   test('a 10 px drag before release does NOT open', () => {
     expect(shouldOpenOnRelease(
-      { x: 10, y: 10, el },
-      { x: 20, y: 10, el, button: 0 },
+      { x: 10, y: 10, key },
+      { x: 20, y: 10, key, button: 0 },
     )).toBe(false)
   })
 
   test('release without a recorded press does NOT open', () => {
-    expect(shouldOpenOnRelease(null, { x: 10, y: 10, el, button: 0 })).toBe(false)
+    expect(shouldOpenOnRelease(null, { x: 10, y: 10, key, button: 0 })).toBe(false)
   })
 
   test('releasing over a DIFFERENT link does NOT open', () => {
     expect(shouldOpenOnRelease(
-      { x: 10, y: 10, el },
-      { x: 10, y: 10, el: other, button: 0 },
+      { x: 10, y: 10, key },
+      { x: 10, y: 10, key: other, button: 0 },
     )).toBe(false)
   })
 
-  test('releasing outside any link does NOT open', () => {
+  test('releasing where the link no longer renders STILL opens', () => {
+    // reveal-on-cursor destroys the pressed span during the press, so a
+    // release over no link is the normal case for a click, not a miss.
     expect(shouldOpenOnRelease(
-      { x: 10, y: 10, el },
-      { x: 10, y: 10, el: null, button: 0 },
-    )).toBe(false)
+      { x: 10, y: 10, key },
+      { x: 10, y: 10, key: null, button: 0 },
+    )).toBe(true)
   })
 
   test('the right button never opens', () => {
     expect(shouldOpenOnRelease(
-      { x: 10, y: 10, el },
-      { x: 10, y: 10, el, button: 2 },
+      { x: 10, y: 10, key },
+      { x: 10, y: 10, key, button: 2 },
     )).toBe(false)
   })
 })
@@ -331,3 +334,92 @@ describe('linksLivePreview external-link DOM handlers', () => {
     expect(openSpy).not.toHaveBeenCalled()
   })
 })
+
+// ── Regression: reveal-on-cursor destroys the pressed span (#302 / issue #300) ─
+// CodeMirror's own mousedown observer runs BEFORE our domEventHandlers, so the
+// press moves the caret into the link, `selectionTouches` fires, and the
+// `.cm-lp-link` element that received the mousedown is gone by mouseup.
+
+describe('linksLivePreview — link survives reveal-on-cursor between press and release', () => {
+  let view: EditorView
+  let openSpy: jest.SpyInstance
+
+  function mouse(type: string, x: number, y: number, target: Element, button = 0): void {
+    target.dispatchEvent(new MouseEvent(type, {
+      bubbles: true, cancelable: true, clientX: x, clientY: y, button,
+    }))
+  }
+
+  /** Whatever the pointer is over now — the link if it survived, else the line. */
+  function underPointer(): Element {
+    return view.contentDOM.querySelector('.cm-lp-link')
+      ?? view.contentDOM.firstElementChild as Element
+  }
+
+  beforeAll(() => {
+    const rect = new DOMRect(0, 0, 200, 16)
+    Range.prototype.getClientRects = () => ([rect] as unknown as DOMRectList)
+    Range.prototype.getBoundingClientRect = () => rect
+  })
+
+  beforeEach(() => {
+    openSpy = jest.spyOn(window, 'open').mockImplementation(() => null)
+    view = new EditorView({
+      state: EditorState.create({
+        doc: 'go [text](https://example.com) now\n',
+        selection: { anchor: 0 },
+        extensions: [markdown({ base: markdownLanguage }), linksLivePreview(deps)],
+      }),
+      parent: document.body,
+    })
+  })
+
+  afterEach(() => {
+    view.destroy()
+    openSpy.mockRestore()
+  })
+
+  test('the press really does destroy the rendered link element', () => {
+    // Pins the mechanism this regression test exists for.
+    const pressed = view.contentDOM.querySelector('.cm-lp-link')
+    expect(pressed).not.toBeNull()
+    mouse('mousedown', 5, 5, pressed!)
+    expect(view.contentDOM.querySelector('.cm-lp-link')).toBeNull()
+  })
+
+  test('clicking a markdown link opens it even though its span was re-rendered', () => {
+    mouse('mousedown', 5, 5, view.contentDOM.querySelector('.cm-lp-link')!)
+    mouse('mouseup', 5, 5, underPointer())
+    expect(openSpy).toHaveBeenCalledWith('https://example.com', '_blank', 'noopener,noreferrer')
+  })
+
+  test('a wikilink widget still navigates on press + release', () => {
+    // The widget swallows the mousedown, so its span survives — but it shares
+    // the same press slot, so pin it here too.
+    const nav = jest.fn()
+    const notes = [{ id: 'n1', title: 'My Note', content: '' }] as unknown as Note[]
+    const wikiView = new EditorView({
+      state: EditorState.create({
+        doc: 'go [[My Note]] now\n',
+        selection: { anchor: 0 },
+        extensions: [
+          markdown({ base: markdownLanguage }),
+          linksLivePreview({ getActiveNotes: () => notes, onWikilinkNavigate: nav }),
+        ],
+      }),
+      parent: document.body,
+    })
+    const widget = wikiView.contentDOM.querySelector('.cm-lp-wikilink')!
+    mouse('mousedown', 5, 5, widget)
+    mouse('mouseup', 5, 5, widget)
+    expect(nav).toHaveBeenCalledWith(expect.objectContaining({ id: 'n1' }))
+    wikiView.destroy()
+  })
+
+  test('a drag off a markdown link still selects instead of navigating (#300)', () => {
+    mouse('mousedown', 5, 5, view.contentDOM.querySelector('.cm-lp-link')!)
+    mouse('mousemove', 60, 5, underPointer())
+    mouse('mouseup', 60, 5, underPointer())
+    expect(openSpy).not.toHaveBeenCalled()
+  })
+})

src/components/editor/linksLivePreview.tsx

@@ -56,17 +56,26 @@ export const LINK_DRAG_SLOP_PX = 4
 export interface LinkPress {
   x: number
   y: number
-  /** Identity of the pressed link element. */
-  el: unknown
+  /** What was pressed: the external href, or `wikilink:<target>`. */
+  key: string
 }
 
-/** Pure decision: does this release open the link the press started on? */
+/**
+ * Pure decision: does this release open the link the press started on?
+ *
+ * NOT compared by element identity. CodeMirror's own mousedown observer runs
+ * before our handlers, so the press moves the caret into the link,
+ * reveal-on-cursor rebuilds the decorations, and the pressed `.cm-lp-link`
+ * span is already detached by mouseup. We compare the link's key instead, and
+ * a release over no link at all still counts — only a DIFFERENT link cancels.
+ * The slop check is what keeps a drag-selection from navigating (issue #300).
+ */
 export function shouldOpenOnRelease(
   press: LinkPress | null,
-  release: { x: number; y: number; el: unknown; button: number },
+  release: { x: number; y: number; key: string | null; button: number },
 ): boolean {
   if (!press || release.button !== 0) return false
-  if (release.el == null || release.el !== press.el) return false
+  if (release.key !== null && release.key !== press.key) return false
   return Math.hypot(release.x - press.x, release.y - press.y) < LINK_DRAG_SLOP_PX
 }
 
@@ -111,16 +120,17 @@ class WikilinkWidget extends WidgetType {
       ? `Open: ${note.title}${fragment ? ` → ${fragment}` : ''}`
       : `Note not found: ${title}`
     if (!note) span.classList.add('cm-lp-wikilink-missing')
+    const key = `wikilink:${this.target}`
     // mousedown (not click): stop CodeMirror from moving the caret into the
     // widget. Navigation waits for the release (issue #300).
     span.addEventListener('mousedown', e => {
       e.preventDefault()
       e.stopPropagation()
-      linkPress = e.button === 0 ? { x: e.clientX, y: e.clientY, el: span } : null
+      linkPress = e.button === 0 ? { x: e.clientX, y: e.clientY, key } : null
     })
     span.addEventListener('mouseup', e => {
       const open = shouldOpenOnRelease(linkPress, {
-        x: e.clientX, y: e.clientY, el: span, button: e.button,
+        x: e.clientX, y: e.clientY, key, button: e.button,
       })
       linkPress = null
       if (!open) return
@@ -317,36 +327,36 @@ const linksTheme = EditorView.baseTheme({
 // Plain left-click opens the link — matching Obsidian's live-preview, where a
 // rendered link is clickable directly (no modifier needed) — but only on
 // RELEASE, so dragging a selection off a link selects instead of navigating.
-function linkAt(event: MouseEvent): HTMLElement | null {
+function hrefAt(event: MouseEvent): string | null {
   const target = event.target as HTMLElement | null
-  return (target?.closest?.('.cm-lp-link') as HTMLElement | null) ?? null
+  const el = target?.closest?.('.cm-lp-link') as HTMLElement | null
+  return el?.getAttribute('data-cm-lp-href') ?? null
 }
 
 const externalLinkClickHandler = EditorView.domEventHandlers({
   mousedown(event) {
     // No preventDefault: CodeMirror still gets to start a selection here.
-    const el = event.button === 0 ? linkAt(event) : null
-    linkPress = el ? { x: event.clientX, y: event.clientY, el } : null
+    const href = event.button === 0 ? hrefAt(event) : null
+    linkPress = href ? { x: event.clientX, y: event.clientY, key: href } : null
     return false
   },
   mousemove(event) {
-    // Dragging away (or off the link) cancels the pending open.
+    // Dragging away cancels the pending open. Distance only — the pressed
+    // element is gone by now, so "still over the same link" is unanswerable.
     if (!linkPress) return false
     const moved = Math.hypot(event.clientX - linkPress.x, event.clientY - linkPress.y)
-    if (moved >= LINK_DRAG_SLOP_PX || linkAt(event) !== linkPress.el) linkPress = null
+    if (moved >= LINK_DRAG_SLOP_PX) linkPress = null
     return false
   },
   mouseup(event) {
-    const el = linkAt(event)
-    const open = shouldOpenOnRelease(linkPress, {
-      x: event.clientX, y: event.clientY, el, button: event.button,
+    const press = linkPress
+    const open = shouldOpenOnRelease(press, {
+      x: event.clientX, y: event.clientY, key: hrefAt(event), button: event.button,
     })
     linkPress = null
-    if (!open || !el) return false
-    const href = el.getAttribute('data-cm-lp-href')
-    if (!href) return false
+    if (!open || !press) return false
     event.preventDefault()
-    openExternal(href)
+    openExternal(press.key)
     return true
   },
 })