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
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.

Decision worth a reviewer's opinion: 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.

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 136/136 · 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 8s —— View job

---

Claude Code Review

I reviewed the diff against origin/main (the single PR commit). This is a clean, well-scoped fix. The mark-side placeholder faithfully mirrors the existing node path, the load-path ordering invariant (nodes → marks) is enforced and tested, and all four call sites of preserveUnknownNodesInDocument were updated with the new knownMarkNames arg.

New Issues

No blocking issues found.

Observations (non-blocking)

• 🟡 Medium: libs/dotcms-models/src/lib/unknown-block.util.ts:384 — Symmetric with the node path, but worth calling out explicitly: when parseUnknownBlockOriginalMark hits the raw-string fallback (malformed data-original-mark), restoreUnknownMark returns the placeholder unchanged, so a dotUnsupportedMark mark carrying originalMarkRaw does persist to storage on save. The PR's "persisted to storage: no" holds for the JSON-parse-success path (the common one); the raw fallback keeps the placeholder to preserve the recoverable string — the same documented trade-off as dotUnsupportedBlock. No change needed; just make sure that's the intended contract for marks too.

• 🟡 Medium: libs/dotcms-models/src/lib/unknown-block.util.ts:143 — renderUnknownBlockOriginalMark reuses isJsonContent() (a node predicate that only checks type is a string) to gate a mark payload. It works because a JSONMark also carries a type string, but the name reads as a type-mismatch. A isJsonMark-style guard (or a rename comment) would make the intent clearer. Cosmetic.

On your reviewer question — registering dotUnsupportedMark

I'd keep the preserve approach you shipped, not the strip alternative. Your rationale in the PR body is the deciding factor: the absence of a mark is usually transient (CDN-unreachable customBlocks, API/migrated content with standard TipTap marks like textStyle/color, a version rollback). Stripping turns a temporary outage into permanent, silent data loss for every author who saved during it — which is exactly the failure class #37175 is about. The immutable-name commitment is real but it's a name you fully control and never need to change; that's a smaller, well-understood cost than lossy saves. The symmetry with dotUnsupportedBlock is also worth preserving for maintainability — one mental model, one restore traversal.

Notes

• UnsupportedBlockMark is registered unconditionally in both editors (legacy getEditorMarks() is always spread into extensions at dot-block-editor.component.ts:265; new editor adds UnsupportedMark in createEditorExtensions), so Node.fromJSON can always resolve the placeholder it creates. ✅
• DotLink.addPasteRules() gating on linkOnPaste (AC3) matches the cited @tiptap/extension-youtube pattern and closes the paste-a-URL path that linkOnPaste: false alone left open. I could not read @tiptap/extension-link from node_modules in this environment to confirm its base addPasteRules shape, but the behavior matches TipTap's documented split between the paste rule and the selection-wrap plugin. Assumption: the base addPasteRules returns only the URL auto-link rule. What to verify: with link allowed and linkOnPaste: true (default), pasting still auto-links as before — the tests in editor-extensions.spec.ts cover the disabled path; a positive-path assertion would fully pin it.
• Test coverage is solid: mixed mark lists, missing/non-string type, the ordering invariant (marks never rewritten inside a dotUnsupportedBlock payload), and the raw-fallback restore case are all exercised.

Resolved

• ✅ AC5 (unknown mark blanking the field) and AC3 (paste-URL creating a link) are both addressed by the diff.

· 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

[rjvelazco]

inclusive: false is the right call and I verified it works — typing at the end of decorated text does not inherit the placeholder.

One behaviour worth a comment here, because it cuts against the premise of the whole PR: unsetAllMarks() destroys the preserved payload permanently. Measured on this branch, real editor, real schema:

load    : [{"type":"text","marks":[{"type":"legalCitation"}],"text":"cited"}]
selectAll().unsetAllMarks()
emit    : [{"type":"text","text":"cited"}]        ← legalCitation gone, unrecoverably

That is the toolbar's "clear formatting" button. An author who uses it on a paragraph they cannot even see the mark on (it renders neutral by design) silently discards a payload this PR exists to protect — and unlike the pre-PR blanking, there is no error and no way back.

I don't think it blocks: it is user-initiated, and it is still strictly better than today where the document doesn't load at all. But it is worth either a line in the docblock so the next reader knows, or excluding dotUnsupportedMark from unsetAllMarks. Your call — I'd take the comment as the minimum.

---

🤖 Reviewed by Claude on behalf of @rjvelazco

[rjvelazco]

Nit, but the kind that rots quietly: isJsonContent is being used to validate a mark payload.

It happens to work, because it only asserts typeof value.type === 'string' and a mark shares that shape. But the name now claims something untrue, and the coupling is invisible — if isJsonContent is ever tightened (say it starts requiring content, or rejects objects without a node type in the schema), mark restoration breaks silently and the failure mode is a dotUnsupportedMark written to storage, not an exception.

Same applies at renderUnknownBlockOriginalMark.

An isJsonMark alias next to isJsonContent — even a one-liner delegating to it — would pin the intent and cost nothing.

---

🤖 Reviewed by Claude on behalf of @rjvelazco

[rjvelazco]

Verified this does what the docblock says. Measured on this branch:

[unrestricted]        linkOnPaste=true  autolink=true  pasteRules=[RULE]
[restricted, no link] linkOnPaste=false autolink=false pasteRules=[]

One observation on the gate itself: the rule being suppressed is the autolink-on-paste rule, but it is gated on linkOnPaste, which the docblock correctly says governs wrapping the selection. Those are two different options being conflated.

It is harmless today only because editor-extensions.ts sets both from the same has('link'), so they can never disagree. The moment someone configures them independently — linkOnPaste: false, autolink: true, a reasonable combination meaning "don't wrap my selection but do linkify URLs I paste" — this silently disables the wrong thing.

this.options.autolink || this.options.linkOnPaste would survive that, or a sentence saying the two are deliberately treated as one gate here. Not blocking.

---

🤖 Reviewed by Claude on behalf of @rjvelazco

[rjvelazco]

Heads-up on a collision rather than a defect in this PR — worth knowing before either lands.

editorContentMatchesParsed is the exact function #36985 is about to rewrite (spec PR #37285). Two interactions:

1. This PR partly invalidates one of that spec's acceptance criteria. AC-008 there asserts "a body carrying a mark the schema can't deserialize still triggers setContent rather than blanking the field" — the conservative catch path. After this PR, on the object path, there is no such thing: every unknown mark becomes a placeholder and Node.fromJSON succeeds. That's a strict improvement, but I'll need to re-scope that AC. Good problem to have.

2. Two fresh Sets per call, on a hot path. getKnownNodeNames + getKnownMarkNames each do Object.keys() + new Set() on every invocation. The schema is immutable for the life of the editor, so this is recomputable-once work.

Normally I'd skip that as premature. I'm raising it because #36985 measured this guard running on every node selection — clicking an embedded contentlet re-runs the effect that calls it, via markViewDirty. So it's called far more often than "once per value push" suggests. Still small in absolute terms; genuinely optional, and arguably out of scope here.

No change requested. Flagging so we don't surprise each other at merge.

---

🤖 Reviewed by Claude on behalf of @rjvelazco