fix(block-editor): preserve unknown marks instead of blanking the field (#37175)

[adrianjm-dotCMS]

Closes the two acceptance criteria QA found short on #37175 — see the QA verification comment. The originally reported regression was already fixed by #37205 and stays fixed; this PR is the hardening half.

Proposed Changes

• AC5 — an unknown mark no longer aborts the document. fix(block-editor): register link, emoji and youtube regardless of Allowed Blocks (#37175) #37205 registered link, emoji and youtube, which fixed the two known offenders (link, highlight) but not the failure mode. Any mark the schema does not declare throws RangeError: There is no mark type X in this schema mid-recursion in Node.fromJSON. TipTap catches it, logs [tiptap warn]: Invalid content, and boots an empty document — so the field looks emptied while the stored JSON is intact, and the next save makes that loss real. Unknown nodes were already preserved as dotUnsupportedBlock; marks had no equivalent. Adds the mark-side twin dotUnsupportedMark (libs/dotcms-models/src/lib/unknown-block.util.ts), registered in both editors.
• Preserve rather than strip, mirroring the existing node path as QA suggested. The absence of a mark is usually transient: a customBlocks remote extension whose CDN is unreachable (remote-extensions.loader.ts drops failed loads and boots anyway — correctly), content written through the API or migrated from another CMS (textStyle/color/fontFamily are standard TipTap marks we don't register), or a version rollback. Stripping would turn a 20-minute outage into permanent loss for every author who saved during it.
• AC3 — linkOnPaste: false did not close the link-on-paste path. TipTap's Link returns its URL paste rule from addPasteRules() with no option guard, so pasting text containing a URL still created a link mark on a field where link isn't allowed; linkOnPaste only suppresses wrapping a selection. DotLink now overrides addPasteRules, mirroring how @tiptap/extension-youtube guards its own paste handler.
• Ordering invariant, documented at all three sites: nodes first, then marks. An unknown node is swallowed whole into the placeholder's originalNode attr, and that payload is inert data rather than part of the document tree — running the mark pass first would rewrite the very content the placeholder exists to preserve. There's a test pinning it.
• restoreUnknownBlockNodes now restores both halves, so each editor's single emit path picks it up with no new call sites on the save path.
• libs/new-block-editor/CLAUDE.md: new "Unknown nodes and marks (load-path invariant)" section + the mark inventory row.

Checklist

• Tests
• Translations — none needed, the placeholder renders no user-facing copy
• Security Implications Contemplated — no new input surface; the preserved payload is stored/re-emitted verbatim and never evaluated. JSON.parse of the data-original-mark attribute is already try/caught with a raw-string fallback, same as the node path.

Additional Info

Verified in Chromium against a local instance, with allowedBlocks set to the issue's exact list (bulletList,orderedList,codeBlock,table) and a fixture carrying an undeclared legalCitation mark, in both editors:

	Before (main)	After
Chars rendered on open	0	157
Paragraphs	1 (empty placeholder)	3 + node placeholder for heading
Console	RangeError: There is no mark type legalCitation in this schema	0 errors
Type one word → Save and Publish	995 → 203 bytes, marks []	995 → 1151 bytes, marks ['legalCitation','bold','link']
dotUnsupportedMark persisted to storage	—	no on the JSON-parse-success path — see footnote
Paste text containing a URL	new <a> created	no new <a>

Legacy editor confirmed by dot-old-block-editor in the DOM (feature flag forced via network interception, container config untouched). 203 bytes matches QA's reported 202 — same mechanism.

Footnote on "persisted to storage" — not an absolute guarantee, by design. When the preserved payload no longer parses, restoreUnknownMark keeps the placeholder so originalMarkRaw still round-trips, which means a dotUnsupportedMark carrying originalMarkRaw can reach storage. That mirrors dotUnsupportedBlock exactly and is pinned by keeps a placeholder whose payload is no longer valid during restore. So a placeholder in stored JSON is not a bug; a lost payload would be. Thanks to @rjvelazco for measuring the case the table glossed over.

Decision flagged for reviewers — resolved: preserve. Both reviewers landed on preserving rather than stripping, so no change. Recorded for anyone reading later: this registers a new schema entry, dotUnsupportedMark. Per libs/new-block-editor/CLAUDE.md → "TipTap Node Names Are Immutable", that name can never be changed once content exists. If that's too much of a commitment, the alternative is stripping unknown marks instead of preserving them — it also satisfies AC5 as written ("the affected text survives a save"), is ~20 lines instead of ~200, and the traversal and call sites are already in place.

Out of scope, confirmed by the issue's own scoping ("Scope: new Block Editor only… the legacy editor registers Link unconditionally", "The legacy editor never supported restricting links"): the legacy editor still auto-links pasted URLs, since it uses its own Link.extend in getEditorMarks(). Verified 1 → 2 anchors there. Closing that would change behaviour the ticket explicitly excludes.

Review feedback addressed (second commit):

• isJsonMark — isJsonContent was validating mark payloads. It works, but a tightened isJsonContent would break mark restoration silently, with a dotUnsupportedMark in storage as the only symptom.
• DotLink.addPasteRules now gates on autolink || linkOnPaste. The rule being suppressed is the auto-link-on-paste rule, closer to autolink; linkOnPaste: false + autolink: true is coherent and the old gate would have killed the linkifying. Pinned by a new test that configures the extension directly, since the assembly path cannot produce that combination.
• Clear-formatting limitation documented in createUnsupportedBlockMark and new-block-editor/CLAUDE.md: the toolbar runs unsetAllMarks().clearNodes(), which strips this placeholder along with every real mark — and clearNodes() does the same to dotUnsupportedBlock. Neither renders visible formatting, so the loss is silent and irreversible. Still strictly better than the document not loading, and unlike that, user-initiated. Both notes say the fix belongs to the clear-formatting command in each editor, not to the placeholders.
• Not changed, deliberately: memoizing the known-node/known-mark Sets. The cost is real (the guard runs per node selection via markViewDirty), but editorContentMatchesParsed is the one function this PR and Block Editor: embedded contentlet cannot be selected when text or other content precedes it #36985 both touch, so it belongs in that rewrite.

Also not addressed here, both flagged by QA as separate concerns:

• QA's finding Test Branch and Commit #2 — TypeError: o.can(...).mergeCells is not a function when table isn't in allowedBlocks. Pre-existing, untouched by fix(block-editor): register link, emoji and youtube regardless of Allowed Blocks (#37175) #37205 and by this PR; worth its own issue.
• libs/dotcms-models has no .spec.ts of its own, so the shared util is exercised from its consumers (new-block-editor, block-editor). No E2E added to apps/dotcms-ui-e2e.

Tests: nx test new-block-editor 137/137 · nx test block-editor --testPathPatterns=unknown-block 5/5 · tsc --noEmit clean on all three libs · nx lint new-block-editor / dotcms-models pass. nx test block-editor full suite has 10 pre-existing failures (this.editor.setEditable is not a function, an incomplete spec mock) — identical on clean main, verified by stashing.

This PR fixes: #37175

[claude]

Claude finished @adrianjm-dotCMS's task in 3m 20s —— View job

---

Review: preserve unknown marks (#37175)

• Read core changes (unknown-block.util.ts, editor components, link extension)
• Check for bugs, edge cases, ordering invariant
• Review tests
• Post findings

I traced the full load → edit → save round-trip in both editors, the ordering invariant, the paste-rule gate, and idempotency. The change is symmetric with the existing node path and the tests pin the behaviors that matter.

New Issues

No issues found.

Notes (non-blocking)

Round-trip is sound. preserveUnknownBlockMarks runs strictly after preserveUnknownBlockNodes at all three sites, and replaceUnknownMarksInNode short-circuits on UNKNOWN_BLOCK_NODE_NAME (unknown-block.util.ts:344-347), so marks inside a swallowed originalNode payload are never rewritten — pinned by the "never rewrites marks inside an unsupported-block payload" test. Restore is a single pass that returns the placeholder's originalNode before touching marks (unknown-block.util.ts:400-408), so the two never fight. Both preserve and restore are idempotent (dotUnsupportedMark is itself a known schema name, so a second preserve pass leaves it alone; a second restore finds no placeholder), which keeps the dedupe path in editorContentMatchesParsed from spuriously re-setting content.

The paste-rule gate is correct. has('link') drives both autolink and linkOnPaste (editor-extensions.ts:133-134), so addPasteRules returns [] exactly when link is not an allowed block. Gating on autolink || linkOnPaste rather than linkOnPaste alone is the right instinct and is pinned by the "only autolink enabled" test.

On the design question you raised (registering dotUnsupportedMark as a permanent schema name): preserve-over-strip is the right call, and the immutability cost is already paid — you're mirroring dotUnsupportedBlock, so this isn't a new kind of commitment, just a second instance of one the codebase already accepted. The name is descriptive and namespaced per the CLAUDE.md rule. Stripping would trade ~180 lines for permanent, silent data loss on every transient missing-extension window (CDN outage, migration, rollback) — exactly the failure classes your PR body enumerates. Keep the preserve approach.

Deferred limitation is acknowledged and correctly scoped. "Clear formatting" (unsetAllMarks().clearNodes()) strips both placeholders with no undo path — but that predates this PR on the node side and belongs to the clear-formatting command in each editor, not here. Documented in both the util JSDoc and libs/new-block-editor/CLAUDE.md. Worth its own follow-up issue.

Consistency checks that passed: signature change to preserveUnknownNodesInDocument (added required knownMarkNames) — all call sites updated; parseUnknownBlockOriginalMark mirrors parseUnknownBlockOriginalNode exactly, including the console.warn + raw-string fallback; legacy #knownEditorMarkNames is populated on the same editor-create lifecycle as the node set.

Solid, well-tested change. No blocking concerns.

· branch issue-37175-qa-feedback

[rjvelazco]

Approving with comments — nothing here should block

I reviewed this against a checkout of the branch rather than by reading, because I've spent the last while inside this exact load path on #36985 and wanted to check the edge cases rather than trust the diff. Every load-bearing claim in the PR description holds up. Four non-blocking comments inline, plus one correction to the description below.

What I verified on the branch

Real schema built from createEditorExtensions(...), real Editor instances, real transactions:

Probe	Result
Issue's exact allowedBlocks (bulletList,orderedList,codeBlock,table) → link and dotUnsupportedMark both in schema.marks	✅
Document with a link and an unknown legalCitation mark loads under that restriction, text intact ("a link and a citation")	✅
Two different unknown marks on the same text — both preserved, both restored, and they survive an edit elsewhere in the doc	✅
inclusive: false — typing at the boundary does not inherit the placeholder	✅
Applying bold over unknown-marked text keeps both marks	✅
Ordering invariant — an unknown mark inside a dotUnsupportedBlock payload is left byte-for-byte	✅
Idempotence — load → emit → re-load → emit is stable	✅
Paste rules: [RULE] unrestricted, [] when link isn't allowed	✅
Legacy emit path restores marks (dot-block-editor.component.ts:313, via the shared restoreUnknownBlockNodes) — no new call site needed, as claimed	✅

I went in expecting to find a problem with two same-type marks collapsing, since ProseMirror mark sets are normally exclusive per type. They don't — Mark.setFrom doesn't dedupe, and I confirmed both survive a subsequent transaction. Worth recording because it's the non-obvious part of the design and it works.

I also checked the initialisation ordering in the legacy editor, since an empty #knownEditorMarkNames at load would turn every mark into a placeholder. It's populated synchronously between new Editor(...) and subscribeToEditorEvents(), and writeValue guards on !this.editor, so there's no window. Safe.

One correction to the description

| dotUnsupportedMark persisted to storage | — | no |

Not quite — it can be, by design. When the payload doesn't survive parsing, restoreUnknownMark keeps the placeholder so originalMarkRaw still round-trips. Measured:

in : marks:[{type:'dotUnsupportedMark', attrs:{originalMark:null, originalMarkRaw:'not json'}}]
out: marks:[{type:'dotUnsupportedMark', attrs:{originalMark:null, originalMarkRaw:'not json'}}]   ← persisted

That's correct behaviour and it mirrors the node path exactly — but the table reads as an absolute guarantee, and a reviewer or QA taking it literally would treat a dotUnsupportedMark in stored JSON as a bug. Worth a footnote.

On the decision you flagged

Decision worth a reviewer's opinion: this registers a new schema entry, dotUnsupportedMark … the alternative is stripping unknown marks instead of preserving them.

Preserve. Two reasons, and I'd hold this position under push-back:

1. Your transience argument is the right one and it's stronger than the PR states. A CDN blip on a customBlocks remote extension is not a hypothetical — remote-extensions.loader.ts drops failed loads and boots anyway, correctly. Under the stripping design, a 20-minute outage silently converts to permanent, unrecoverable loss for every author who saved during it, with no error at any point. That's the same shape as the bug being fixed, just slower.
2. The immutability commitment is real but small. You're pinning one name forever. Weigh that against a defect class that has now surfaced three times (#37145 highlight, #37175 link, and this) — every recurrence being some schema entry the editor didn't know about. A permanent name is a cheap price for closing the class rather than the instance.

The ~200-vs-~20 line difference is not the deciding factor: most of those 200 lines are the symmetric twin of code that already exists and is already tested, which is about the cheapest kind of 200 lines there is.

Scope

I agree with both exclusions. The legacy auto-link behaviour is explicitly out of scope per the issue's own scoping, and QA's mergeCells finding is a genuinely separate pre-existing defect — worth its own issue so it doesn't get lost.

The one thing I'd want before merge

Nothing in the code. But the unsetAllMarks behaviour in my first inline comment is a real, measured way to lose the payload this PR protects, and it isn't covered by a test or a comment. A docblock line would satisfy me.

---

🤖 Reviewed by Claude on behalf of @rjvelazco

[adrianjm-dotCMS]

Thanks @rjvelazco — the two-same-type-marks probe and the legacy init-ordering check are both things I hadn't verified, and Mark.setFrom not deduping is the non-obvious part of the design. Good to have it recorded.

Your description correction is the most useful comment here. The table read as an absolute guarantee and it isn't: the raw-string fallback keeps the placeholder on purpose so originalMarkRaw round-trips, mirroring the node path, and it's already pinned by keeps a placeholder whose payload is no longer valid during restore. Purely my wording — someone in QA reading it literally would have filed a dotUnsupportedMark in stored JSON as a bug. Footnoted in the description.

Decision: preserve. Your second argument is the one I'll reuse — a permanent name is cheap against a defect class that has surfaced three times (#37145 highlight, #37175 link, this).

One thing neither review covered, since half the diff lives there: the legacy editor was verified at runtime, not just read. dot-old-block-editor confirmed in the DOM (flag forced via network interception, container config untouched), restricted field, mark preserved, link and bold intact, zero console errors. That's how the legacy paste behaviour we agree is out of scope turned up.

Pushed in 6eff8fc: isJsonMark, the paste gate on either flag plus the autolink-only test, and the clear-formatting limitation documented. Skipped the Set memoization for the reason in that thread. 137/137.