fix(mobile): serialize a list the engine nested inside a paragraph (#22145)

`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
This commit is contained in:
Jinwoo Hong
2026-09-22 01:11:13 -04:00
committed by GitHub
parent 7650abe224
commit 35897da0aa
4 changed files with 172 additions and 6 deletions
@@ -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 {
@@ -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)
@@ -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.
*
@@ -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;')