Skip to content

Commit 1130eee

Browse files
mttrbrtsclaude
andcommitted
fix(html): keep table caption CommonMark valid
Address Copilot review feedback on PR #640: - Flatten <caption> content to inline-only nodes before wrapping in Strong, so flow content (e.g. nested <p>, lists) can no longer become invalid block children of a Strong node. - Handle <caption> inside the table branch instead of a standalone rule, so a caption appearing outside a table no longer leaks a node without a $class into the output. - Emit the bolded caption as its own Paragraph block and drop the inline '\n\n' Text hack, leaving block-level spacing to the Markdown serializer. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Matt Roberts <code@rbrts.uk>
1 parent 37d3494 commit 1130eee

2 files changed

Lines changed: 94 additions & 31 deletions

File tree

packages/markdown-html/src/HtmlTransformer.test.js

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -107,16 +107,50 @@ describe('html table deserialization', () => {
107107
text: 'Employee Details'
108108
}
109109
]
110-
},
111-
{
112-
$class: `${commonmarkNamespace}.Text`,
113-
text: '\n\n'
114110
}
115111
]
116112
});
117113
expect(ciceroMarkDom.nodes[1].$class).toBe(`${commonmarkNamespace}.Table`);
118114
});
119115

116+
it('flattens block content in a caption to inline-only Strong children', () => {
117+
const ciceroMarkDom = htmlTransformer.toCiceroMark(`
118+
<table>
119+
<caption><p>See <em>also</em></p></caption>
120+
<thead>
121+
<tr><th>Name</th></tr>
122+
</thead>
123+
<tbody>
124+
<tr><td>Ada</td></tr>
125+
</tbody>
126+
</table>
127+
`);
128+
129+
expect(ciceroMarkDom.nodes).toHaveLength(2);
130+
const strong = ciceroMarkDom.nodes[0].nodes[0];
131+
expect(strong.$class).toBe(`${commonmarkNamespace}.Strong`);
132+
// every child of Strong must be inline (no Paragraph leaked in)
133+
strong.nodes.forEach(child => {
134+
expect(child.$class).not.toBe(`${commonmarkNamespace}.Paragraph`);
135+
});
136+
expect(strong.nodes).toEqual([
137+
{ $class: `${commonmarkNamespace}.Text`, text: 'See ' },
138+
{
139+
$class: `${commonmarkNamespace}.Emph`,
140+
nodes: [{ $class: `${commonmarkNamespace}.Text`, text: 'also' }]
141+
}
142+
]);
143+
expect(ciceroMarkDom.nodes[1].$class).toBe(`${commonmarkNamespace}.Table`);
144+
});
145+
146+
it('does not leak a caption node when it appears outside a table', () => {
147+
const ciceroMarkDom = htmlTransformer.toCiceroMark('<caption>Orphan caption</caption>');
148+
149+
// the old caption rule produced a node with no $class; ensure none leak
150+
ciceroMarkDom.nodes.forEach(n => expect(typeof n.$class).toBe('string'));
151+
expect(ciceroMarkDom.nodes.some(n => n.type === 'caption')).toBe(false);
152+
});
153+
120154
it('normalizes whitespace in table cells', () => {
121155
const ciceroMarkDom = htmlTransformer.toCiceroMark(`
122156
<table>

packages/markdown-html/src/rules.js

Lines changed: 56 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -498,16 +498,39 @@ const HTML_BLOCK_RULE = {
498498
}
499499
};
500500

501-
const CAPTION_RULE = {
502-
deserialize(el, next, ignoreSpace) {
503-
if (el.tagName && el.tagName.toLowerCase() === 'caption') {
504-
return {
505-
type: 'caption',
506-
nodes: next(el.childNodes, ignoreSpace)
507-
};
508-
}
501+
// CommonMark inline node classes. A <caption> may contain flow content
502+
// (paragraphs, lists, etc.), but a Strong node may only contain inline
503+
// children, so caption content is flattened to these classes before it is
504+
// wrapped in Strong.
505+
const INLINE_CLASSES = [
506+
'Text', 'Emph', 'Strong', 'Code', 'Link', 'Image',
507+
'Softbreak', 'Linebreak', 'HtmlInline'
508+
].map(name => `${CommonMarkModel.NAMESPACE}.${name}`);
509+
510+
/**
511+
* Flatten caption content to inline-only nodes so it can be safely wrapped in
512+
* a Strong node. Block-level wrappers (e.g. Paragraph) are unwrapped to their
513+
* inline children; nodes with no inline content are dropped.
514+
* @param {Array} nodes - the list of nodes
515+
* @returns {Array} - the inline-only list of nodes
516+
*/
517+
function toInlineNodes(nodes) {
518+
if (!nodes) {
519+
return [];
509520
}
510-
};
521+
const list = Array.isArray(nodes) ? nodes : [nodes];
522+
return list.reduce((acc, node) => {
523+
if (!node) {
524+
return acc;
525+
}
526+
if (INLINE_CLASSES.includes(node.$class)) {
527+
acc.push(node);
528+
} else if (node.nodes) {
529+
acc.push(...toInlineNodes(node.nodes));
530+
}
531+
return acc;
532+
}, []);
533+
}
511534

512535
/**
513536
* Clean table cell nodes by removing Softbreaks and normalizing whitespace.
@@ -570,7 +593,6 @@ const TABLE_RULE = {
570593
deserialize(el, next, ignoreSpace) {
571594
if (el.tagName && el.tagName.toLowerCase() === 'table') {
572595
const children = next(el.childNodes, ignoreSpace);
573-
const captionNode = children.find(c => c.type === 'caption');
574596
let tableNodes = children.filter(node =>
575597
node.$class === `${CommonMarkModel.NAMESPACE}.TableHead` ||
576598
node.$class === `${CommonMarkModel.NAMESPACE}.TableBody`
@@ -601,21 +623,29 @@ const TABLE_RULE = {
601623
nodes: tableNodes,
602624
};
603625

604-
if (captionNode) {
605-
const captionParagraph = {
606-
$class: `${CommonMarkModel.NAMESPACE}.Paragraph`,
607-
nodes: [
608-
{
609-
$class: `${CommonMarkModel.NAMESPACE}.Strong`,
610-
nodes: cleanTableNodes(captionNode.nodes)
611-
},
612-
{
613-
$class: `${CommonMarkModel.NAMESPACE}.Text`,
614-
text: '\n\n'
615-
}
616-
]
617-
};
618-
return [captionParagraph, table];
626+
// A <caption> is handled here (rather than via its own rule) so the
627+
// caption node never leaks into the output when it appears outside
628+
// of a table. Its content is flattened to inline-only nodes so the
629+
// Strong wrapper stays valid CommonMark, and the bolded caption is
630+
// emitted as its own Paragraph block before the table - block-level
631+
// spacing is left to the Markdown serializer.
632+
const captionElement = Array.from(el.childNodes).find(
633+
child => child.tagName && child.tagName.toLowerCase() === 'caption'
634+
);
635+
if (captionElement) {
636+
const captionNodes = cleanTableNodes(toInlineNodes(next(captionElement.childNodes, ignoreSpace)));
637+
if (captionNodes.length > 0) {
638+
const captionParagraph = {
639+
$class: `${CommonMarkModel.NAMESPACE}.Paragraph`,
640+
nodes: [
641+
{
642+
$class: `${CommonMarkModel.NAMESPACE}.Strong`,
643+
nodes: captionNodes
644+
}
645+
]
646+
};
647+
return [captionParagraph, table];
648+
}
619649
}
620650

621651
return table;
@@ -677,8 +707,7 @@ const rules = [
677707
HTML_INLINE_RULE,
678708
HTML_BLOCK_RULE,
679709
IMAGE_RULE,
680-
TABLE_RULE,
681-
CAPTION_RULE
710+
TABLE_RULE
682711
];
683712

684713

0 commit comments

Comments
 (0)