diff --git a/CLAUDE.md b/CLAUDE.md index 5cfe148..5846814 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -145,3 +145,26 @@ Before designing solutions, enumerate the complete problem space: 3. **Identify the coupling.** Features that seem separate often share a communication channel (clipboard, file format, API). Design them together, not as afterthoughts to each other. Only after this enumeration should you design the implementation. + +## Plan Documents + +When creating implementation plans, save them to `docs/plans/` with a ULID filename: + +``` +docs/plans/01KHRS817S0EERAW6NG7FZT1K6.md +``` + +ULIDs are time-sortable unique identifiers. Generate one with: + +```bash +node -e " +const t = Date.now(); +const chars = '0123456789ABCDEFGHJKMNPQRSTVWXYZ'; +let ulid = ''; +for (let i = 0; i < 10; i++) { ulid = chars[t >> (i * 5) & 31] + ulid; } +for (let i = 0; i < 16; i++) { ulid += chars[Math.floor(Math.random() * 32)]; } +console.log(ulid); +" +``` + +Plans should document the design decisions and rationale, not just the implementation steps. They serve as a record of why things were built a certain way. diff --git a/docs/plans/01KHRS817S0EERAW6NG7FZT1K6.md b/docs/plans/01KHRS817S0EERAW6NG7FZT1K6.md new file mode 100644 index 0000000..e56aa6c --- /dev/null +++ b/docs/plans/01KHRS817S0EERAW6NG7FZT1K6.md @@ -0,0 +1,136 @@ +# URL Paste Autolink Refactoring Plan + +## Goal +Refactor the URL paste autolink code to be cleaner and handle the case where pasted content is an autolink (link where href = text) from another application. + +## Unified Flow + +1. **Detect if slice is a URL paste:** + - Plain text that is a valid HTTP URL, OR + - Has link mark where `href === text` (autolink from another app) + - If neither → return false, let PM handle normally + +2. **Extract `href`** from whichever case matched + +3. **Determine link text content:** + - Default: `linkText = href` + - If selection is non-empty AND inline (doesn't cross block boundaries): + `linkText = selected content (preserving marks like bold/italic)` + +4. **Construct the link slice:** + - Create slice containing `linkText` with link mark pointing to `href` + +5. **Paste using `tr.replaceSelection(slice)`:** + - PM handles all structural fixup (empty selection inserts, non-empty replaces, multi-block merges) + +6. **Cleanup:** Remove stored link mark so next typed char isn't linked + +## Helper Functions Needed + +### `isInlineSelection(state): boolean` +Check if current selection is non-empty and doesn't cross block boundaries. + +```typescript +function isInlineSelection(state: EditorState): boolean { + const { from, to, $from, $to } = state.selection; + if (from === to) return false; // empty selection + // Same parent block = inline selection + return $from.sameParent($to); +} +``` + +### `addLinkMarkToFragment(fragment, mark, schema): Fragment` +Recursively add a link mark to all text nodes in a fragment. + +```typescript +function addLinkMarkToFragment(fragment: Fragment, mark: Mark): Fragment { + const nodes: Node[] = []; + fragment.forEach((node) => { + if (node.isText) { + nodes.push(node.mark(mark.addToSet(node.marks))); + } else if (node.content.size > 0) { + nodes.push(node.copy(addLinkMarkToFragment(node.content, mark))); + } else { + nodes.push(node); + } + }); + return Fragment.from(nodes); +} +``` + +## Refactored Paste Handler Logic + +```typescript +// 1. Detect URL and extract href +const sliceText = slice.content.textBetween(0, slice.content.size, "", "").trim(); + +let href: string | null = null; + +// Check for plain text URL +const plainUrl = parseHttpUrl(sliceText); +if (plainUrl) { + href = plainUrl.href; +} else { + // Check for autolink (link mark where href === text) + let linkHref: string | null = null; + slice.content.descendants((node) => { + const linkMark = node.marks?.find((m) => m.type === schema.marks.link); + if (linkMark) { + linkHref = linkMark.attrs.href; + return false; + } + }); + if (linkHref && linkHref === sliceText) { + href = linkHref; + } +} + +if (!href) { + // Not a URL paste, fall through + // ... rest of paste handler +} + +// 2. Determine link text content +const { state, dispatch } = view; +const { selection } = state; +const linkMark = schema.marks.link.create({ href }); + +let linkSlice: Slice; + +if (selection.empty || !isInlineSelection(state)) { + // Empty or multi-block: link text = href + const linkNode = schema.text(href, [linkMark]); + linkSlice = new Slice(Fragment.from(linkNode), 0, 0); +} else { + // Inline selection: link text = selected content with link mark added + const { from, to } = selection; + const selectedSlice = state.doc.slice(from, to); + const linkedContent = addLinkMarkToFragment(selectedSlice.content, linkMark); + linkSlice = new Slice(linkedContent, selectedSlice.openStart, selectedSlice.openEnd); +} + +// 3. Paste +const tr = state.tr.replaceSelection(linkSlice); +tr.removeStoredMark(schema.marks.link); +dispatch(tr); +return true; +``` + +## What Gets Removed + +- The `hasLinkMark` check that skipped autolinks entirely +- The separate Part 1 / Part 2 code paths +- The "Case 2b" link extension logic (selecting inside existing link) + +## Behavior Changes + +| Scenario | Old Behavior | New Behavior | +|----------|--------------|--------------| +| Paste `http://x.com` (autolink) with empty selection | Pasted as-is (link preserved) | Same - pasted as link | +| Paste `http://x.com` with inline selection "click" | Fell through to default paste (replaced "click" with the link) | "click" becomes link to x.com | +| Paste `http://bar.com` (text≠href) | Fell through to default | Falls through (not an autolink) | +| Paste plain URL with selection inside existing link | Extended to full link, replaced href | Just replaces selection (simpler) | + +## Questions + +1. Is dropping the "Case 2b" link extension logic okay? (It was for when selection is inside an existing link - it extended to cover the whole link before replacing href) diff --git a/src/editor/editor.ts b/src/editor/editor.ts index 359fb43..91cb43e 100644 --- a/src/editor/editor.ts +++ b/src/editor/editor.ts @@ -28,12 +28,22 @@ import { createImageNodeView } from "./imageNodeView"; import { categorizeImageSrc, type ImageSrcType } from "./imageUtils"; import { createMathDisplayNodeView } from "./mathNodeView"; import { createMathPlugin } from "./mathPlugin"; -import { schema } from "./schema"; +import { parseHttpUrl, schema } from "./schema"; import { normalizeTablesInSlice } from "./tableNormalize"; // Re-export for backward compatibility export { categorizeImageSrc, type ImageSrcType }; +/** + * Check if current selection is non-empty and inline (doesn't cross block boundaries). + */ +function isInlineSelection(state: EditorState): boolean { + const { from, to, $from, $to } = state.selection; + if (from === to) return false; // empty selection + // Same parent block = inline selection + return $from.sameParent($to); +} + interface ImageToProcess { node: Node; src: string; @@ -370,6 +380,52 @@ export function mountEditor(host: HTMLElement): EditorView { } } + // URL autolink: If pasted content is a single text node containing a valid + // URL, create a link. An "autolink" is either plain text that parses as a + // URL, or text with exactly one link mark where href === text (from an app + // that auto-linked the URL). No other marks allowed, no content outside. + let href: string | null = null; + if (slice.content.childCount === 1) { + let node = slice.content.firstChild; + // Unwrap if single paragraph containing single child + if (node?.type.name === "paragraph" && node.content.childCount === 1) { + node = node.content.firstChild; + } + if (node?.isText && node.text) { + const url = parseHttpUrl(node.text); + if ( + url && + (node.marks.length === 0 || + (node.marks.length === 1 && + node.marks[0].type === schema.marks.link && + node.marks[0].attrs.href === node.text)) + ) { + href = url.href; + } + } + } + + if (href) { + const { state, dispatch } = view; + const { selection } = state; + const linkMark = schema.marks.link.create({ href }); + + let tr: Transaction; + if (!selection.empty && isInlineSelection(state)) { + // Inline selection: add link mark to existing content + tr = state.tr.addMark(selection.from, selection.to, linkMark); + } else { + // Empty or multi-block selection: insert href as linked text + const linkNode = schema.text(href, [linkMark]); + const linkSlice = new Slice(Fragment.from(linkNode), 0, 0); + tr = state.tr.replaceSelection(linkSlice); + } + + tr.removeStoredMark(schema.marks.link); + dispatch(tr); + return true; + } + // Normalize tables in pasted content to enforce GFM semantics // (flatten spanning cells, ensure first row is header cells) slice = normalizeTablesInSlice(slice, schema); diff --git a/src/editor/schema.ts b/src/editor/schema.ts index 9fbceed..8b3d5eb 100644 --- a/src/editor/schema.ts +++ b/src/editor/schema.ts @@ -3,6 +3,44 @@ import { marks, nodes } from "prosemirror-schema-basic"; import { bulletList, listItem, orderedList } from "prosemirror-schema-list"; import { tableNodes } from "prosemirror-tables"; +/** + * Validate that an href is safe for use in links. + * Accepts http, https, and mailto URLs only. + * Used by parseDOM for tags and paste sanitization. + */ +export function isSafeHref(href: string): boolean { + try { + const url = new URL(href); + return ( + url.protocol === "http:" || + url.protocol === "https:" || + url.protocol === "mailto:" + ); + } catch { + return false; + } +} + +/** + * Parse text as an HTTP(S) URL suitable for autolink. + * Stricter than isSafeHref: only http/https with non-empty hostname. + * Returns the parsed URL if valid, null otherwise. + */ +export function parseHttpUrl(text: string): URL | null { + try { + const url = new URL(text); + if ( + (url.protocol === "http:" || url.protocol === "https:") && + url.hostname !== "" + ) { + return url; + } + return null; + } catch { + return null; + } +} + // Generate table node specs from prosemirror-tables // TODO: GFM has per-column alignment (left/center/right/none). // Add `alignments: Alignment[]` attr to table node when we implement alignment UI. @@ -177,6 +215,16 @@ export const schema = new Schema({ const attrs: Record = { src, class: "pm-image" }; if (alt) attrs.alt = alt; if (title) attrs.title = title; + + // If image has a link mark, wrap in + const linkMark = node.marks.find((m) => m.type.name === "link"); + if (linkMark) { + return [ + "a", + { href: linkMark.attrs.href, class: "pm-image-link" }, + ["img", attrs], + ]; + } return ["img", attrs]; }, }, @@ -192,9 +240,8 @@ export const schema = new Schema({ tag: "a[href]", getAttrs(dom) { const href = (dom as HTMLElement).getAttribute("href") || ""; - // Only allow safe URL schemes (http, https, mailto) and relative paths - if (!/^(https?|mailto):/i.test(href) && !href.startsWith("/")) { - return false; // Reject this link + if (!isSafeHref(href)) { + return false; // Strip link, keep text } return { href,