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 `
` 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 `
` 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 `
` 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, `
${body}
`) + 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', 'plain again
') + await setContent(page, `- ${body}`, `${body}
` 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 `
`. + */ +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 + * `
` before a `
alpha
` leaves ``. 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