Skip to content

Fix/unset link clear transient hyperlink styleid #3190

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
christos8333 wants to merge 2 commits into
base: main
Choose a base branch
from
+189 −5
Open

Fix/unset link clear transient hyperlink styleid #3190

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
02bff26
fix: clear transient hyperlink styleId on unlink
christos8333 May 7, 2026
374cf73
test: add unlink regression coverage for transient hyperlink style cl…
christos8333 May 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
95 changes: 91 additions & 4 deletions packages/super-editor/src/editors/v1/extensions/link/link.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { Attribute } from '@core/Attribute.js';
import { getMarkRange } from '@core/helpers/getMarkRange.js';
import { findOrCreateRelationship } from '@core/parts/adapters/relationships-mutation.js';
import { sanitizeHref, encodeTooltip, UrlValidationConstants } from '@superdoc/url-validation';
import { TRANSIENT_HYPERLINK_STYLE_IDS } from '@extensions/run/calculateInlineRunPropertiesPlugin.js';

/**
* Target frame options
Expand Down Expand Up @@ -36,6 +37,7 @@ import { sanitizeHref, encodeTooltip, UrlValidationConstants } from '@superdoc/u
* @property {string} [docLocation] - Location in target hyperlink
* @property {string} [tooltip] - Tooltip for the link
* @property {string} [rId] @internal Word relationship ID for internal links
* @property {boolean} [hadUnderline] @internal Whether selection already had underline before setLink
*/

/**
Expand Down Expand Up @@ -119,6 +121,12 @@ export const Link = Mark.create({
* @param {string} [rId] - Word relationship ID for internal links
*/
rId: { default: this.options.htmlAttributes.rId || null },
/**
* @private
* @category Attribute
* @param {boolean} [hadUnderline] - keep if underline existed before link creation
*/
hadUnderline: { default: false, rendered: false },
/**
* @category Attribute
* @param {string} [text] - Display text for the link
Expand Down Expand Up @@ -226,6 +234,16 @@ export const Link = Mark.create({
to = from + finalText.length;
}

let hadUnderline = false;
Copy link
Copy Markdown
Contributor

@caio-pizzol caio-pizzol May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

repro:

  1. type "abcdef"
  2. underline only "abc"
  3. select all
  4. click the link icon, paste a URL, Apply
  5. click the link, click Remove

result: "abcd" stays underlined. "d" was plain to start with, and there's no easy way to clear it.

the flag is one true/false for the whole selection, so any partial underline turns it on. fix: track it per character, or drop the underline-removal step and let the styleId cleanup handle the visual.

if (underlineMarkType) {
state.doc.nodesBetween(from, to, (node) => {
Copy link
Copy Markdown
Contributor

@caio-pizzol caio-pizzol May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

repro:

  1. type "hi", select all
  2. click the link icon
  3. change "Text" in the dialog to something longer than "hi"
  4. paste a URL, click Apply

the editor crashes: TypeError: Cannot read properties of undefined (reading 'nodeSize').

fix: do the hadUnderline scan before line 234 reassigns to, or scan tr.doc after the new text is inserted.

if (!node.isText) return;
if (node.marks.some((mark) => mark.type === underlineMarkType)) {
hadUnderline = true;
}
});
}

if (linkMarkType) tr = tr.removeMark(from, to, linkMarkType);
if (underlineMarkType) tr = tr.removeMark(from, to, underlineMarkType);

Expand All @@ -237,7 +255,7 @@ export const Link = Mark.create({
if (id) rId = id;
}

const linkAttrs = { text: finalText, rId };
const linkAttrs = { text: finalText, rId, hadUnderline };
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Track underline preservation per segment

When a selection mixes underlined and plain text, this stores one hadUnderline value on the entire new link mark as soon as any text in the range was underlined. In that scenario (for example, underline only the first word, select the whole phrase, set a link, then unset it), unsetLink treats the whole link as pre-underlined and skips removing underline everywhere, leaving link-added underline on text that was originally plain. Preserve this state per link segment or remove underline only from segments that did not have it before linking.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@caio-pizzol caio-pizzol May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@christos8333 valid - i reproduced this through the toolbar flow. flagged it in the review at line 237 with a fix sketch.

Copy link
Copy Markdown
Contributor

@caio-pizzol caio-pizzol May 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

repro:

  1. type "hi", select all, link to https://first.example.com, Apply
  2. click the link, change URL to https://second.example.com, Apply
  3. click the link, Remove

result: "hi" is still underlined, even though it was plain to start with.

editing a link re-runs setLink, which scans and sees the underline the first link added. the flag flips to true and unlink leaves it alone.

if (sanitizedHref?.href) {
linkAttrs.href = sanitizedHref.href;
}
Expand All @@ -258,11 +276,80 @@ export const Link = Mark.create({
*/
unsetLink:
() =>
({ chain }) => {
return chain()
.unsetMark('underline', { extendEmptyMarkRange: true })
({ chain, state, editor }) => {
const { selection } = state;
const linkMarkType = editor.schema.marks.link;

let { from, to } = selection;

if (selection.empty && linkMarkType) {
const range = getMarkRange(selection.$from, linkMarkType);
if (range) {
from = range.from;
to = range.to;
}
}

let hadUnderline = false;
if (linkMarkType) {
state.doc.nodesBetween(from, to, (node) => {
if (!node.isText || hadUnderline) return;
const linkMark = node.marks.find((mark) => mark.type === linkMarkType);
if (linkMark?.attrs?.hadUnderline) {
hadUnderline = true;
}
});
}

const commandChain = chain();
if (!hadUnderline) {
commandChain.unsetMark('underline', { extendEmptyMarkRange: true });
}

return commandChain
.unsetColor()
.unsetMark('link', { extendEmptyMarkRange: true })
.command(({ tr }) => {
const textStyleMarkType = tr.doc.type.schema.marks.textStyle;
if (textStyleMarkType) {
tr.doc.nodesBetween(from, to, (node, pos) => {
if (!node.isText) return;

node.marks.forEach((mark) => {
if (mark.type !== textStyleMarkType) return;
if (!TRANSIENT_HYPERLINK_STYLE_IDS.has(mark.attrs?.styleId)) return;

const clearedAttrs = { ...mark.attrs, styleId: null };
tr.removeMark(pos, pos + node.nodeSize, mark);
tr.addMark(pos, pos + node.nodeSize, textStyleMarkType.create(clearedAttrs));
});
});
}

const runNodesToUpdate = [];
tr.doc.nodesBetween(from, to, (node, pos) => {
if (node.type.name !== 'run') return;
if (!TRANSIENT_HYPERLINK_STYLE_IDS.has(node.attrs?.runProperties?.styleId)) return;
runNodesToUpdate.push({ node, pos });
});

runNodesToUpdate
.sort((a, b) => b.pos - a.pos)
.forEach(({ node, pos }) => {
const mappedPos = tr.mapping.map(pos);
tr.setNodeMarkup(
mappedPos,
node.type,
{
...node.attrs,
runProperties: { ...node.attrs.runProperties, styleId: null },
},
node.marks,
);
});

return true;
})
.run();
},

Expand Down
2 changes: 1 addition & 1 deletion packages/super-editor/src/editors/v1/extensions/run/calculateInlineRunPropertiesPlugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ const RUN_PROPERTIES_DERIVED_FROM_MARKS = new Set([
'position',
]);

const TRANSIENT_HYPERLINK_STYLE_IDS = new Set(['Hyperlink', 'FollowedHyperlink']);
export const TRANSIENT_HYPERLINK_STYLE_IDS = new Set(['Hyperlink', 'FollowedHyperlink']);

const RUN_PROPERTY_PRESERVE_META_KEY = 'sdPreserveRunPropertiesKeys';

Expand Down
97 changes: 97 additions & 0 deletions packages/super-editor/src/editors/v1/tests/editor/relationships.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,103 @@ describe('Relationships tests', () => {
expect(found.attributes.Target).toBe('https://www.superdoc.dev');
});

it.each(['Hyperlink', 'FollowedHyperlink'])(
'clears transient textStyle and runProperties styleId "%s" when unsetting a link',
(styleId) => {
editor.commands.insertContent('link');
editor.commands.selectAll();
editor.commands.setLink({ href: 'https://www.superdoc.dev' });
editor.commands.setMark('textStyle', { styleId });
editor.commands.command(({ tr, dispatch }) => {
const runNodesToPatch = [];

tr.doc.descendants((node, pos) => {
if (node.type.name !== 'run') return;

runNodesToPatch.push({ node, pos });
});

runNodesToPatch
.sort((a, b) => b.pos - a.pos)
.forEach(({ node, pos }) => {
tr.setNodeMarkup(
pos,
node.type,
{
...node.attrs,
runProperties: { ...node.attrs.runProperties, styleId },
},
node.marks,
);
});

dispatch(tr);
return true;
});

editor.commands.unsetLink();

const textStyleMarks = [];
const runNodes = [];
editor.state.doc.descendants((node) => {
if (!node.isText) return;
node.marks.forEach((mark) => {
if (mark.type.name === 'textStyle') {
textStyleMarks.push(mark);
}
});
});
editor.state.doc.descendants((node) => {
if (node.type.name !== 'run') return;
runNodes.push(node);
});

expect(textStyleMarks.length).toBeGreaterThan(0);
textStyleMarks.forEach((mark) => {
expect(mark.attrs.styleId).toBeNull();
});
expect(runNodes.length).toBeGreaterThan(0);
runNodes.forEach((runNode) => {
expect(runNode.attrs.runProperties?.styleId).toBeNull();
});
},
);

it('preserves pre-existing underline after unsetLink', () => {
editor.commands.insertContent('link');
editor.commands.selectAll();
editor.commands.setUnderline();
editor.commands.setLink({ href: 'https://www.superdoc.dev' });
editor.commands.unsetLink();

let hasUnderline = false;
editor.state.doc.descendants((node) => {
if (!node.isText) return;
if (node.marks.some((mark) => mark.type.name === 'underline')) {
hasUnderline = true;
}
});

expect(hasUnderline).toBe(true);
});

it('removes underline on unsetLink when underline was not pre-existing', () => {
editor.commands.insertContent('link');
editor.commands.selectAll();
editor.commands.setLink({ href: 'https://www.superdoc.dev' });
editor.commands.unsetLink();

let hasUnderline = false;
editor.state.doc.descendants((node) => {
if (!node.isText) return;
if (node.marks.some((mark) => mark.type.name === 'underline')) {
hasUnderline = true;
}
});

expect(hasUnderline).toBe(false);
});

it('tests that the uploaded image has a rId and a relationship', async () => {
const blob = await fetch(imageBase64).then((res) => res.blob());
const file = new File([blob], 'image.png', { type: 'image/png' });
Expand Down
Loading