fix(editor): live-preview links open again after #302 - #304
Merged
Conversation
PR #302 made links open on mouse RELEASE and required the release to land on the SAME DOM element as the press. That element does not survive the press: CodeMirror's built-in mousedown observer runs BEFORE any EditorView.domEventHandlers handler, so it places the caret inside the link first, `selectionTouches` then reports the link as touched, buildDecorations drops its decorations, and the `.cm-lp-link` span is already detached by the time mouseup fires. Identity never matched, so nothing opened — every markdown link on beta went dead. Compare the link's key (the href, or `wikilink:<target>`) instead of the element, taken from the press and not from the release. A release over no link at all now counts as a match, because that is the normal case once reveal-on-cursor has swallowed the span; only a DIFFERENT link cancels. The <4 px slop check is what still stops a drag-selection from navigating, so #300's intent is unchanged — the mousemove handler drops its "still over the same element" clause for the same reason. Verified in real Chromium: on unfixed dev a markdown-link click produces no window.open; with the fix all three link kinds open and a drag off a link still selects raw text. The new e2e spec pins that in a real browser — the jsdom test #302 shipped held a stale element reference and stayed green while beta was broken. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011sEc9ZFMVwNR9u8iKp4sZu
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011sEc9ZFMVwNR9u8iKp4sZu
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the beta regression Jon reported on 04/09: clicking a link in live preview does nothing.
Root cause
PR #302 (issue #300) moved link opening from mousedown to mouseup and gated it on the release landing on the same DOM element as the press. That element cannot survive the press:
mousedownobserver runs before anyEditorView.domEventHandlershandler (inputState.runHandlers: observers first, then handlers). It places the caret inside the link and dispatches synchronously.selectionTouches()now reports the link as touched, sobuildDecorationsdrops its decorations and reveals the raw[text](url)..cm-lp-linkspan that received the mousedown is detached before mouseup ever fires.shouldOpenOnReleasecomparedrelease.el !== press.el→ always false → nothing opened.Because the observer runs first, no amount of
preventDefault()in our handler can stop the caret move. Element identity is simply not available at release time.Wikilinks were unaffected (their widget calls
stopPropagation()on mousedown, so the caret never moves and the span survives) — this was a markdown-link / autolink break, which is the common case.The fix
Compare the link's key — the href, or
wikilink:<target>— instead of the element, and take the href to open from the press, not the release:< 4 pxslop check is what still stops a drag-selection from navigating, so Links UX #300's intent is unchanged.mousemovedrops its "still over the same element" clause for the same reason — the element is gone, distance is the answerable question.Checked the other two commits of #302: the
--color-obsidianLinktoken (b954ce6) has a propervar(--obsidian-link, var(--obsidian-accent-purple, hsl(217,88%,50%)))fallback chain and renders fine; the paste toggle (4f0f0c0) is not on the click path.Verification
Real Chromium, dev server on :3001:
devwindow.opennot calledhttps://example.com/mdTests
New, failing before the fix and passing after:
linksLivePreview — link survives reveal-on-cursor between press and releasethe press really does destroy the rendered link element— pins the mechanismclicking a markdown link opens it even though its span was re-rendered— the regressiona wikilink widget still navigates on press + releasea drag off a markdown link still selects instead of navigating (#300)shouldOpenOnRelease › releasing where the link no longer renders STILL opens(replaces the old test that encoded the broken contract)e2e/links-open.spec.ts— real-browser coverage for all three link kinds plus the drag case. fix(editor): links open on release, paste-format toggle, link colour token (#300) #302's jsdom test held a stale reference to the link element, which is why it stayed green while beta was dead.npm run typecheck && npm run lint && npm test→ clean, 3335 passing.🤖 Generated with Claude Code
https://claude.ai/code/session_011sEc9ZFMVwNR9u8iKp4sZu