diff --git a/src/renderer/src/components/editor/details-markdown-html.ts b/src/renderer/src/components/editor/details-markdown-html.ts index 03f3ee9a645..2d40050d2eb 100644 --- a/src/renderer/src/components/editor/details-markdown-html.ts +++ b/src/renderer/src/components/editor/details-markdown-html.ts @@ -1,9 +1,10 @@ import type { MarkdownToken } from '@tiptap/core' import { - getMarkdownFenceRanges, - isInsideMarkdownFenceRange, + isInsideRange, + markdownCodeSpanRanges, + markdownFenceRanges, type MarkdownFenceRanges -} from './markdown-fence-scanner' +} from './markdown-scan-ranges' // Toggle summaries can render at heading scales 1–5, mirroring the plain // heading levels the slash menu / toolbar dropdown offer (h1–h5). @@ -92,12 +93,38 @@ export function renderDetailsAttributes(attrs: Record | undefin return attributes.join(' ') } +export function findDetailsBlockStart(content: string): number { + if (!content.includes(']*>/i) if (!openingMatch) { @@ -106,7 +133,8 @@ export function matchDetailsHtmlBlock( const detailsTagPattern = /<\/?details\b[^>]*>/gi detailsTagPattern.lastIndex = start - const fenceRanges = precomputedFenceRanges ?? getMarkdownFenceRanges(content) + const fenceRanges = precomputedFenceRanges ?? markdownFenceRanges(content) + const codeSpanRanges = precomputedCodeSpanRanges ?? markdownCodeSpanRanges(content) let depth = 0 @@ -117,7 +145,10 @@ export function matchDetailsHtmlBlock( } const tag = tagMatch[0] - if (tagMatch.index !== start && isInsideMarkdownFenceRange(tagMatch.index, fenceRanges)) { + if ( + tagMatch.index !== start && + (isInsideRange(tagMatch.index, fenceRanges) || isInsideRange(tagMatch.index, codeSpanRanges)) + ) { continue } @@ -250,6 +281,7 @@ function stripEditableNestedDetails(bodyHtml: string, nestingLevel: number): str let index = 0 // Why: without sharing this, N sibling toggles rescan the whole body N times. let fenceRanges: MarkdownFenceRanges | null = null + let codeSpanRanges: MarkdownFenceRanges | null = null for (;;) { const nestedStart = indexOfAsciiIgnoreCase(bodyHtml, '` with no class. Any other class value (or any other unrecognized attribute, e.g. `id="x"`) fails that check entirely and the block is kept as passthrough HTML, whose raw source bytes are never touched by the details serializer.', + "- The read-only preview (`MarkdownPreviewBody.tsx`) allowlists `className: 'orca-details'` through `rehype-sanitize` *if the source HTML already has it* — it does not inject the class. A GitHub-authored file with no class already rendered unstyled in preview before this fix; that is unchanged, not something this fix touches.", + '', + "A newly created or user-authored `
` block should never have Orca write a styling class into its markdown source — GitHub-flavored files never carry that class, and the DOM styling path doesn't need it in the source. But a file already saved by an earlier Orca version does carry the class in its source, and dropping it unconditionally caused a second bug: the round-trip eligibility check compares serialized output against the source's literal bytes, so a legacy file that loses its class on save stops matching and regresses back to Source mode.", + '', + 'The final design: `parseDetailsAttributes` records whether the block\'s opening tag had the legacy class (`hasLegacyStylingClass`), that flag is carried on the TipTap node as a genuine schema attribute (declared in `OrcaDetails.addAttributes`, alongside the existing `variant` attribute, so it survives edits and undo), and `renderDetailsAttributes` re-emits `class="orca-details"` only when the flag is set. A block with no class in its source stays classless through every subsequent save; a block that already had the class keeps it.', + '', + "User-provided attributes (a custom `class`, an `id`, etc.) don't need special handling under this design — they never reach the details tokenizer at all. `isEditableDetailsHtmlBlock` rejects any attribute outside its allowlist (`open`, the legacy class, `data-orca-toggle`), so those blocks are treated as opaque passthrough HTML and their source bytes round-trip verbatim, untouched by `renderDetailsAttributes`. Verified directly (see below).", + '', + '## Commits', + '', + '- `93f807fbfd59a9854ecb3bf24c0a3a1051a9b79f` — initial fix: serializer stops always writing the class.', + '- `6213aa9a99fb7717a5d968b6bbd21a00d66e2374` — follow-up: preserves the class when the source already had it, fixing a legacy-file regression the initial commit introduced.', + '', + 'Both on branch `fb/rtme-details-roundtrip` in `/Users/fbarthelemy/Code/orca/fb-rtme-details/`.', + '', + '## Review findings and verification', + '', + "Before making the follow-up change, ran three ad hoc probe tests against the first commit's code to check for regressions, all removed after verification:", + '', + '- Case — Source — Result before fix #2', + '- Legacy Orca-saved file — `
…` — `getMarkdownRichModeUnsupportedReason` returned `\'html-or-jsx\'` — **confirmed regression**, the file would fall back to Source mode.', + '- User custom class — `
…` — Round-tripped byte-identical to input; eligibility `null`. Passthrough HTML, unaffected.', + '- User `id` attribute — `
…` — Round-tripped byte-identical to input; eligibility `null`. Passthrough HTML, unaffected.', + '', + 'After the follow-up commit, re-ran the same three probes plus a bare-details control:', + '', + '- Case — Result after fix #2', + '- Legacy Orca-saved file — `getMarkdownRichModeUnsupportedReason` returns `null`; round-trips byte-identical to source.', + "- Bare `
` with no class — Still gets no class on save (fix #1's intent preserved).", + '- User custom class / `id` — Unchanged — still passthrough, still byte-identical.', + '', + 'All four passed. These are now permanent tests: `markdown-rich-mode.test.ts` ("allows a details block already carrying the legacy orca-details class"), `markdown-round-trip.test.ts` ("round-trips an orca-authored nested toggle unchanged", restored to its original byte-identical assertion, plus a new "does not backfill the legacy styling class onto a freshly nested toggle" case covering a legacy outer block wrapping a freshly authored inner block).', + '', + '## Diff summary', + '', + '**Commit 1** (`93f807fb`):', + '', + '- `details-markdown-html.ts`: `renderDetailsAttributes` stops prepending `class="orca-details"` unconditionally.', + '- `rich-markdown-details-extension.ts`: `renderMarkdown` only adds a leading space before the attribute string when attributes exist, avoiding a stray `
`.', + '- Test files updated to match the new (classless) serialized output.', + '', + '**Commit 2** (`6213aa9a`):', + '', + "- `rich-markdown-details-extension.ts`: `OrcaDetails.addAttributes` gains `hasLegacyStylingClass` (default `false`, not a DOM attribute — `parseHTML`/`renderHTML` are no-ops since it's markdown-only state).", + '- `details-markdown-html.ts`: `parseDetailsAttributes` sets `hasLegacyStylingClass` from a new `LEGACY_STYLING_CLASS_PATTERN` regex matching only the exact `class="orca-details"` form (the only class value `hasOnlySupportedDetailsAttributes` tolerates). `renderDetailsAttributes` re-emits the class when that flag is `true`.', + '- `markdown-rich-mode.test.ts`: added the legacy-class eligibility regression test.', + '- `markdown-round-trip.test.ts`: restored the nested-legacy-toggle test to its original byte-identical assertion (previously incorrectly rewritten in commit 1 to expect the class dropped); added the backfill-prevention test.', + '', + '## Commands run (final state, after both commits)', + '', + '- Command — Result', + '- `pnpm test src/renderer/src/components/editor` — pass — 208 files, 1416 tests, 2 skipped', + '- `pnpm tc` — pass — no type errors', + '- `npx oxlint` (full repo) — pass — exit 0, no findings', + '- `npx oxfmt --write` on changed files — applied, no changes needed on the second pass', + '- pre-commit hook (oxlint + oxfmt via lint-staged), both commits — pass, no manual fixes needed', + '', + '## Draft: GitHub issue', + '', + '**Title:** `[Bug]: Markdown files with
blocks always open in Source mode`', + '', + '**Body:**', + '', + '### Operating system', + '', + 'macOS', + '', + '### Orca version', + '', + '1.4.198', + '', + '### Details', + '', + 'Opening a markdown file that contains a `
` disclosure block always lands in Source mode with the "this file contains HTML, JSX, or MDX" banner, even for the smallest possible block and even though Orca has a dedicated rich-editor extension for details blocks that never gets a chance to run.', + '', + 'Minimal reproduction: create a markdown file containing exactly this and open it in Orca.', + '', + '```markdown', + '
', + 'x', + '', + 'body', + '', + '
', + '```', + '', + 'Expected: the file opens in the rich editor with the details block rendered as a collapsible toggle.', + '', + 'Actual: the file falls back to Source mode with the HTML/JSX/MDX banner.', + '', + 'The mechanism is that the markdown serializer always adds `class="orca-details"` to the `
` tag it writes out ([`renderDetailsAttributes`](https://github.com/stablyai/orca/blob/7dd183d82dafb768bb2506d161438a6d0894fd53/src/renderer/src/components/editor/details-markdown-html.ts#L79-L92)), and the rich-mode eligibility check compares that serialized output against the file\'s literal opening tag byte-for-byte ([`preservesEmbeddedHtml`](https://github.com/stablyai/orca/blob/7dd183d82dafb768bb2506d161438a6d0894fd53/src/renderer/src/components/editor/markdown-rich-mode.ts#L211-L221)), so a source file without that class never matches and the file is treated as unsupported HTML.', + '', + '## Draft: PR body', + '', + '## ELI5', + '', + 'Any markdown file with a `
` disclosure block used to always open in Source mode instead of the rich editor, even for a two-line block with no attributes.', + '', + '## What Changed', + '', + 'The markdown serializer only writes `class="orca-details"` into a saved `
` tag when the file\'s own source already had that class. A block authored fresh, or one that came from a plain markdown file, never gets the class written into it, since the class is already applied to the rendered editor node independently through the extension\'s `HTMLAttributes` configuration.', + '', + '## Why', + '', + "Rich-mode eligibility detects embedded HTML and then verifies it survives a round trip through the editor by comparing the re-serialized output against the source's literal opening tag. The serializer used to always add a class the source didn't have, so that comparison failed for every `
` block that didn't already carry the class, and the file fell back to Source mode. Making the class conditional on the source removes the mismatch at its origin, and keeps files already saved by Orca (which do carry the class) round-tripping unchanged.", + '', + '## Linked Issue', + '', + 'Fixes #TBD', + '', + '## Visual Proof', + '', + '**Before:** opening a markdown file containing a details block lands in Source mode with the HTML banner, and the collapsible toggle is not interactive.', + '', + '**After:** the same file opens in the rich editor with the details block rendered as a collapsible block.', + '', + '## Testing', + '', + '- [ ] I manually tested these changes locally', + '- [x] Automated tests added/updated, or explained why not below', + '', + "Added regression tests in `markdown-rich-mode.test.ts` for a minimal bare details block, one with `open`, one already carrying the legacy styling class, and a full document shape (front matter, prose, a details block with bold text/link/list/fenced code, a second `
` block). Updated existing round-trip assertions in `markdown-round-trip.test.ts` and `rich-markdown-details-keyboard.test.ts` to match; added a test confirming a legacy-class outer block doesn't backfill the class onto a freshly authored nested block; confirmed by test that user-provided attributes like a custom `class` or `id` are unaffected, since those blocks are treated as passthrough HTML rather than going through this serializer. `pnpm test`, `pnpm tc`, and `oxlint` all pass.", + '', + '## AI Disclosure', + '', + 'Authored with Claude Code (Claude Fable 5.1 and Sonnet teammates) under my review.', + '', + '## Review', + '', + "Removes a source-mutating side effect from the details-block markdown serializer: it now writes the styling class into a saved file's `
` tag only when the file's source already carried that class, since the class is otherwise applied independently to the rendered DOM node. The unconditional injection was why any file with a details block failed the rich-mode round-trip check and fell back to Source mode; making it conditional both fixes new files and keeps files already saved by Orca round-tripping unchanged. User-provided attributes are unaffected, since blocks with attributes outside the small supported set are treated as opaque passthrough HTML rather than going through this serializer.", + '', + '## Agent skill upstream boundary', + '', + '- [x] Not applicable, or this change follows `docs/reference/agent-skill-sharing-upstream-boundary.md` and copies or mechanically translates no upstream skill-installer source, tests, fixtures, registry entries, path tables, comments, or documentation.', + '', + '## Notes', + '', + 'Ensure no issues in: Security, Cross-platoform support (Linux, Windows, Mac), Remote SSH, Mobile, general backwards compatibility, performance', + '', + '## Checklist', + '', + '- [x] This PR is small and focused', + '- [x] I explained what changed and why (including ELI5)', + '- [ ] Before/after screenshots or videos attached for UI changes, or `N/A` with reason', + '- [x] Self-reviewed for correctness, security, and performance', + '- [x] Cross-platform, SSH/remote, and path/shortcut impact considered (or N/A)', + '- [x] `pnpm lint`, `pnpm typecheck`, `pnpm test`, and `pnpm build` pass (or CI will cover; local preferred)', + '', + '## Author', + '', + 'X: [@fbartho](https://x.com/fbartho). I prefer Mastodon: [@fbartho@mastodon.social](https://mastodon.social/@fbartho).' +].join('\n') + function roundTripMarkdown(content: string): string { const codec = createRichMarkdownEditorCodec() const editor = new Editor({ @@ -323,6 +486,30 @@ describe('rich markdown round trip', () => { expect(roundTripMarkdown('
\n')).toBe('
') }) + it('preserves a prose mention of
inside inline code', () => { + const input = 'Text with `
` inline and a `` mention.\n' + expect(roundTripMarkdown(input)).toBe(input.trimEnd()) + }) + + it('preserves a fenced code block containing
', () => { + const input = ['```', '
', 'Not a block', '```', ''].join('\n') + expect(roundTripMarkdown(input)).toBe(input.trimEnd()) + }) + + it('keeps a details block intact when its body mentions
in a code span', () => { + // The body text still contains a literal
-shaped tag, so + // isEditableDetailsHtmlBlock keeps this passthrough HTML rather than rich + // mode — the regression this guards is the tag-depth pairing scan closing + // the block early at that code-span match and truncating the raw source. + const input = + '
Toggle

See `

` for reference and more body text after.

\n' + expect(roundTripMarkdown(input)).toBe(input.trimEnd()) + }) + + it('round-trips a real-world document with multiple
mentions in code spans', () => { + expect(roundTripMarkdown(REPORT_DOCUMENT_FIXTURE)).toBe(REPORT_DOCUMENT_FIXTURE) + }) + it('inserts editable text toggles from slash commands', () => { expect(slashCommandMarkdown('toggle-text')).toBe( '
\n\n\n\n\n
' diff --git a/src/renderer/src/components/editor/markdown-scan-ranges.ts b/src/renderer/src/components/editor/markdown-scan-ranges.ts new file mode 100644 index 00000000000..e46a2932207 --- /dev/null +++ b/src/renderer/src/components/editor/markdown-scan-ranges.ts @@ -0,0 +1,95 @@ +// Fence ranges depend only on the scanned string, so callers scanning one body +// repeatedly compute them once and share them across sibling matches. +export type MarkdownFenceRanges = readonly (readonly [number, number])[] + +export function markdownFenceRanges(content: string): MarkdownFenceRanges { + const ranges: [number, number][] = [] + let offset = 0 + let openFence: { closingPattern: RegExp; start: number } | null = null + + for (const lineMatch of content.matchAll(/[^\r\n]*(?:\r\n|\n|\r|$)/g)) { + const line = lineMatch[0] + if (line === '') { + break + } + + const lineText = line.replace(/(?:\r\n|\n|\r)$/u, '') + if (openFence) { + // Built once per fence: rebuilding it per line recompiled the same regex for every fenced line. + if (openFence.closingPattern.test(lineText)) { + ranges.push([openFence.start, offset + line.length]) + openFence = null + } + } else { + const openingFenceMatch = lineText.match(/^ {0,3}(`{3,}|~{3,})/u) + if (openingFenceMatch?.[1]) { + openFence = { + closingPattern: new RegExp( + `^ {0,3}${openingFenceMatch[1][0]}{${openingFenceMatch[1].length},}\\s*$` + ), + start: offset + } + } + } + + offset += line.length + } + + if (openFence) { + ranges.push([openFence.start, content.length]) + } + + return ranges +} + +export function isInsideRange(index: number, ranges: MarkdownFenceRanges): boolean { + return ranges.some(([start, end]) => index >= start && index < end) +} + +// CommonMark code spans: a backtick run only closes on a run of the same +// length, so `` `
` `` is one span even though `
` alone +// isn't. Mirrors the tick-matching in raw-markdown-html.ts's inline scan. +export function markdownCodeSpanRanges(content: string): MarkdownFenceRanges { + const ranges: [number, number][] = [] + let index = 0 + + while (index < content.length) { + if (content[index] !== '`') { + index += 1 + continue + } + + let tickCount = 0 + while (content[index + tickCount] === '`') { + tickCount += 1 + } + + const spanStart = index + let searchFrom = index + tickCount + let closingIndex = -1 + while (searchFrom < content.length) { + const candidate = content.indexOf('`'.repeat(tickCount), searchFrom) + if (candidate === -1) { + break + } + if ( + (candidate === 0 || content[candidate - 1] !== '`') && + content[candidate + tickCount] !== '`' + ) { + closingIndex = candidate + break + } + searchFrom = candidate + 1 + } + + if (closingIndex === -1) { + index += tickCount + continue + } + + ranges.push([spanStart, closingIndex + tickCount]) + index = closingIndex + tickCount + } + + return ranges +} diff --git a/src/renderer/src/components/editor/rich-markdown-details-extension.ts b/src/renderer/src/components/editor/rich-markdown-details-extension.ts index 1bfcfc09a15..dede1057d34 100644 --- a/src/renderer/src/components/editor/rich-markdown-details-extension.ts +++ b/src/renderer/src/components/editor/rich-markdown-details-extension.ts @@ -7,6 +7,7 @@ import { detailsBodyHtmlToMarkdown, escapeDetailsHtml, extractDetailsSummaryHtml, + findDetailsBlockStart, isEditableDetailsHtmlBlock, matchDetailsHtmlBlock, parseDetailsAttributes, @@ -228,7 +229,7 @@ const OrcaDetails = Details.extend({ markdownTokenizer: { name: 'details', level: 'block', - start: '