Skip to content

fix(links): link styles should be removed when tracked change is rejected #2465

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
palmer-cl wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix(links): link styles should be removed when tracked change is rejected #2465

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
8289f71
fix(links): link styles should be removed when tracked change link is…
palmer-cl Mar 19, 2026
6c73115
fix(track-changes): ensure textStyle attributes are preserved when re…
palmer-cl Mar 19, 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
14 changes: 13 additions & 1 deletion packages/layout-engine/layout-bridge/src/cache.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,22 +152,28 @@ const hashRuns = (block: FlowBlock): string => {
// (Fix for PR #1551: previously /\s+/g normalization caused cache collisions)
const text = 'text' in run && typeof run.text === 'string' ? run.text : '';

// Include formatting marks that affect measurement (mirroring paragraph approach)
// Include formatting marks that affect visual output (mirroring paragraph approach).
Copy link
Contributor

@caio-pizzol caio-pizzol Mar 19, 2026

Choose a reason for hiding this comment

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

this block is copy-pasted at line ~300. a small helper would mean future additions (like the underline/strike/link ones in this PR) only need one edit. worth it?

const bold = 'bold' in run ? run.bold : false;
const italic = 'italic' in run ? run.italic : false;
const underline = 'underline' in run ? run.underline : undefined;
const strike = 'strike' in run ? run.strike : false;
const color = 'color' in run ? run.color : undefined;
const fontSize = 'fontSize' in run ? run.fontSize : undefined;
const fontFamily = 'fontFamily' in run ? run.fontFamily : undefined;
const highlight = 'highlight' in run ? run.highlight : undefined;
const link = 'link' in run ? run.link : undefined;

// Build marks string including all formatting properties
const marks = [
bold ? 'b' : '',
italic ? 'i' : '',
underline ? `u:${JSON.stringify(underline)}` : '',
strike ? 's' : '',
color ?? '',
fontSize !== undefined ? `fs:${fontSize}` : '',
fontFamily ? `ff:${fontFamily}` : '',
highlight ? `hl:${highlight}` : '',
link ? `ln:${JSON.stringify(link)}` : '',
].join('');

// Use type guard to safely access comment metadata
Expand Down Expand Up @@ -293,17 +299,23 @@ const hashRuns = (block: FlowBlock): string => {
const text = 'src' in run || run.kind === 'lineBreak' || run.kind === 'break' ? '' : (run.text ?? '');
const bold = 'bold' in run ? run.bold : false;
const italic = 'italic' in run ? run.italic : false;
const underline = 'underline' in run ? run.underline : undefined;
const strike = 'strike' in run ? run.strike : false;
const color = 'color' in run ? run.color : undefined;
const fontSize = 'fontSize' in run ? run.fontSize : undefined;
const fontFamily = 'fontFamily' in run ? run.fontFamily : undefined;
const highlight = 'highlight' in run ? run.highlight : undefined;
const link = 'link' in run ? run.link : undefined;
const marks = [
bold ? 'b' : '',
italic ? 'i' : '',
underline ? `u:${JSON.stringify(underline)}` : '',
strike ? 's' : '',
color ?? '',
fontSize !== undefined ? `fs:${fontSize}` : '',
fontFamily ? `ff:${fontFamily}` : '',
highlight ? `hl:${highlight}` : '',
link ? `ln:${JSON.stringify(link)}` : '',
].join('');

// Include tracked change metadata in hash
Expand Down
30 changes: 20 additions & 10 deletions packages/layout-engine/layout-bridge/src/diff.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -404,25 +404,35 @@ const paragraphBlocksEqual = (a: FlowBlock & { kind: 'paragraph' }, b: FlowBlock
for (let i = 0; i < a.runs.length; i += 1) {
const runA = a.runs[i];
const runB = b.runs[i];
if (
('src' in runA || runA.kind === 'lineBreak' || runA.kind === 'break' || runA.kind === 'fieldAnnotation'
const leftText =
'src' in runA || runA.kind === 'lineBreak' || runA.kind === 'break' || runA.kind === 'fieldAnnotation'
? ''
: runA.text) !==
('src' in runB || runB.kind === 'lineBreak' || runB.kind === 'break' || runB.kind === 'fieldAnnotation'
? ''
: runB.text) ||
: runA.text;
const rightText =
'src' in runB || runB.kind === 'lineBreak' || runB.kind === 'break' || runB.kind === 'fieldAnnotation'
? ''
: runB.text;
const leftUnderline = JSON.stringify('underline' in runA ? runA.underline : undefined);
const rightUnderline = JSON.stringify('underline' in runB ? runB.underline : undefined);
const leftLink = JSON.stringify('link' in runA ? runA.link : undefined);
const rightLink = JSON.stringify('link' in runB ? runB.link : undefined);

const mismatch =
leftText !== rightText ||
fieldAnnotationKey(runA) !== fieldAnnotationKey(runB) ||
('bold' in runA ? runA.bold : false) !== ('bold' in runB ? runB.bold : false) ||
('italic' in runA ? runA.italic : false) !== ('italic' in runB ? runB.italic : false) ||
leftUnderline !== rightUnderline ||
('strike' in runA ? runA.strike : false) !== ('strike' in runB ? runB.strike : false) ||
('color' in runA ? runA.color : undefined) !== ('color' in runB ? runB.color : undefined) ||
('fontSize' in runA ? runA.fontSize : undefined) !== ('fontSize' in runB ? runB.fontSize : undefined) ||
('fontFamily' in runA ? runA.fontFamily : undefined) !== ('fontFamily' in runB ? runB.fontFamily : undefined) ||
('highlight' in runA ? runA.highlight : undefined) !== ('highlight' in runB ? runB.highlight : undefined) ||
leftLink !== rightLink ||
getTrackedChangeKey(runA) !== getTrackedChangeKey(runB) ||
getCommentKey(runA) !== getCommentKey(runB)
) {
return false;
}
getCommentKey(runA) !== getCommentKey(runB);

if (mismatch) return false;
}
return true;
};
Expand Down
97 changes: 97 additions & 0 deletions packages/layout-engine/layout-bridge/test/cache.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -941,6 +941,103 @@ describe('MeasureCache', () => {
expect(cache.get(table2, 800, 600)).toBeUndefined();
});

it('invalidates cache when underline changes in table cells', () => {
const table1: TableBlock = {
kind: 'table',
id: 'table-underline',
rows: [
{
id: 'row-0',
cells: [
{
id: 'cell-0',
paragraph: {
kind: 'paragraph',
id: 'para-0',
runs: [{ text: 'Hello', fontFamily: 'Arial', fontSize: 12, underline: { style: 'single' } }],
},
},
],
},
],
};

const table2: TableBlock = {
kind: 'table',
id: 'table-underline',
rows: [
{
id: 'row-0',
cells: [
{
id: 'cell-0',
paragraph: {
kind: 'paragraph',
id: 'para-0',
runs: [{ text: 'Hello', fontFamily: 'Arial', fontSize: 12 }],
},
},
],
},
],
};

cache.set(table1, 800, 600, { totalHeight: 50 });
expect(cache.get(table2, 800, 600)).toBeUndefined();
});

it('invalidates cache when hyperlink mark changes in table cells', () => {
const table1: TableBlock = {
kind: 'table',
id: 'table-link',
rows: [
{
id: 'row-0',
cells: [
{
id: 'cell-0',
paragraph: {
kind: 'paragraph',
id: 'para-0',
runs: [
{
text: 'Hello',
fontFamily: 'Arial',
fontSize: 12,
link: { href: 'https://example.com' } as any,
},
],
},
},
],
},
],
};

const table2: TableBlock = {
kind: 'table',
id: 'table-link',
rows: [
{
id: 'row-0',
cells: [
{
id: 'cell-0',
paragraph: {
kind: 'paragraph',
id: 'para-0',
runs: [{ text: 'Hello', fontFamily: 'Arial', fontSize: 12 }],
},
},
],
},
],
};

cache.set(table1, 800, 600, { totalHeight: 50 });
expect(cache.get(table2, 800, 600)).toBeUndefined();
});

it('creates cache hit when table formatting is identical', () => {
const table1: TableBlock = {
kind: 'table',
Expand Down
45 changes: 45 additions & 0 deletions packages/layout-engine/layout-bridge/test/diff.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,51 @@ describe('computeDirtyRegions', () => {
expect(result.firstDirtyIndex).toBe(0);
});

it('detects underline changes', () => {
const prev = [
{
kind: 'paragraph' as const,
id: '0-paragraph',
runs: [{ text: 'Hello', fontFamily: 'Arial', fontSize: 12, underline: { style: 'single' as const } }],
},
];
const next = [
{
kind: 'paragraph' as const,
id: '0-paragraph',
runs: [{ text: 'Hello', fontFamily: 'Arial', fontSize: 12 }],
},
];
const result = computeDirtyRegions(prev, next);
expect(result.firstDirtyIndex).toBe(0);
});

it('detects hyperlink changes', () => {
const prev = [
{
kind: 'paragraph' as const,
id: '0-paragraph',
runs: [
{
text: 'Hello',
fontFamily: 'Arial',
fontSize: 12,
link: { href: 'https://example.com' } as any,
},
],
},
];
const next = [
{
kind: 'paragraph' as const,
id: '0-paragraph',
runs: [{ text: 'Hello', fontFamily: 'Arial', fontSize: 12 }],
},
];
const result = computeDirtyRegions(prev, next);
expect(result.firstDirtyIndex).toBe(0);
});

it('treats identical fontSize and fontFamily as stable', () => {
const prev = [
{
Expand Down
10 changes: 10 additions & 0 deletions packages/super-editor/src/extensions/run/calculateInlineRunPropertiesPlugin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ const RUN_PROPERTIES_DERIVED_FROM_MARKS = new Set([
'position',
]);

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

const RUN_PROPERTY_PRESERVE_META_KEY = 'sdPreserveRunPropertiesKeys';

/**
Expand Down Expand Up @@ -384,6 +386,14 @@ function getInlineRunProperties(
if (existingRunProperties != null) {
Object.keys(existingRunProperties).forEach((key) => {
if (RUN_PROPERTIES_DERIVED_FROM_MARKS.has(key) && !preservedDerivedKeys.has(key)) return;
if (
key === 'styleId' &&
TRANSIENT_HYPERLINK_STYLE_IDS.has(existingRunProperties[key]) &&
(runPropertiesFromMarks.styleId == null || runPropertiesFromMarks.styleId === '')
) {
// Link-derived character styles must not survive after link/textStyle marks are removed.
return;
}
if (key in inlineRunProperties) return;
if (existingRunProperties[key] === undefined) return;
inlineRunProperties[key] = existingRunProperties[key];
Expand Down
17 changes: 17 additions & 0 deletions packages/super-editor/src/extensions/run/calculateInlineRunPropertiesPlugin.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,23 @@ describe('calculateInlineRunPropertiesPlugin', () => {
expect(runNode?.attrs.runProperties).toEqual({ styleId: 'Style1' });
});

it('drops stale Hyperlink styleId after hyperlink marks are removed', () => {
decodeRPrFromMarksMock.mockImplementation(() => ({}));
resolveRunPropertiesMock.mockImplementation(() => ({}));

const schema = makeSchema();
const boldMark = schema.marks.bold.create();
const doc = paragraphDoc(schema, { runProperties: { bold: true, styleId: 'Hyperlink' } }, [boldMark]);
const state = createState(schema, doc);
const { from, to } = runTextRange(state.doc, 0, doc.textContent.length);

const tr = state.tr.removeMark(from, to, schema.marks.bold);
const { state: nextState } = state.applyTransaction(tr);

const runNode = nextState.doc.nodeAt(runPos(nextState.doc) ?? 0);
expect(runNode?.attrs.runProperties).toBeNull();
});

it('does not carry over mark-derived properties from existing runProperties', () => {
decodeRPrFromMarksMock.mockImplementation(() => ({ fontFamily: { ascii: 'Arial' } }));
resolveRunPropertiesMock.mockImplementation(() => ({ fontFamily: { ascii: 'Arial' } }));
Expand Down
Loading
Loading