From 04ad2925c50b9bd5c646857ac2ba82665e3c56ee Mon Sep 17 00:00:00 2001 From: yousefed Date: Mon, 7 Sep 2026 12:25:53 +0200 Subject: [PATCH 1/3] fix(core): find links at both edges when editing or deleting, pre-fill only from the selected text The link mark is not inclusive, so a mark lookup at either edge of a link misses it, and editLink / deleteLink compensated with a +1 that covers the left edge only. getLinkMarkAtPos now uses tiptap's getMarkRange, which looks after and then before the position, and reads the mark off the node the range starts with; the callers drop the +1. getSelectedLinkUrl, which decides whether the link form opens pre-filled, deliberately reads only the node the selection starts in: a selection that starts right after a link must not pre-fill that link's URL, while selecting only the last character of a link, or a one-character link, must. Tests: headless cases for the selection lookup at every edge, and a mounted case where editing a link with only its last character selected used to split it. --- .../managers/StyleManager.browser.test.ts | 78 +++++++++++++++ .../src/editor/managers/StyleManager.test.ts | 98 +++++++++++++++++++ .../core/src/editor/managers/StyleManager.ts | 41 +++++--- 3 files changed, 203 insertions(+), 14 deletions(-) create mode 100644 packages/core/src/editor/managers/StyleManager.browser.test.ts create mode 100644 packages/core/src/editor/managers/StyleManager.test.ts diff --git a/packages/core/src/editor/managers/StyleManager.browser.test.ts b/packages/core/src/editor/managers/StyleManager.browser.test.ts new file mode 100644 index 0000000000..eceff3c1f8 --- /dev/null +++ b/packages/core/src/editor/managers/StyleManager.browser.test.ts @@ -0,0 +1,78 @@ +import { TextSelection } from "prosemirror-state"; +import { afterEach, beforeEach, describe, expect, it } from "vite-plus/test"; + +import { BlockNoteEditor } from "../BlockNoteEditor.js"; + +// `editLink` focuses the view when done, so it needs a mounted editor; the +// lookup itself is covered headless in StyleManager.test.ts. +describe("editLink", () => { + let editor: BlockNoteEditor; + let container: HTMLElement; + + beforeEach(() => { + container = document.createElement("div"); + const mountPoint = document.createElement("div"); + container.append(mountPoint); + document.body.append(container); + editor = BlockNoteEditor.create({ + initialContent: [ + { + type: "paragraph", + content: [ + { type: "text", text: "before ", styles: {} }, + { type: "link", href: "https://example.com", content: "link" }, + { type: "text", text: " after", styles: {} }, + ], + }, + ], + }); + editor.mount(mountPoint); + }); + + afterEach(() => { + editor.unmount(); + container.remove(); + }); + + /** Text nodes carrying a link mark, in document order, with positions. */ + function links() { + const found: { href: string; text: string; from: number; to: number }[] = + []; + editor.prosemirrorState.doc.descendants((node, pos) => { + const link = node.marks.find((mark) => mark.type.name === "link"); + if (node.isText && link) { + found.push({ + href: link.attrs.href, + text: node.text ?? "", + from: pos, + to: pos + node.nodeSize, + }); + } + }); + return found; + } + + // The edit path has the same edge as `getSelectedLinkUrl`: a lookup at + // `position + 1` is outside the link at its end, so with only the last + // character selected the form pre-filled the URL and then re-linked that + // one character, splitting the link. Fails without `getMarkRange` in + // `getLinkMarkAtPos`. + it("edits the whole link with only its last character selected", () => { + const [link] = links(); + expect(link).toMatchObject({ href: "https://example.com", text: "link" }); + editor.transact((tr) => + tr.setSelection(TextSelection.create(tr.doc, link.to - 1, link.to)), + ); + + editor.editLink("https://changed.example", "link"); + + expect(links()).toEqual([ + { + href: "https://changed.example", + text: "link", + from: link.from, + to: link.to, + }, + ]); + }); +}); diff --git a/packages/core/src/editor/managers/StyleManager.test.ts b/packages/core/src/editor/managers/StyleManager.test.ts new file mode 100644 index 0000000000..7090c7ddf8 --- /dev/null +++ b/packages/core/src/editor/managers/StyleManager.test.ts @@ -0,0 +1,98 @@ +import { TextSelection } from "prosemirror-state"; +import { beforeEach, describe, expect, it } from "vite-plus/test"; + +import { BlockNoteEditor } from "../BlockNoteEditor.js"; + +// The "last character" and "one-character link" cases fail with a lookup at +// `from + 1`: that lands on the link's end boundary, where the non-inclusive +// link mark is not reported. +describe("getSelectedLinkUrl", () => { + let editor: BlockNoteEditor; + + beforeEach(() => { + editor = BlockNoteEditor.create({ + initialContent: [ + { + type: "paragraph", + content: [ + { type: "text", text: "before ", styles: {} }, + { type: "link", href: "https://example.com", content: "link" }, + { type: "text", text: " after ", styles: {} }, + { type: "link", href: "https://one.example", content: "x" }, + ], + }, + ], + }); + }); + + /** Start and end positions of the text node carrying `href`. */ + function linkRange(href: string) { + let range: { from: number; to: number } | undefined; + editor.prosemirrorState.doc.descendants((node, pos) => { + if ( + node.isText && + node.marks.some( + (mark) => mark.type.name === "link" && mark.attrs.href === href, + ) + ) { + range = { from: pos, to: pos + node.nodeSize }; + } + return range === undefined; + }); + if (!range) { + throw new Error(`no link ${href} in the document`); + } + return range; + } + + function select(from: number, to = from) { + editor.transact((tr) => + tr.setSelection(TextSelection.create(tr.doc, from, to)), + ); + } + + it("reads the URL for the whole link", () => { + const { from, to } = linkRange("https://example.com"); + select(from, to); + expect(editor.getSelectedLinkUrl()).toBe("https://example.com"); + }); + + it("reads the URL for the link's last character", () => { + const { to } = linkRange("https://example.com"); + select(to - 1, to); + expect(editor.getSelectedLinkUrl()).toBe("https://example.com"); + }); + + it("reads the URL for a one-character link", () => { + const { from, to } = linkRange("https://one.example"); + select(from, to); + expect(editor.getSelectedLinkUrl()).toBe("https://one.example"); + }); + + it("reads the URL for a caret inside the link", () => { + const { from } = linkRange("https://example.com"); + select(from + 2); + expect(editor.getSelectedLinkUrl()).toBe("https://example.com"); + }); + + it("reads no URL outside links", () => { + const { from } = linkRange("https://example.com"); + select(from - 3, from - 1); + expect(editor.getSelectedLinkUrl()).toBeUndefined(); + }); + + it("reads no URL for a selection starting right after a link", () => { + const { to } = linkRange("https://example.com"); + select(to, to + 3); + expect(editor.getSelectedLinkUrl()).toBeUndefined(); + }); + + // The edit path is covered in StyleManager.browser.test.ts (it needs a + // mounted view); the lookup it relies on is pinned here. + it("finds the link at either edge for editing", () => { + const { from, to } = linkRange("https://example.com"); + expect(editor.getLinkMarkAtPos(from)?.href).toBe("https://example.com"); + expect(editor.getLinkMarkAtPos(to)?.href).toBe("https://example.com"); + expect(editor.getLinkMarkAtPos(from - 2)).toBeUndefined(); + }); +}); diff --git a/packages/core/src/editor/managers/StyleManager.ts b/packages/core/src/editor/managers/StyleManager.ts index 123ac6187b..7d2f8c4d7a 100644 --- a/packages/core/src/editor/managers/StyleManager.ts +++ b/packages/core/src/editor/managers/StyleManager.ts @@ -147,22 +147,26 @@ export class StyleManager< } /** - * Find the link mark and its range at the given position. + * Find the link mark and its range at the given position, including at + * either edge of the link. * Returns undefined if there is no link at that position. */ public getLinkMarkAtPos(pos: number) { return this.editor.transact((tr) => { - const resolvedPos = tr.doc.resolve(pos); - const linkMark = resolvedPos - .marks() - .find((mark) => mark.type.name === "link"); - - if (!linkMark) { + // The link mark is not inclusive, so `$pos.marks()` leaves it out at + // both ends of a link, and callers compensated with `pos + 1`, which + // covers the left end only. tiptap's `getMarkRange` looks at the node + // after `pos` and then the one before, so it finds the link at both + // ends; the mark itself is read off the node the range starts with. + const linkType = this.editor.pmSchema.marks["link"]; + const range = getMarkRange(tr.doc.resolve(pos), linkType); + if (!range) { return undefined; } - - const range = getMarkRange(resolvedPos, linkMark.type); - if (!range) { + const linkMark = tr.doc + .nodeAt(range.from) + ?.marks.find((mark) => mark.type === linkType); + if (!linkMark) { return undefined; } @@ -176,11 +180,20 @@ export class StyleManager< } /** - * Gets the URL of the last link in the current selection, or `undefined` if there are no links in the selection. + * Gets the URL of the link the current selection starts in, or `undefined` + * if it does not start in one. */ public getSelectedLinkUrl() { return this.editor.transact((tr) => { - return this.getLinkMarkAtPos(tr.selection.from)?.href; + // The node the selection starts in, on purpose not `getLinkMarkAtPos` + // (which also looks at the node before `from`): a selection starting + // right after a link must not pre-fill the link form with that link's + // URL. A `from + 1` lookup would miss the right end of a link (a + // selection of its last character, or a one-character link, reads as + // no link). + const node = tr.doc.nodeAt(tr.selection.from); + const linkMark = node?.marks.find((mark) => mark.type.name === "link"); + return linkMark?.attrs.href as string | undefined; }); } @@ -222,7 +235,7 @@ export class StyleManager< position = this.editor.transact((tr) => tr.selection.anchor), ) { this.editor.transact((tr) => { - const linkData = this.getLinkMarkAtPos(position + 1); + const linkData = this.getLinkMarkAtPos(position); const { from, to } = linkData || { from: tr.selection.from, to: tr.selection.to, @@ -246,7 +259,7 @@ export class StyleManager< position = this.editor.transact((tr) => tr.selection.anchor), ) { this.editor.transact((tr) => { - const linkData = this.getLinkMarkAtPos(position + 1); + const linkData = this.getLinkMarkAtPos(position); const { from, to } = linkData || { from: tr.selection.from, to: tr.selection.to, From 1a3c6f94f6491226e2f1ecbee4fc0b21d2f61625 Mon Sep 17 00:00:00 2001 From: Nick the Sick Date: Tue, 15 Sep 2026 11:42:44 +0200 Subject: [PATCH 2/3] test(core): cover link editing boundaries and document end --- .../managers/StyleManager.browser.test.ts | 80 +++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/packages/core/src/editor/managers/StyleManager.browser.test.ts b/packages/core/src/editor/managers/StyleManager.browser.test.ts index eceff3c1f8..79269e7aea 100644 --- a/packages/core/src/editor/managers/StyleManager.browser.test.ts +++ b/packages/core/src/editor/managers/StyleManager.browser.test.ts @@ -76,3 +76,83 @@ describe("editLink", () => { ]); }); }); + +// Regression cases from #3081, exercised against #3058's shared lookup. +describe.each(["Google", "x"])("link boundaries (%s)", (text) => { + let editor: BlockNoteEditor; + let container: HTMLElement; + + beforeEach(() => { + container = document.createElement("div"); + document.body.append(container); + editor = BlockNoteEditor.create({ + initialContent: [ + { + id: "block", + type: "paragraph", + content: [ + { type: "link", href: "https://google.com", content: text }, + ], + }, + ], + }); + editor.mount(container); + }); + + afterEach(() => { + editor.unmount(); + container.remove(); + }); + + it.each(["start", "end"] as const)( + "edits the whole link at its %s", + (edge) => { + editor.setTextCursorPosition("block", edge); + editor.editLink("https://changed.example", "Google Search"); + expect(editor.getBlock("block")?.content).toEqual([ + { + type: "link", + href: "https://changed.example", + content: [{ type: "text", text: "Google Search", styles: {} }], + }, + ]); + }, + ); + + it.each(["start", "end"] as const)( + "removes the link at its %s, preserving text", + (edge) => { + editor.setTextCursorPosition("block", edge); + editor.deleteLink(); + expect(editor.getBlock("block")?.content).toEqual([ + { type: "text", text, styles: {} }, + ]); + }, + ); + + it("edits at the document end without throwing, using the selection fallback", () => { + editor.setTextCursorPosition("block", "start"); + const end = editor.prosemirrorState.doc.content.size; + expect(editor.getLinkMarkAtPos(end)).toBeUndefined(); + editor.editLink("https://changed.example", "New", end); + expect(editor.getBlock("block")?.content).toEqual([ + { + type: "link", + href: "https://changed.example", + content: [{ type: "text", text: "New", styles: {} }], + }, + { + type: "link", + href: "https://google.com", + content: [{ type: "text", text, styles: {} }], + }, + ]); + }); + + it("deletes at the document end without throwing or changing an unselected link", () => { + editor.setTextCursorPosition("block", "end"); + const before = editor.document; + editor.deleteLink(editor.prosemirrorState.doc.content.size); + expect(editor.document).toEqual(before); + }); +}); From 94ed6955f0a914b5de9868d1728c68e08cf0bdec Mon Sep 17 00:00:00 2001 From: Nick the Sick Date: Tue, 15 Sep 2026 11:43:37 +0200 Subject: [PATCH 3/3] test(core): reuse link fixture for focused boundary regressions --- .../managers/StyleManager.browser.test.ts | 118 ++++++------------ 1 file changed, 41 insertions(+), 77 deletions(-) diff --git a/packages/core/src/editor/managers/StyleManager.browser.test.ts b/packages/core/src/editor/managers/StyleManager.browser.test.ts index 79269e7aea..4caa7d355c 100644 --- a/packages/core/src/editor/managers/StyleManager.browser.test.ts +++ b/packages/core/src/editor/managers/StyleManager.browser.test.ts @@ -5,7 +5,7 @@ import { BlockNoteEditor } from "../BlockNoteEditor.js"; // `editLink` focuses the view when done, so it needs a mounted editor; the // lookup itself is covered headless in StyleManager.test.ts. -describe("editLink", () => { +describe.each(["link", "x"])("link editing (%s)", (text) => { let editor: BlockNoteEditor; let container: HTMLElement; @@ -20,7 +20,7 @@ describe("editLink", () => { type: "paragraph", content: [ { type: "text", text: "before ", styles: {} }, - { type: "link", href: "https://example.com", content: "link" }, + { type: "link", href: "https://example.com", content: text }, { type: "text", text: " after", styles: {} }, ], }, @@ -59,100 +59,64 @@ describe("editLink", () => { // `getLinkMarkAtPos`. it("edits the whole link with only its last character selected", () => { const [link] = links(); - expect(link).toMatchObject({ href: "https://example.com", text: "link" }); + expect(link).toMatchObject({ href: "https://example.com", text }); editor.transact((tr) => tr.setSelection(TextSelection.create(tr.doc, link.to - 1, link.to)), ); - editor.editLink("https://changed.example", "link"); + editor.editLink("https://changed.example", text); expect(links()).toEqual([ { href: "https://changed.example", - text: "link", + text, from: link.from, to: link.to, }, ]); }); -}); - -// Regression cases from #3081, exercised against #3058's shared lookup. -describe.each(["Google", "x"])("link boundaries (%s)", (text) => { - let editor: BlockNoteEditor; - let container: HTMLElement; - beforeEach(() => { - container = document.createElement("div"); - document.body.append(container); - editor = BlockNoteEditor.create({ - initialContent: [ - { - id: "block", - type: "paragraph", - content: [ - { type: "link", href: "https://google.com", content: text }, - ], - }, - ], - }); - editor.mount(container); - }); - - afterEach(() => { - editor.unmount(); - container.remove(); - }); - - it.each(["start", "end"] as const)( - "edits the whole link at its %s", - (edge) => { - editor.setTextCursorPosition("block", edge); - editor.editLink("https://changed.example", "Google Search"); - expect(editor.getBlock("block")?.content).toEqual([ - { - type: "link", - href: "https://changed.example", - content: [{ type: "text", text: "Google Search", styles: {} }], - }, - ]); - }, - ); - - it.each(["start", "end"] as const)( - "removes the link at its %s, preserving text", - (edge) => { - editor.setTextCursorPosition("block", edge); - editor.deleteLink(); - expect(editor.getBlock("block")?.content).toEqual([ - { type: "text", text, styles: {} }, - ]); - }, - ); - - it("edits at the document end without throwing, using the selection fallback", () => { - editor.setTextCursorPosition("block", "start"); - const end = editor.prosemirrorState.doc.content.size; - expect(editor.getLinkMarkAtPos(end)).toBeUndefined(); - editor.editLink("https://changed.example", "New", end); - expect(editor.getBlock("block")?.content).toEqual([ + it("replaces the whole link at its end", () => { + const [link] = links(); + editor.transact((tr) => + tr.setSelection(TextSelection.create(tr.doc, link.to)), + ); + editor.editLink("https://changed.example", "updated"); + expect(links()).toEqual([ { - type: "link", href: "https://changed.example", - content: [{ type: "text", text: "New", styles: {} }], - }, - { - type: "link", - href: "https://google.com", - content: [{ type: "text", text, styles: {} }], + text: "updated", + from: link.from, + to: link.from + 7, }, ]); + expect(editor.prosemirrorState.doc.textContent).toBe( + "before updated after", + ); + }); + + it("unlinks at the end without removing text", () => { + const [link] = links(); + editor.transact((tr) => + tr.setSelection(TextSelection.create(tr.doc, link.to)), + ); + editor.deleteLink(); + expect(links()).toEqual([]); + expect(editor.prosemirrorState.doc.textContent).toBe( + `before ${text} after`, + ); }); - it("deletes at the document end without throwing or changing an unselected link", () => { - editor.setTextCursorPosition("block", "end"); - const before = editor.document; - editor.deleteLink(editor.prosemirrorState.doc.content.size); - expect(editor.document).toEqual(before); + it("does not throw at the document end", () => { + expect(() => + editor.editLink( + "https://changed.example", + "updated", + editor.prosemirrorState.doc.content.size, + ), + ).not.toThrow(); + expect(() => + editor.deleteLink(editor.prosemirrorState.doc.content.size), + ).not.toThrow(); }); });