. The
+ // schema div-form is still emitted on the raw-HTML path (blockToHtml),
+ // because DOM parsers drop comment nodes inside columns/cells.
+ return standaloneCommentFor("pagebreak");
- case "subpages": {
- // Emit the schema-matching div[data-type="subpages"] so marked passes it
- // through as a block and generateJSON rebuilds the subpages atom. The old
- // `{{SUBPAGES}}` literal had no parseHTML inverse, so on import it stayed
- // as plain text — the embed rendered as the literal "{{SUBPAGES}}" on the
- // page after a round-trip (red-team: subpages round-trip data loss).
- // `data-recursive` carries the recursive toggle so it round-trips too.
- const recursive = node.attrs?.recursive ? ` data-recursive="true"` : "";
- return `
`;
- }
+ case "subpages":
+ // #293 canon #5: a subpages block serializes as a STANDALONE comment —
+ // `` by default, or ``
+ // when the recursive toggle is set. Same rationale as pageBreak: readable
+ // markdown, invisible in renderers, re-materialized on import. The div
+ // form (`
`) is retained on the raw-HTML path
+ // (blockToHtml) since comments cannot survive a DOM parse inside columns.
+ return node.attrs?.recursive
+ ? standaloneCommentFor("subpages", { recursive: true })
+ : standaloneCommentFor("subpages");
case "status": {
// Inline status pill. The schema reads the label from the element's
@@ -1033,6 +1039,19 @@ export function convertProseMirrorToMarkdown(content: any): string {
// marked and would round-trip as literal "| a | b |" text (review #7).
case "table":
return tableToHtml(block.content || []);
+ // #293 canon #5: on the TOP-LEVEL path (processNode) subpages/pageBreak
+ // serialize as standalone `` comments, but a comment node is
+ // discarded by the DOM parse stage (jsdom/parse5) that reads back a raw-
+ // HTML block — so inside a column/cell the comment form would silently
+ // vanish (latent data loss; these atoms previously fell through to the
+ // `default`
). Here we KEEP the schema-matching div-form so the
+ // node survives the raw-HTML round trip.
+ case "pageBreak":
+ return `
`;
+ case "subpages": {
+ const recursive = block.attrs?.recursive ? ` data-recursive="true"` : "";
+ return `
`;
+ }
// columns/column, math, media, embed, attachment, mention, etc. already
// emit schema-matching HTML from processNode.
case "columns":
diff --git a/packages/prosemirror-markdown/src/lib/markdown-to-prosemirror.ts b/packages/prosemirror-markdown/src/lib/markdown-to-prosemirror.ts
index 73ed5eb1..f401781b 100644
--- a/packages/prosemirror-markdown/src/lib/markdown-to-prosemirror.ts
+++ b/packages/prosemirror-markdown/src/lib/markdown-to-prosemirror.ts
@@ -329,36 +329,87 @@ function bridgeTaskLists(html: string): string {
* comment it re-expresses the encoded attributes in a form the schema's
* parseHTML already understands, then removes the comment so it cannot leak.
*
- * For #9 the only handled name is `attrs`, and the only handled key is
- * `textAlign` on a paragraph/heading: it is written as an inline
- * `text-align` style on the parent element, which the docmost-schema textAlign
- * global attribute reads back. Fail-open everywhere: a malformed comment (null
- * from parseAttachedComment), an unknown name, a comment whose parent is not a
- * `
`/``, or an unknown/empty attr value is left inert (the comment is
- * dropped by generateJSON anyway). Future decisions (#4/#8) extend the
- * per-name handling here without changing the parse primitive.
+ * This pass materializes BOTH comment conventions, discriminated by position:
+ *
+ * - ATTACHED comments (#9 `attrs`): a comment sitting INSIDE a ``/``
+ * (same rendered line as visible content). The only handled key is
+ * `textAlign`, re-expressed as an inline `text-align` style on the parent,
+ * which the docmost-schema textAlign global attribute reads back.
+ * - STANDALONE machinery comments (#5 `subpages`/`pagebreak`): a lone comment
+ * line, which `marked` renders as an HTML block so jsdom makes it a DIRECT
+ * child of ``. These are replaced with the schema-matching block div
+ * (`` / `
`)
+ * that the schema's parseHTML rebuilds into the atom.
+ *
+ * Position determines legality: an `attrs` comment is honored only in attached
+ * position, a `subpages`/`pagebreak` comment only in standalone position; a
+ * comment in the wrong position is left INERT (generateJSON drops it). Fail-open
+ * everywhere: a malformed comment (null from parseAttachedComment), an unknown
+ * name, a wrong-position comment, or an unknown/empty attr value is ignored.
+ * Future decisions (#4/#8) extend the per-name handling here without changing
+ * the parse primitive.
*/
-function applyAttachedComments(html: string): string {
+function applyCommentDirectives(html: string): string {
// Cheap early-out: no comments at all -> nothing to intercept.
if (!html.includes("
+// — invisible in any markdown renderer, yet round-tripping (the importer
+// materializes them back into the block atom before generateJSON drops the
+// comment). Position determines legality: they are honored ONLY standalone; a
+// comment attached after visible text is INERT. Inside a raw-HTML container
+// (columns/cells) the DOM parse stage discards comment nodes, so there the
+// schema `
` form is emitted instead. These tests assert the
+// EXACT emitted markdown and a lossless round trip (non-vacuous).
+
+const doc = (...nodes: any[]) => ({ type: 'doc', content: nodes });
+const text = (t: string) => ({ type: 'text', text: t });
+const para = (...inline: any[]) => ({ type: 'paragraph', content: inline });
+
+// Recursively collect every node type present in a doc.
+const collectTypes = (n: any, set = new Set
()): Set => {
+ if (!n || typeof n !== 'object') return set;
+ if (n.type) set.add(n.type);
+ if (Array.isArray(n.content)) n.content.forEach((c: any) => collectTypes(c, set));
+ return set;
+};
+
+// Find the subpages node anywhere in a doc (for attribute assertions).
+const findNode = (n: any, type: string): any => {
+ if (!n || typeof n !== 'object') return undefined;
+ if (n.type === type) return n;
+ if (Array.isArray(n.content)) {
+ for (const c of n.content) {
+ const hit = findNode(c, type);
+ if (hit) return hit;
+ }
+ }
+ return undefined;
+};
+
+describe('standaloneCommentFor primitive (#293 #5)', () => {
+ it('emits a name-only comment when there are no attrs', () => {
+ expect(standaloneCommentFor('pagebreak')).toBe('');
+ expect(standaloneCommentFor('subpages')).toBe('');
+ expect(standaloneCommentFor('subpages', {})).toBe('');
+ expect(standaloneCommentFor('subpages', null)).toBe('');
+ });
+
+ it('emits a compact JSON body when attrs are present', () => {
+ expect(standaloneCommentFor('subpages', { recursive: true })).toBe(
+ '',
+ );
+ });
+
+ it('shares the attached encoder `--` escaping (payload cannot close early)', () => {
+ const s = standaloneCommentFor('subpages', { note: 'a--b' });
+ expect(s).toBe('');
+ // No premature `--` inside the payload -> the comment cannot terminate early.
+ expect(s.slice(''.length)).not.toContain('--');
+ });
+});
+
+describe('subpages standalone serialization (#293 #5)', () => {
+ it('default subpages -> exactly ', () => {
+ expect(convertProseMirrorToMarkdown(doc({ type: 'subpages' }))).toBe('');
+ });
+
+ it('markdown -> a subpages node, byte-stable re-export', async () => {
+ const md = '';
+ const doc2 = await markdownToProseMirror(md);
+ expect(collectTypes(doc2).has('subpages')).toBe(true);
+ expect(convertProseMirrorToMarkdown(doc2)).toBe(md);
+ });
+
+ it('recursive subpages -> and round-trips recursive:true', async () => {
+ const md = convertProseMirrorToMarkdown(
+ doc({ type: 'subpages', attrs: { recursive: true } }),
+ );
+ expect(md).toBe('');
+ const doc2 = await markdownToProseMirror(md);
+ const node = findNode(doc2, 'subpages');
+ expect(node).toBeTruthy();
+ expect(node.attrs.recursive).toBe(true);
+ // Byte-stable second export closes the loop.
+ expect(convertProseMirrorToMarkdown(doc2)).toBe(md);
+ });
+});
+
+describe('pageBreak standalone serialization (#293 #5)', () => {
+ it('pageBreak -> exactly ', () => {
+ expect(convertProseMirrorToMarkdown(doc({ type: 'pageBreak' }))).toBe('');
+ });
+
+ it('markdown round-trips to a pageBreak node, byte-stable', async () => {
+ const md = '';
+ const doc2 = await markdownToProseMirror(md);
+ expect(collectTypes(doc2).has('pageBreak')).toBe(true);
+ expect(convertProseMirrorToMarkdown(doc2)).toBe(md);
+ });
+});
+
+describe('subpages inside a column uses the div-form, not a comment (#293 #5)', () => {
+ // A column is a raw-HTML block: the DOM parse stage discards comment nodes, so
+ // a comment inside it would silently vanish. The converter MUST emit the
+ // schema div-form there instead.
+ const columnsDoc = doc({
+ type: 'columns',
+ content: [
+ { type: 'column', attrs: { width: '50%' }, content: [{ type: 'subpages' }] },
+ { type: 'column', attrs: { width: '50%' }, content: [para(text('side'))] },
+ ],
+ });
+
+ it('serializes the column subpages as , not ', () => {
+ const md = convertProseMirrorToMarkdown(columnsDoc);
+ expect(md).toContain('data-type="subpages"');
+ // The bare standalone comment must NOT appear inside the raw-HTML column.
+ expect(md).not.toContain('');
+ });
+
+ it('round-trips back to a subpages node still inside a column', async () => {
+ const md = convertProseMirrorToMarkdown(columnsDoc);
+ const doc2 = await markdownToProseMirror(md);
+ const column = findNode(doc2, 'column');
+ expect(column).toBeTruthy();
+ expect(collectTypes(column).has('subpages')).toBe(true);
+ });
+});
+
+describe('position legality / fail-open (#293 #5)', () => {
+ it('an ATTACHED after paragraph text is INERT (no subpages node)', async () => {
+ const doc2 = await markdownToProseMirror('para text ');
+ expect(collectTypes(doc2).has('subpages')).toBe(false);
+ // The paragraph text survives, and no comment marker leaks into the body.
+ expect(JSON.stringify(doc2)).not.toContain(' is INERT (no crash, no subpages node)', async () => {
+ const doc2 = await markdownToProseMirror('');
+ expect(collectTypes(doc2).has('subpages')).toBe(false);
+ });
+});
+
+describe('multi-node document order across standalone comments (#293 #5)', () => {
+ // The riskiest part of the parser change: a LEADING standalone comment is
+ // parsed at document level (outside ) and must be re-inserted into the
+ // body in document order, interleaved correctly with real block content. A
+ // MID-document comment (pageBreak here) exercises the in-body branch. This
+ // locks the ordering the review flagged as covered only by manual checks.
+ const topTypes = (d: any) => (d.content || []).map((n: any) => n.type);
+
+ it('leading + mid + trailing standalone comments keep document order', async () => {
+ const d = doc(
+ { type: 'subpages' }, // leading -> parsed at document level
+ para(text('a')),
+ { type: 'pageBreak' }, // mid -> parsed in-body
+ para(text('b')),
+ );
+ const md = convertProseMirrorToMarkdown(d);
+ expect(md).toBe('\n\na\n\n\n\nb');
+ const d2 = await markdownToProseMirror(md);
+ // Order must be preserved exactly, not just membership.
+ expect(topTypes(d2)).toEqual([
+ 'subpages',
+ 'paragraph',
+ 'pageBreak',
+ 'paragraph',
+ ]);
+ // And byte-stable on re-export.
+ expect(convertProseMirrorToMarkdown(d2)).toBe(md);
+ });
+
+ it('two leading standalone comments keep their relative order', async () => {
+ const d = doc({ type: 'subpages' }, { type: 'pageBreak' }, para(text('x')));
+ const md = convertProseMirrorToMarkdown(d);
+ expect(md).toBe('\n\n\n\nx');
+ const d2 = await markdownToProseMirror(md);
+ expect(topTypes(d2)).toEqual(['subpages', 'pageBreak', 'paragraph']);
+ expect(convertProseMirrorToMarkdown(d2)).toBe(md);
+ });
+
+ it('a trailing standalone comment stays last', async () => {
+ const d = doc(para(text('x')), { type: 'subpages' });
+ const md = convertProseMirrorToMarkdown(d);
+ expect(md).toBe('x\n\n');
+ const d2 = await markdownToProseMirror(md);
+ expect(topTypes(d2)).toEqual(['paragraph', 'subpages']);
+ expect(convertProseMirrorToMarkdown(d2)).toBe(md);
+ });
+});
diff --git a/packages/prosemirror-markdown/test/markdown-converter-gaps.test.ts b/packages/prosemirror-markdown/test/markdown-converter-gaps.test.ts
index f3e69944..b3d8839b 100644
--- a/packages/prosemirror-markdown/test/markdown-converter-gaps.test.ts
+++ b/packages/prosemirror-markdown/test/markdown-converter-gaps.test.ts
@@ -36,29 +36,30 @@ async function roundTrip(node: any): Promise<{ md1: string; doc2: any; md2: stri
// existing documented `it.fails` bugs in markdown-roundtrip.property.test.ts).
// ---------------------------------------------------------------------------
describe('pageBreak data loss (no converter case — SPEC §11 divergence)', () => {
- it('exports a pageBreak node to the schema-matching block div', () => {
- // FIXED: a standalone pageBreak now emits the block-level HTML div so the
- // node survives instead of being erased to "".
+ it('exports a pageBreak node to the standalone comment (#293 #5)', () => {
+ // #293 canon #5: a standalone pageBreak now serializes as the readable,
+ // renderer-invisible comment `` (re-materialized on import),
+ // instead of the earlier raw
block.
expect(convertProseMirrorToMarkdown(doc({ type: 'pageBreak' }))).toBe(
- '
',
+ '',
);
});
it('keeps a pageBreak sitting BETWEEN two paragraphs on export', () => {
- // FIXED: with surrounding content the divider is emitted as its own block
+ // With surrounding content the divider is emitted as its own comment line
// between the two paragraphs (joined by the doc "\n\n"), no longer dropped.
const out = convertProseMirrorToMarkdown(
doc(para(text('before')), { type: 'pageBreak' }, para(text('after'))),
);
expect(out).toBe(
- 'before\n\n
\n\nafter',
+ 'before\n\n\n\nafter',
);
- expect(out).toContain('pageBreak');
+ expect(out).toContain('');
});
// FIXED: a pageBreak node now survives an export -> import -> export cycle
- // because the FIRST export emits the schema-matching block div, which marked
- // passes through and generateJSON rebuilds into a pageBreak node again.
+ // because the FIRST export emits the standalone comment, which the importer
+ // materializes back into a pageBreak node again.
it('a pageBreak node round-trips (export -> import yields a pageBreak)', async () => {
const { md1, doc2 } = await roundTrip({ type: 'pageBreak' });
expect(md1).not.toBe('');
@@ -68,18 +69,18 @@ describe('pageBreak data loss (no converter case — SPEC §11 divergence)', ()
});
// ---------------------------------------------------------------------------
-// 2. subpages round-trip (`case "subpages"` emits the schema-matching div).
+// 2. subpages round-trip (#293 #5 standalone comment).
//
// It used to emit the literal `{{SUBPAGES}}`, which has no markdown/HTML meaning,
// so on re-import the subpages BLOCK came back as a plain PARAGRAPH carrying the
// literal string (the embed rendered as visible "{{SUBPAGES}}" text on the page
-// after a sync — data loss). It now emits `
` like the
-// other embed nodes, so the schema's parseHTML rebuilds the subpages node.
+// after a sync — data loss). Per canon #5 it now emits the standalone comment
+// ``, which the importer materializes back into a subpages node.
// ---------------------------------------------------------------------------
-describe('subpages round-trip (schema-matching div)', () => {
- it('emits the subpages div and re-imports as a subpages node (no literal leak)', async () => {
+describe('subpages round-trip (standalone comment #293 #5)', () => {
+ it('emits the subpages comment and re-imports as a subpages node (no literal leak)', async () => {
const { md1, doc2 } = await roundTrip({ type: 'subpages' });
- expect(md1).toBe('
');
+ expect(md1).toBe('');
const collect = (n: any): string[] => [
n.type,
diff --git a/packages/prosemirror-markdown/test/markdown-converter-golden.test.ts b/packages/prosemirror-markdown/test/markdown-converter-golden.test.ts
index 0a4b69c0..2a8ca1eb 100644
--- a/packages/prosemirror-markdown/test/markdown-converter-golden.test.ts
+++ b/packages/prosemirror-markdown/test/markdown-converter-golden.test.ts
@@ -147,8 +147,8 @@ describe('paragraph.textAlign -> attached comment (#293 #9)', () =>
});
describe('subpages token + unknown-in-container fallback', () => {
- it('subpages emits the schema-matching div (round-trips, unlike the old {{SUBPAGES}} literal)', () => {
- expect(c({ type: 'subpages' })).toBe('
');
+ it('subpages emits the standalone comment (#293 #5, unlike the old {{SUBPAGES}} literal)', () => {
+ expect(c({ type: 'subpages' })).toBe('');
});
it('an unknown block inside a raw-HTML container is wrapped in
(never markdown)', () => {