fix(editor): ignore <details> inside code spans when tokenizing

The details markdown extension registered a bare `<details` string as
its marked start hook, which @tiptap/markdown turns into a raw
indexOf scan over unlexed block source. A `<details>` mention inside
backtick code spans or fenced code therefore split the surrounding
paragraph and let marked's own HTML-block rule swallow the remaining
prose into a details node, corrupting the saved file.

findDetailsBlockStart replaces the literal start hook with a scan that
skips fenced and code-span ranges and requires the tag to open a line
per CommonMark's HTML-block rule. matchDetailsHtmlBlock's tag-depth
pairing scan gets the same code-span exclusion, fixing a related gap
where a </details>-shaped code span inside a real block's body could
close the tag pairing early.

The scanning helpers move to markdown-scan-ranges.ts to keep
details-markdown-html.ts under the project's max-lines limit.

# Conflicts:
#	src/renderer/src/components/editor/details-markdown-html.ts
This commit is contained in:
Neil
2026-09-18 00:32:47 -07:00
parent 78121c2dd5
commit 4dbc6ffcae
4 changed files with 325 additions and 9 deletions
@@ -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 15, mirroring the plain
// heading levels the slash menu / toolbar dropdown offer (h1h5).
@@ -92,12 +93,38 @@ export function renderDetailsAttributes(attrs: Record<string, unknown> | undefin
return attributes.join(' ')
}
export function findDetailsBlockStart(content: string): number {
if (!content.includes('<details')) {
return -1
}
const fenceRanges = markdownFenceRanges(content)
const codeSpanRanges = markdownCodeSpanRanges(content)
const tagPattern = /<details\b/gi
for (;;) {
const match = tagPattern.exec(content)
if (!match) {
return -1
}
const index = match.index
if (isInsideRange(index, fenceRanges) || isInsideRange(index, codeSpanRanges)) {
continue
}
const lineStart = content.lastIndexOf('\n', index - 1) + 1
const indent = content.slice(lineStart, index)
if (indent.length <= 3 && /^ *$/.test(indent)) {
return index
}
}
}
export function matchDetailsHtmlBlock(
content: string,
start: number,
// Ranges depend only on `content`, so callers scanning one body repeatedly
// compute them once and share them across sibling matches.
precomputedFenceRanges?: MarkdownFenceRanges
precomputedFenceRanges?: MarkdownFenceRanges,
precomputedCodeSpanRanges?: MarkdownFenceRanges
): DetailsHtmlBlock | null {
const openingMatch = content.slice(start).match(/^<details\b[^>]*>/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, '<details', index)
@@ -261,8 +293,9 @@ function stripEditableNestedDetails(bodyHtml: string, nestingLevel: number): str
return null
}
fenceRanges ??= getMarkdownFenceRanges(bodyHtml)
const nested = matchDetailsHtmlBlock(bodyHtml, nestedStart, fenceRanges)
fenceRanges ??= markdownFenceRanges(bodyHtml)
codeSpanRanges ??= markdownCodeSpanRanges(bodyHtml)
const nested = matchDetailsHtmlBlock(bodyHtml, nestedStart, fenceRanges, codeSpanRanges)
if (!nested || !isEditableDetailsHtmlBlock(nested, nestingLevel + 1)) {
return null
}
@@ -6,6 +6,169 @@ import { createRichMarkdownEditorCodec } from './rich-markdown-source-transport'
import type { SlashCommandId } from './rich-markdown-slash-commands'
import { slashCommands } from './rich-markdown-slash-commands'
const REPORT_DOCUMENT_FIXTURE = [
'# Fix report: details blocks disabling rich mode',
'',
'## Decision: option A (stop unconditionally writing the styling class into markdown output)',
'',
'Traced every consumer of `class="orca-details"` before choosing:',
'',
"- The rendered DOM node gets the class independently, from `OrcaDetails.configure({ HTMLAttributes: { class: 'orca-details' } })` in `rich-markdown-details-extension.ts` — this does not read the markdown source string at all.",
'- `renderDetailsAttributes` (`details-markdown-html.ts`) had exactly one call site: the markdown serializer itself. Nothing else in the pipeline reads the class from serialized markdown text.',
'- The tokenizer already treats the class as optional: `hasOnlySupportedDetailsAttributes` strips it if present but never requires it, and `isEditableDetailsHtmlBlock` accepts a bare `<details>` 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 `<details>` 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 — `<details class="orca-details">…` — `getMarkdownRichModeUnsupportedReason` returned `\'html-or-jsx\'` — **confirmed regression**, the file would fall back to Source mode.',
'- User custom class — `<details class="my-notes" open>…` — Round-tripped byte-identical to input; eligibility `null`. Passthrough HTML, unaffected.',
'- User `id` attribute — `<details id="x">…` — 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 `<details>` 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 `<details >`.',
'- 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 <details> blocks always open in Source mode`',
'',
'**Body:**',
'',
'### Operating system',
'',
'macOS',
'',
'### Orca version',
'',
'1.4.198',
'',
'### Details',
'',
'Opening a markdown file that contains a `<details>` 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',
'<details>',
'<summary>x</summary>',
'',
'body',
'',
'</details>',
'```',
'',
'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 `<details>` 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 `<details>` 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 `<details>` 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 `<details>` 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 `<details open>` 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 `<details>` 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('<details>\n')).toBe('<details>')
})
it('preserves a prose mention of <details> inside inline code', () => {
const input = 'Text with `<details>` inline and a `<summary>` mention.\n'
expect(roundTripMarkdown(input)).toBe(input.trimEnd())
})
it('preserves a fenced code block containing <details>', () => {
const input = ['```', '<details>', '<summary>Not a block</summary>', '```', ''].join('\n')
expect(roundTripMarkdown(input)).toBe(input.trimEnd())
})
it('keeps a details block intact when its body mentions </details> in a code span', () => {
// The body text still contains a literal </details>-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 =
'<details><summary>Toggle</summary><p>See `</details>` for reference and more body text after.</p></details>\n'
expect(roundTripMarkdown(input)).toBe(input.trimEnd())
})
it('round-trips a real-world document with multiple <details> 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(
'<details class="orca-details" open>\n<summary></summary>\n\n\n\n</details>'
@@ -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 `` `<details>` `` is one span even though `<details>` 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
}
@@ -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: '<details',
start: findDetailsBlockStart,
tokenize(src, _tokens, lexer) {
const detailsBlock = matchDetailsHtmlBlock(src, 0)
if (!detailsBlock || !isEditableDetailsHtmlBlock(detailsBlock)) {