From 32e99c6e425d1b242bd186be7efdabfd38f4ba25 Mon Sep 17 00:00:00 2001 From: claude code agent 227 Date: Fri, 26 Jun 2026 18:01:39 +0300 Subject: [PATCH] fix(editor,git-sync): parse details `open` as a boolean so open state survives render/round-trip Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/lib/details/details.test.ts | 59 +++++++++++++++++++ .../editor-ext/src/lib/details/details.ts | 2 +- packages/git-sync/src/lib/docmost-schema.ts | 2 +- .../git-sync/test/roundtrip-all-nodes.test.ts | 29 +++++++-- 4 files changed, 85 insertions(+), 7 deletions(-) create mode 100644 packages/editor-ext/src/lib/details/details.test.ts diff --git a/packages/editor-ext/src/lib/details/details.test.ts b/packages/editor-ext/src/lib/details/details.test.ts new file mode 100644 index 00000000..55c6f3a2 --- /dev/null +++ b/packages/editor-ext/src/lib/details/details.test.ts @@ -0,0 +1,59 @@ +import { describe, it, expect } from "vitest"; +import { Editor } from "@tiptap/core"; +import { Document } from "@tiptap/extension-document"; +import { Paragraph } from "@tiptap/extension-paragraph"; +import { Text } from "@tiptap/extension-text"; +import { Details } from "./details"; +import { DetailsSummary } from "./details-summary"; +import { DetailsContent } from "./details-content"; + +// The `details` node's `open` attribute must parse to a strict BOOLEAN. The old +// `getAttribute("open")` returned "" (falsy) for `
` and `null` +// when absent, so a parsed-open details rendered without `open` and collapsed. +// `hasAttribute` yields a real boolean, so open state survives parse → render. + +const extensions = [ + Document, + Paragraph, + Text, + Details, + DetailsSummary, + DetailsContent, +]; + +/** Parse an HTML string through the schema and return the first details node. */ +function parseDetails(html: string): any { + const editor = new Editor({ extensions, content: html }); + const json = editor.getJSON(); + const find = (n: any): any => { + if (!n || typeof n !== "object") return undefined; + if (n.type === "details") return n; + if (Array.isArray(n.content)) { + for (const c of n.content) { + const hit = find(c); + if (hit) return hit; + } + } + return undefined; + }; + const details = find(json); + editor.destroy(); + return details; +} + +describe("details node: open attribute parses as a strict boolean", () => { + const body = + 'S

b

'; + + it("parses
to open === true", () => { + const details = parseDetails(`
${body}
`); + expect(details).toBeDefined(); + expect(details.attrs.open).toBe(true); + }); + + it("parses
(no open) to open === false", () => { + const details = parseDetails(`
${body}
`); + expect(details).toBeDefined(); + expect(details.attrs.open).toBe(false); + }); +}); diff --git a/packages/editor-ext/src/lib/details/details.ts b/packages/editor-ext/src/lib/details/details.ts index 41c66dca..246aa134 100644 --- a/packages/editor-ext/src/lib/details/details.ts +++ b/packages/editor-ext/src/lib/details/details.ts @@ -39,7 +39,7 @@ export const Details = Node.create({ return { open: { default: false, - parseHTML: (e) => e.getAttribute("open"), + parseHTML: (e) => e.hasAttribute("open"), renderHTML: (a) => (a.open ? { open: "" } : {}), }, }; diff --git a/packages/git-sync/src/lib/docmost-schema.ts b/packages/git-sync/src/lib/docmost-schema.ts index e6761d2f..3595866b 100644 --- a/packages/git-sync/src/lib/docmost-schema.ts +++ b/packages/git-sync/src/lib/docmost-schema.ts @@ -470,7 +470,7 @@ const Details = Node.create({ return { open: { default: false, - parseHTML: (el: HTMLElement) => el.getAttribute("open"), + parseHTML: (el: HTMLElement) => el.hasAttribute("open"), renderHTML: (attrs: Record) => attrs.open ? { open: "" } : {}, }, diff --git a/packages/git-sync/test/roundtrip-all-nodes.test.ts b/packages/git-sync/test/roundtrip-all-nodes.test.ts index 5b3baa84..6bc634a9 100644 --- a/packages/git-sync/test/roundtrip-all-nodes.test.ts +++ b/packages/git-sync/test/roundtrip-all-nodes.test.ts @@ -200,11 +200,30 @@ describe('git-sync converter: lose-prone atoms keep their VALUES across a round const back = await markdownToProseMirror(md); const details = findNode(back, 'details'); expect(details).toBeDefined(); - // The schema parses the present `open` boolean attribute to "" (its raw - // value); a DROPPED open parses to the default `false`. Asserting it is no - // longer the default proves the nested path now preserves open — parity with - // the top-level
case. RED before the fix (open === false). - expect(details.attrs?.open).not.toBe(false); + // `open` must round-trip as a STRICT boolean `true` — not "" (the old raw + // getAttribute value) and not the default `false` (a dropped attribute). + // Before the schema parseHTML fix (hasAttribute), `
` parsed to + // "" — falsy, so it rendered as a bare
and collapsed. RED before + // the fix (open was "" or false, never === true). + expect(details.attrs?.open).toBe(true); + }); + + it('B: a TOP-LEVEL details keeps open as strict boolean true', async () => { + const original = doc({ + type: 'details', + attrs: { open: true }, + content: [ + { type: 'detailsSummary', content: [T('S')] }, + { type: 'detailsContent', content: [P(T('b'))] }, + ], + }); + const md = convertProseMirrorToMarkdown(original); + const back = await markdownToProseMirror(md); + const details = findNode(back, 'details'); + expect(details).toBeDefined(); + // Strict boolean, proving the value survives as `true` (not ""/false). + // RED before the fix: parseHTML returned getAttribute("open") === "". + expect(details.attrs?.open).toBe(true); }); it('D: htmlEmbed source VALUE and height survive', async () => {