From 35897da0aa9cecd7444033add87b467eff7e4f1d Mon Sep 17 00:00:00 2001
From: Jinwoo Hong <73622457+Jinwoo-H@users.noreply.github.com>
Date: Tue, 22 Sep 2026 01:11:13 -0400
Subject: [PATCH] fix(mobile): serialize a list the engine nested inside a
 paragraph (#22145)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`insertUnorderedList` puts the `<ul>` inside the `<p>` it was given rather than replacing it —
measured on WebKit 26.4 and Chromium 147 both — and `blockMarkdown` read such a paragraph inline.
A bullet list the user typed came back as the paragraph's own text with no marker, so it did not
survive a markdown round trip, on the page and in the native WebView alike.

The serializer now reads structure wherever the list sits: text before it is a paragraph, the list
is a list, text after is a paragraph. The DOM is left as the engine made it and no branch asks
which engine is running. The parse side needs no mirror — it already renders `- x` as a top-level
`<ul>`, which is the shape the fixed serializer reports, and the flat control case pins that.

The unit fixture is built through the paragraph's own `innerHTML`: the HTML parser closes a `<p>`
before a `<ul>`, so a markup string on the editor gives two siblings and would measure the flat
shape. Each case asserts the nesting it got before it reads anything.

Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb
---
 ...bile-web-app-rich-markdown-render.test.mjs | 63 +++++++++++++++++--
 .../rich-markdown/html-block-markdown.ts      | 43 ++++++++++++-
 .../rich-markdown/html-list-markdown.ts       | 10 +++
 .../rich-markdown/markdown-round-trip.test.ts | 62 ++++++++++++++++++
 4 files changed, 172 insertions(+), 6 deletions(-)

diff --git a/config/scripts/mobile-web-app-rich-markdown-render.test.mjs b/config/scripts/mobile-web-app-rich-markdown-render.test.mjs
index 46bc797267a..b9c93dbc5c9 100644
--- a/config/scripts/mobile-web-app-rich-markdown-render.test.mjs
+++ b/config/scripts/mobile-web-app-rich-markdown-render.test.mjs
@@ -356,10 +356,11 @@ async function setContentWithin(page, markdown, selector) {
  * The plain paragraph a command case starts from, numbered so no two are the same.
  *
  * The content prop is what resets the document, and the controller only pushes when it differs
- * from what the editor last reported. One command breaks that: WebKit's `insertUnorderedList`
- * nests the `<ul>` inside the `<p>` it was given, and the serializer walks back out with the same
- * text — so re-setting the same string after it is a no-op, and the next command ran against the
- * list rather than against a paragraph.
+ * from what the editor last reported. Re-setting the same string is therefore a no-op, and the
+ * next command would run against the document the one before it left. Both list commands used to
+ * make that worse rather than better: `insertUnorderedList` nests the `<ul>` inside the `<p>` it
+ * was given on both engines, and the serializer read the paragraph inline, so pressing it reported
+ * the paragraph's own text back unchanged.
  */
 const bodyFor = (index) => `body text ${String(index)}`
 
@@ -503,6 +504,60 @@ describeEditor(
           }
         }, 600_000)
 
+        /**
+         * A list typed on the surface, read back as markdown, and rendered from that markdown.
+         *
+         * `insertUnorderedList` puts the `<ul>` inside the `<p>` it was given rather than replacing
+         * it — measured here on WebKit 26.4 and Chromium 147 both — and a serializer that read such
+         * a paragraph inline reported its own text with no marker, so the bullet the user pressed
+         * was gone the moment the host saved what the document reported.
+         */
+        it('reports a typed bullet list as a list, and renders that markdown back as one', async () => {
+          const { page, consoleErrors } = await openPage(browser)
+          try {
+            const body = 'bullet round trip'
+            await setContent(page, body, `<p>${body}</p>`)
+            await select(page, 'all')
+            await page.evaluate(() => {
+              globalThis.__orcaEditor.changes.length = 0
+            })
+            await press(page, 'Bullet list')
+            await page.waitForFunction(
+              () => document.querySelector('#first-surface #editor')?.querySelector('ul') !== null,
+              null,
+              { timeout: 15_000 }
+            )
+            await page.waitForFunction(() => globalThis.__orcaEditor.changes.length > 0, null, {
+              timeout: 15_000
+            })
+            // The precondition: the engine really did nest the list inside the paragraph. An engine
+            // that replaced the paragraph would leave this case measuring the flat shape, which the
+            // serializer never got wrong.
+            expect(
+              await page.evaluate(
+                () =>
+                  document.querySelector('#first-surface #editor ul')?.parentElement?.tagName ??
+                  null
+              )
+            ).toBe('P')
+            // What the host would save.
+            expect(await page.evaluate(() => globalThis.__orcaEditor.changes.at(-1))).toBe(
+              `- ${body}`
+            )
+
+            // And the trip closes: that markdown comes back in as a list rather than a paragraph.
+            // Through a different document first, because the prop already holds this string and
+            // re-setting the same one is a no-op.
+            await setContent(page, 'plain again', '<p>plain again</p>')
+            await setContent(page, `- ${body}`, `<ul><li><p>${body}</p></li></ul>`)
+
+            expect(await page.evaluate(() => globalThis.__orcaCspViolations)).toEqual([])
+            expect(consoleErrors).toEqual([])
+          } finally {
+            await page.close()
+          }
+        }, 600_000)
+
         it('answers Link and Image from a modal rather than from a prompt that returns null', async () => {
           const { page, consoleErrors } = await openPage(browser)
           try {
diff --git a/mobile/src/components/rich-markdown/html-block-markdown.ts b/mobile/src/components/rich-markdown/html-block-markdown.ts
index 5fe02f1f5c9..8fa61677634 100644
--- a/mobile/src/components/rich-markdown/html-block-markdown.ts
+++ b/mobile/src/components/rich-markdown/html-block-markdown.ts
@@ -1,7 +1,46 @@
 import { codeFenceFor } from './markdown-code-fence'
 import { escapeTableCell } from './markdown-table-rows'
 import { inlineChildren, inlineMarkdown, textContent } from './html-inline-markdown'
-import { listMarkdown } from './html-list-markdown'
+import { holdsUnownedList, listMarkdown } from './html-list-markdown'
+
+/**
+ * A paragraph that carries a list, as the blocks it really holds: text, the list, then text.
+ *
+ * `insertUnorderedList` nests the `<ul>` inside the `<p>` it was given rather than replacing it —
+ * measured on WebKit 26.4 and Chromium 147 both — and reading such a paragraph inline gave back its
+ * own text with no marker, so a list the user typed did not survive a round trip. Structure decides
+ * what a list is; the DOM is left as the engine made it.
+ */
+function blocksAroundLists(element: Element): string {
+  const blocks: string[] = []
+  let inline = ''
+  const flushInline = () => {
+    if (inline.trim()) {
+      blocks.push(inline.trim())
+    }
+    inline = ''
+  }
+  for (const child of Array.from(element.childNodes)) {
+    if (!(child instanceof Element)) {
+      inline += inlineMarkdown(child)
+      continue
+    }
+    const tag = child.tagName.toLowerCase()
+    if (tag === 'ul' || tag === 'ol') {
+      flushInline()
+      blocks.push(listMarkdown(child, 0))
+      continue
+    }
+    if (holdsUnownedList(child)) {
+      flushInline()
+      blocks.push(blocksAroundLists(child))
+      continue
+    }
+    inline += inlineMarkdown(child)
+  }
+  flushInline()
+  return blocks.filter(Boolean).join('\n\n')
+}
 
 /**
  * One top-level node of the editable surface as a markdown block.
@@ -21,7 +60,7 @@ export function blockMarkdown(node: Node): string {
     return `${'#'.repeat(Number(tag.slice(1)))} ${inlineChildren(node).trim()}`
   }
   if (tag === 'p' || tag === 'div') {
-    return inlineChildren(node).trim()
+    return holdsUnownedList(node) ? blocksAroundLists(node) : inlineChildren(node).trim()
   }
   if (tag === 'blockquote') {
     return inlineChildren(node)
diff --git a/mobile/src/components/rich-markdown/html-list-markdown.ts b/mobile/src/components/rich-markdown/html-list-markdown.ts
index 8e853f19f6b..ba4bfe12265 100644
--- a/mobile/src/components/rich-markdown/html-list-markdown.ts
+++ b/mobile/src/components/rich-markdown/html-list-markdown.ts
@@ -22,6 +22,16 @@ export function directNestedLists(item: Element): Element[] {
   return Array.from(item.querySelectorAll('ul, ol')).filter((list) => list.closest('li') === item)
 }
 
+/**
+ * Whether an element carries a list that no list item owns, and so is a block of its own.
+ *
+ * The mirror of `directNestedLists`: a list under an `li` is that item's, serialized at its own
+ * indentation, and any other list is a block wherever the engine put it — including inside a `<p>`.
+ */
+export function holdsUnownedList(element: Element): boolean {
+  return Array.from(element.querySelectorAll('ul, ol')).some((list) => list.closest('li') === null)
+}
+
 /**
  * A list element as markdown, two spaces deeper per level of nesting.
  *
diff --git a/mobile/src/components/rich-markdown/markdown-round-trip.test.ts b/mobile/src/components/rich-markdown/markdown-round-trip.test.ts
index 2cd84c1d627..cc8f0767029 100644
--- a/mobile/src/components/rich-markdown/markdown-round-trip.test.ts
+++ b/mobile/src/components/rich-markdown/markdown-round-trip.test.ts
@@ -29,6 +29,29 @@ function surface(markdown: string, options: { editable?: boolean } = {}) {
   return { scope, editor, html: editor.innerHTML }
 }
 
+/**
+ * The surface holding a paragraph that carries a list inside it, which is what an engine leaves.
+ *
+ * Built through the paragraph's own `innerHTML` rather than the editor's: the HTML parser closes a
+ * `<p>` before a `<ul>`, so `editor.innerHTML = '<p><ul>...'` gives two siblings and would measure
+ * the flat shape while claiming to measure the nested one. Each case asserts the nesting it got.
+ */
+function nestedListSurface(paragraphMarkup: string) {
+  document.body.innerHTML = RICH_MARKDOWN_EDITOR_MARKUP
+  const scope = createRichMarkdownEditorScope()
+  scope.editable = true
+  startEditorSurface(scope)
+  const editor = document.getElementById('editor')!
+  const paragraph = document.createElement('p')
+  paragraph.innerHTML = paragraphMarkup
+  editor.append(paragraph)
+  return { scope, editor }
+}
+
+/** The item markup WebKit wraps the paragraph's text in: a styled span and a trailing break. */
+const webkitItem = (text: string) =>
+  `<li><span style="font-family: var(--font-sans);">${text}</span><br></li>`
+
 describe('the editor document, from markdown and back', () => {
   it('renders and serializes nested bullet, ordered and task lists with indentation intact', () => {
     const markdown = [
@@ -49,6 +72,45 @@ describe('the editor document, from markdown and back', () => {
     expect(currentMarkdown(scope)).toBe(markdown)
   })
 
+  it('serializes a bullet list the engine nested inside a paragraph, as the flat shape does', () => {
+    // Captured from the render rig, not written from memory: on WebKit 26.4 and Chromium 147 alike,
+    // `insertUnorderedList` over `<p>alpha</p>` leaves `<p><ul><li>alpha</li></ul></p>` rather than
+    // replacing the paragraph, and WebKit additionally wraps the item's text in a styled span.
+    const { scope, editor } = nestedListSurface(
+      `<ul>${webkitItem('alpha')}${webkitItem('beta')}</ul>`
+    )
+
+    expect(editor.querySelector('ul')?.parentElement?.tagName).toBe('P')
+    const markdown = ['- alpha', '- beta'].join('\n')
+    expect(currentMarkdown(scope)).toBe(markdown)
+    // The flat shape the renderer produces from that same source, so the two shapes agree.
+    expect(currentMarkdown(surface(markdown).scope)).toBe(markdown)
+  })
+
+  it('serializes a numbered list the engine nested inside a paragraph', () => {
+    // The same capture with the Numbered list command: `<p><ol><li>...</li></ol></p>` on both.
+    const { scope, editor } = nestedListSurface(
+      `<ol>${webkitItem('first')}${webkitItem('second')}</ol>`
+    )
+
+    expect(editor.querySelector('ol')?.parentElement?.tagName).toBe('P')
+    expect(currentMarkdown(scope)).toBe(['1. first', '2. second'].join('\n'))
+  })
+
+  it('reads a paragraph that holds a list and text around it as separate blocks', () => {
+    // The trailing half is captured: leaving the list with two returns and typing puts the text in
+    // a `<div>` beside the `<ul>`, both still inside the one `<p>`. Text before the list is the
+    // same rule read forward — a run of inline content is a paragraph wherever it sits.
+    const { scope, editor } = nestedListSurface(
+      'before the list<ul><li>solo</li></ul><div>after the list</div>'
+    )
+
+    expect(editor.querySelector('ul')?.parentElement?.tagName).toBe('P')
+    expect(currentMarkdown(scope)).toBe(
+      ['before the list', '', '- solo', '', 'after the list'].join('\n')
+    )
+  })
+
   it('renders markdown entities as characters without double-escaping them', () => {
     const { html } = surface('R&D &amp; Sales and &lt;tag&gt;')