fix: Editor math block parsing and NaN media dimensions (#12668)

* fix: Block math not closed by trailing $$ on a content line

The closing delimiter check compared a 3-character slice against the
2-character "$$" delimiter, so block math closed on the same line as
content (e.g. "c = d$$") was never detected and the block swallowed the
rest of the document. Use the delimiter length rather than a hardcoded
slice. Also fix the indexOf sentinel comparison (!== 1 instead of
!== -1) in inline math parsing, which terminated correctly only by
coincidence.

Adds tests for the math markdown rules and moves the findNodes test
helper into shared/test/editor for reuse.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix: NaN width and height parsed for video and image nodes

Video parseDOM and parseMarkdown used parseInt on a missing attribute,
storing NaN instead of null and persisting it to markdown as NaNxNaN.
Image size syntax with a missing dimension (e.g. "=x100") hit the same
issue through optional regex groups. Parse dimensions only when
present, matching the existing guard in Image parseDOM, and correct the
video getAttrs element type.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix: Normalize non-numeric video dimensions, avoid serializing nullxnull

Review feedback: parseInt could still produce NaN when the attribute
exists but is not numeric (e.g. width="auto"), and toMarkdown wrote
null dimensions as "nullxnull". Parse dimensions through a helper that
normalizes non-finite values to null, and serialize nullish dimensions
as empty strings, which still round-trips as a video node.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* test

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>

shared/editor/nodes/Image.tsx

@@ -52,8 +52,8 @@ const parseTitleAttribute = (tokenTitle: string): TitleAttributes => {
 
   const match = tokenTitle.match(imageSizeRegex);
   if (match) {
-    attributes.width = parseInt(match[1], 10);
-    attributes.height = parseInt(match[2], 10);
+    attributes.width = match[1] ? parseInt(match[1], 10) : undefined;
+    attributes.height = match[2] ? parseInt(match[2], 10) : undefined;
     tokenTitle = tokenTitle.replace(imageSizeRegex, "");
   }
 

shared/editor/nodes/Video.tsx

@@ -17,6 +17,11 @@ import attachmentsRule from "../rules/links";
 import type { ComponentProps } from "../types";
 import Node from "./Node";
 
+const parseDimension = (value: string | null): number | null => {
+  const parsed = parseInt(value ?? "", 10);
+  return Number.isFinite(parsed) ? parsed : null;
+};
+
 export default class Video extends Node {
   get name() {
     return "video";
@@ -56,12 +61,12 @@ export default class Video extends Node {
         {
           priority: 100,
           tag: "video",
-          getAttrs: (dom: HTMLAnchorElement) => ({
+          getAttrs: (dom: HTMLVideoElement) => ({
             id: dom.id,
             title: dom.getAttribute("title"),
             src: dom.getAttribute("src"),
-            width: parseInt(dom.getAttribute("width") ?? "", 10),
-            height: parseInt(dom.getAttribute("height") ?? "", 10),
+            width: parseDimension(dom.getAttribute("width")),
+            height: parseDimension(dom.getAttribute("height")),
           }),
         },
       ],
@@ -184,7 +189,9 @@ export default class Video extends Node {
   toMarkdown(state: MarkdownSerializerState, node: ProsemirrorNode) {
     state.ensureNewLine();
     state.write(
-      `[${node.attrs.title} ${node.attrs.width}x${node.attrs.height}](${node.attrs.src})\n\n`
+      `[${node.attrs.title} ${node.attrs.width ?? ""}x${
+        node.attrs.height ?? ""
+      }](${node.attrs.src})\n\n`
     );
     state.ensureNewLine();
   }
@@ -195,8 +202,8 @@ export default class Video extends Node {
       getAttrs: (tok: Token) => ({
         src: tok.attrGet("src"),
         title: tok.attrGet("title"),
-        width: parseInt(tok.attrGet("width") ?? "", 10),
-        height: parseInt(tok.attrGet("height") ?? "", 10),
+        width: parseDimension(tok.attrGet("width")),
+        height: parseDimension(tok.attrGet("height")),
       }),
     };
   }

shared/editor/rules/checkboxes.test.ts

@@ -1,4 +1,4 @@
-import { extensionManager, schema } from "../../test/editor";
+import { extensionManager, findNodes, schema } from "../../test/editor";
 
 const serializer = extensionManager.serializer();
 const parser = extensionManager.parser({
@@ -6,29 +6,19 @@ const parser = extensionManager.parser({
   plugins: extensionManager.rulePlugins,
 });
 
-interface ProsemirrorNode {
-  type: string;
-  content?: ProsemirrorNode[];
-  attrs?: Record<string, unknown>;
-}
-
 it("preserves mixed checkbox and regular items in a list", () => {
   const markdown = `- [x] Checked item
 - Regular item
 - [ ] Unchecked item`;
 
   const ast = parser.parse(markdown);
-  const json = ast?.toJSON();
-
-  const checkboxList = json?.content?.find(
-    (node: ProsemirrorNode) => node.type === "checkbox_list"
-  );
+  const [checkboxList] = findNodes(ast?.toJSON(), "checkbox_list");
 
   expect(checkboxList).toBeDefined();
   expect(checkboxList?.content).toHaveLength(3);
-  expect(checkboxList?.content[0].type).toBe("checkbox_item");
-  expect(checkboxList?.content[1].type).toBe("checkbox_item");
-  expect(checkboxList?.content[2].type).toBe("checkbox_item");
+  expect(checkboxList?.content?.[0].type).toBe("checkbox_item");
+  expect(checkboxList?.content?.[1].type).toBe("checkbox_item");
+  expect(checkboxList?.content?.[2].type).toBe("checkbox_item");
 });
 
 it("round-trips mixed checkbox lists through serializer", () => {
@@ -52,22 +42,15 @@ it("does not convert nested bullet list items inside checkbox lists", () => {
 - [ ] Second checkbox`;
 
   const ast = parser.parse(markdown);
-  const json = ast?.toJSON();
-
-  const checkboxList = json?.content?.find(
-    (node: ProsemirrorNode) => node.type === "checkbox_list"
-  );
+  const [checkboxList] = findNodes(ast?.toJSON(), "checkbox_list");
 
   expect(checkboxList).toBeDefined();
   expect(checkboxList?.content).toHaveLength(2);
-  expect(checkboxList?.content[0].type).toBe("checkbox_item");
-  expect(checkboxList?.content[1].type).toBe("checkbox_item");
+  expect(checkboxList?.content?.[0].type).toBe("checkbox_item");
+  expect(checkboxList?.content?.[1].type).toBe("checkbox_item");
 
   // Nested list should remain a bullet_list, not a checkbox_list
-  const nestedContent = checkboxList?.content[0].content;
-  const nestedList = nestedContent?.find(
-    (node: ProsemirrorNode) => node.type === "bullet_list"
-  );
+  const [nestedList] = findNodes(checkboxList?.content?.[0], "bullet_list");
   expect(nestedList).toBeDefined();
   expect(nestedList?.content?.[0].type).toBe("list_item");
 });

shared/editor/rules/math.test.ts

@@ -0,0 +1,50 @@
+import type { JSONNode } from "../../test/editor";
+import { extensionManager, findNodes, schema } from "../../test/editor";
+
+const parser = extensionManager.parser({
+  schema,
+  plugins: extensionManager.rulePlugins,
+});
+
+const parseToJSON = (markdown: string): JSONNode | undefined =>
+  parser.parse(markdown)?.toJSON();
+
+describe("math markdown rules", () => {
+  it("parses inline math", () => {
+    const doc = parseToJSON("before $x + y$ after");
+    const nodes = findNodes(doc, "math_inline");
+
+    expect(nodes).toHaveLength(1);
+    expect(nodes[0].content?.[0].text).toBe("x + y");
+  });
+
+  it("parses block math with closing delimiter on its own line", () => {
+    const doc = parseToJSON("$$\na = b\n$$\n\nparagraph after");
+    const nodes = findNodes(doc, "math_block");
+
+    expect(nodes).toHaveLength(1);
+    expect(nodes[0].content?.[0].text).toContain("a = b");
+    expect(findNodes(doc, "paragraph")).toHaveLength(1);
+  });
+
+  it("parses block math with closing delimiter at the end of a content line", () => {
+    const doc = parseToJSON("$$\na = b\nc = d$$\n\nparagraph after");
+    const blocks = findNodes(doc, "math_block");
+
+    expect(blocks).toHaveLength(1);
+    expect(blocks[0].content?.[0].text).toContain("a = b");
+    expect(blocks[0].content?.[0].text).toContain("c = d");
+
+    // The paragraph following the block must not be swallowed into the math
+    const paragraphs = findNodes(doc, "paragraph");
+    expect(paragraphs).toHaveLength(1);
+    expect(blocks[0].content?.[0].text).not.toContain("paragraph after");
+  });
+
+  it("leaves unclosed inline math as plain text", () => {
+    const doc = parseToJSON("price is $5 and rising");
+
+    expect(findNodes(doc, "math_inline")).toHaveLength(0);
+    expect(findNodes(doc, "math_block")).toHaveLength(0);
+  });
+});

shared/editor/rules/math.ts

@@ -61,7 +61,7 @@ function mathInline(state: StateInline, silent: boolean): boolean {
   // we have found an opening delimiter already
   const start = state.pos + inlineMathDelimiter.length;
   match = start;
-  while ((match = state.src.indexOf(inlineMathDelimiter, match)) !== 1) {
+  while ((match = state.src.indexOf(inlineMathDelimiter, match)) !== -1) {
     // found potential delimeter, look for escapes, pos will point to
     // first non escape when complete
     pos = match - 1;
@@ -166,7 +166,10 @@ function mathDisplay(
       break;
     }
 
-    if (state.src.slice(pos, max).trim().slice(-3) === blockMathDelimiter) {
+    if (
+      state.src.slice(pos, max).trim().slice(-blockMathDelimiter.length) ===
+      blockMathDelimiter
+    ) {
       lastPos = state.src.slice(0, max).lastIndexOf(blockMathDelimiter);
       lastLine = state.src.slice(pos, lastPos);
       found = true;

shared/test/editor.ts

@@ -238,3 +238,35 @@ export function doc(
 ) {
   return schema.nodes.doc.create(null, content);
 }
+
+/**
+ * A plain-object representation of a ProseMirror node, as returned by
+ * `Node.toJSON()`.
+ */
+export interface JSONNode {
+  type: string;
+  content?: JSONNode[];
+  attrs?: Record<string, unknown>;
+  text?: string;
+}
+
+/**
+ * Recursively collects all nodes of the given type from a `Node.toJSON()`
+ * tree, including the root node itself.
+ *
+ * @param node - the JSON node to search, may be undefined for convenience.
+ * @param type - the node type name to match.
+ * @returns array of matching nodes in document order.
+ */
+export function findNodes(
+  node: JSONNode | undefined,
+  type: string
+): JSONNode[] {
+  if (!node) {
+    return [];
+  }
+  return [
+    ...(node.type === type ? [node] : []),
+    ...(node.content ?? []).flatMap((child) => findNodes(child, type)),
+  ];
+}