From 5c788dff22fcc72455e36c5fd3e55d0120f4692f Mon Sep 17 00:00:00 2001 From: Chris Huber Date: Mon, 26 Oct 2026 02:24:52 -0400 Subject: [PATCH] Keep card and callout content through conversion and canonicalize A class-bearing card
converted to an empty core/group and was reported as success (#3458). The catalog's container converters (callout, details, media-text) built their inner markup by falling back to raw HTML when no catalog pattern matched inside. A container block saves only its inner blocks, so canonicalize's re-save of that group discarded the image, heading and text and was adopted as an info-level normalization. Container inner content is now always block markup: unmatched inner HTML becomes the same html-fallback island an unmatched top-level element gets. The callout group keeps the authored classes the site's CSS targets instead of replacing them with an is-style- class no theme registers. Canonicalize gains one guard at the owning layer: when an invalid block's re-save would drop words or media sources it showed, the block is kept as written (serialize() emits an invalid block's original source, as the editor does until a person recovers it), listed in preservedBlocks, and reported as an invalid_block_preserved warning. The new content-preservation table converts the issue's cards and every compose fixture through convertReport on the real worker and checks the output keeps the input's words and media. It fails on trunk (16 of 30), with the catalog change reverted (13), with the guard disabled (3), and with raw inner HTML restored (12). --- .../blocks-engine/src/__fixtures__/cases.json | 8 +- .../__tests__/content-preservation.test.ts | 108 ++++++++++++++++++ .../src/__tests__/pool.contract.test.ts | 1 + packages/blocks-engine/src/catalog.ts | 15 +-- packages/blocks-engine/src/pool/types.ts | 1 + packages/blocks-engine/src/report/findings.ts | 9 ++ packages/blocks-engine/src/report/schema.ts | 1 + packages/blocks-engine/src/wp/canonicalize.ts | 64 ++++++++--- 8 files changed, 182 insertions(+), 25 deletions(-) create mode 100644 packages/blocks-engine/src/__tests__/content-preservation.test.ts diff --git a/packages/blocks-engine/src/__fixtures__/cases.json b/packages/blocks-engine/src/__fixtures__/cases.json index bab4edac9..4f26aa16b 100644 --- a/packages/blocks-engine/src/__fixtures__/cases.json +++ b/packages/blocks-engine/src/__fixtures__/cases.json @@ -109,13 +109,13 @@ "id": "compose.generic-details", "op": "compose", "input": "
Q

A

", - "expected": "\n
Q

A

\n" + "expected": "\n
Q\n\n

A

\n\n
\n" }, { "id": "compose.generic-callout", "op": "compose", "input": "

Note

body

", - "expected": "\n

Note

body

\n" + "expected": "\n
\n\n

Note

body

\n\n
\n" }, { "id": "compose.generic-pullquote", @@ -133,7 +133,7 @@ "id": "compose.generic-media-text", "op": "compose", "input": "
\"A\"/

Title

copy

", - "expected": "\n
\"A\"/

Title

copy

\n" + "expected": "\n
\"A\"/
\n

Title

copy

\n
\n" }, { "id": "compose.sample-youtube-iframe", @@ -145,7 +145,7 @@ "id": "compose.sample-callout-div", "op": "compose", "input": "

Shipping pause

Orders resume Monday.

", - "expected": "\n

Shipping pause

Orders resume Monday.

\n" + "expected": "\n
\n\n

Shipping pause

Orders resume Monday.

\n\n
\n" }, { "id": "compose.sample-semantic-section-fallback", diff --git a/packages/blocks-engine/src/__tests__/content-preservation.test.ts b/packages/blocks-engine/src/__tests__/content-preservation.test.ts new file mode 100644 index 000000000..04efea3db --- /dev/null +++ b/packages/blocks-engine/src/__tests__/content-preservation.test.ts @@ -0,0 +1,108 @@ +import { afterAll, describe, expect, it } from 'vitest'; + +import cases from '../__fixtures__/cases.json' with { type: 'json' }; +import { convertReport } from '../index.js'; +import { createWorker } from '../pool/pool.js'; +import { canonicalize } from '../wp/canonicalize'; + +type Fixture = { id: string; op: string; input: string }; + +// What a reader of the page sees: its words, and the media it loads (a +// source may become an embed URL, so any occurrence of it counts). +function words(html: string): string[] { + return html + .replace(//gu, ' ') + .replace(/<[^>]+>/gu, ' ') + .split(/\s+/u) + .filter(Boolean); +} + +function missing(input: string, output: string): string[] { + const left = words(output); + const lostWords = words(input).filter((word) => { + const at = left.indexOf(word); + if (at === -1) return true; + left.splice(at, 1); + return false; + }); + const lostSources = Array.from(input.matchAll(/\ssrc="([^"]+)"/gu), (match) => match[1]).filter( + (source) => !output.includes(source), + ); + return [...lostWords, ...lostSources]; +} + +const pool = createWorker({ size: 1 }); +afterAll(() => pool.stop()); + +const card = + '
A

Coco Rope Tug

Braided cotton rope.

'; + +const inputs: Array<[string, string]> = [ + ['a card', card], + ['a card with a price and a link', '

Tug

$12

Add
'], + ['cards in a grid', `
${card}${card}
`], + ['a callout', '

Heads up.

'], + ['an accordion item', '

Shipping?

Two days.

'], + ...(cases as Fixture[]) + .filter((fixture) => fixture.op === 'compose') + .map((fixture): [string, string] => [fixture.id, fixture.input]), +]; + +describe('conversion keeps what the page shows', () => { + it.each(inputs)('%s', async (_name, html) => { + const report = await convertReport(html, { url: 'https://x.test/' }, { pool }); + + expect(missing(html, report.blockMarkup)).toEqual([]); + // Converted blocks hold their content themselves; none rely on the + // canonicalize guard keeping an invalid block. + expect(report.diagnostics.map((finding) => finding.code)).not.toContain('invalid_block_preserved'); + }); + + it('keeps a card class the site styles and invents no block style', async () => { + const report = await convertReport(card, { url: 'https://x.test/' }, { pool }); + + expect(report.blockMarkup).toContain('paw-card'); + expect(report.blockMarkup).not.toContain('is-style-'); + expect(report.status).toBe('success_with_warnings'); + }); +}); + +describe('canonicalize keeps an invalid block whose re-save drops content', () => { + const lossy: Array<[string, string, string[]]> = [ + [ + 'a group holding raw HTML', + '\n

Kept words

\n', + ['core/group'], + ], + [ + 'the same group inside columns', + '\n
\n
\n

Kept words

\n
\n
\n', + ['core/group'], + ], + ]; + + it.each(lossy)('%s', (_name, markup, preserved) => { + const result = canonicalize(markup); + + expect(missing(markup, result.html)).toEqual([]); + expect(result.preservedBlocks).toEqual(preserved); + }); + + it('still fixes an invalid block whose re-save keeps its content', () => { + const markup = '\n

Hello there

\n'; + const result = canonicalize(markup); + + expect(result.html).toContain('

Hello there

'); + expect(result.preservedBlocks).toEqual([]); + }); + + it('reports a kept block as a warning', async () => { + const markup = lossy[0][1]; + const report = await convertReport(markup, { url: '' }, { pool }); + + expect(report.status).toBe('success_with_warnings'); + expect(report.diagnostics).toContainEqual( + expect.objectContaining({ code: 'invalid_block_preserved', severity: 'warning', selector: 'core/group' }), + ); + }); +}); diff --git a/packages/blocks-engine/src/__tests__/pool.contract.test.ts b/packages/blocks-engine/src/__tests__/pool.contract.test.ts index 89157be70..a66701b39 100644 --- a/packages/blocks-engine/src/__tests__/pool.contract.test.ts +++ b/packages/blocks-engine/src/__tests__/pool.contract.test.ts @@ -58,6 +58,7 @@ describe('worker pool contract', () => { }[]; htmlIslandDistinctCount?: number; htmlIslandOccurrencesTruncated?: boolean; + preservedBlocks?: string[]; degraded: boolean; }>(); diff --git a/packages/blocks-engine/src/catalog.ts b/packages/blocks-engine/src/catalog.ts index e9120af4e..37c6a8d6a 100644 --- a/packages/blocks-engine/src/catalog.ts +++ b/packages/blocks-engine/src/catalog.ts @@ -130,10 +130,11 @@ function recurseInner( ctx: ConversionContext, htmlFallback: HtmlFallbackEmitter, ): string { - const nested = genericHtmlToBlocks(html, ctx, htmlFallback); - if (nested && nested.trim()) return `\n${nested}\n`; - const clean = sanitize(html).trim(); - return clean ? clean : ''; + if (!sanitize(html).trim()) return ''; + // Inner content is always block markup: a container block saves only its + // inner blocks, so raw HTML inside it is discarded on the editor's re-save. + const nested = genericHtmlToBlocks(html, ctx, htmlFallback) ?? htmlFallback(html); + return `\n${nested}\n`; } const HEADING_TAGS = new Set(['h1', 'h2', 'h3', 'h4', 'h5', 'h6']); @@ -189,11 +190,11 @@ function tryCallout( if (!CALLOUT_RE.test(cls)) return null; const inner = recurseInner($el.html() ?? '', ctx, htmlFallback); if (!inner.trim()) return null; - const matchedClass = (cls.match(CALLOUT_RE) || [])[0] ?? 'callout'; - const className = `wp-block-group is-style-${matchedClass}`; + // The authored classes are what the site's CSS targets; keep them as is. + const className = cls.split(/\s+/).filter((name) => name && name !== 'wp-block-group').join(' '); return ( `\n` + - `
${inner}
\n` + + `
${inner}
\n` + `` ); } diff --git a/packages/blocks-engine/src/pool/types.ts b/packages/blocks-engine/src/pool/types.ts index 47d129cf9..2eef18f60 100644 --- a/packages/blocks-engine/src/pool/types.ts +++ b/packages/blocks-engine/src/pool/types.ts @@ -21,6 +21,7 @@ export interface FixResult { htmlIslandOccurrences?: HtmlIslandOccurrence[]; htmlIslandDistinctCount?: number; htmlIslandOccurrencesTruncated?: boolean; + preservedBlocks?: string[]; degraded: boolean; } diff --git a/packages/blocks-engine/src/report/findings.ts b/packages/blocks-engine/src/report/findings.ts index d369a3b83..afe5d45e3 100644 --- a/packages/blocks-engine/src/report/findings.ts +++ b/packages/blocks-engine/src/report/findings.ts @@ -267,6 +267,15 @@ export function buildReport({ message: issue, })); + for (const name of fixResult.preservedBlocks ?? []) { + diagnostics.push({ + code: 'invalid_block_preserved', + severity: 'warning', + message: `${name} kept as written: the editor's re-save would drop content it shows.`, + selector: name, + }); + } + if (fixResult.degraded) { diagnostics.push({ code: 'conversion_degraded', diff --git a/packages/blocks-engine/src/report/schema.ts b/packages/blocks-engine/src/report/schema.ts index 35f75fc60..ee717c7f3 100644 --- a/packages/blocks-engine/src/report/schema.ts +++ b/packages/blocks-engine/src/report/schema.ts @@ -23,6 +23,7 @@ export const CONVERSION_FINDING_CODES = [ 'conversion_degraded', 'fallback_inventory_truncated', 'content_dropped', + 'invalid_block_preserved', 'hero_image_layering_risk', 'body_text_promoted_to_heading', 'heading_inside_list_item', diff --git a/packages/blocks-engine/src/wp/canonicalize.ts b/packages/blocks-engine/src/wp/canonicalize.ts index f50573e00..8565e9730 100644 --- a/packages/blocks-engine/src/wp/canonicalize.ts +++ b/packages/blocks-engine/src/wp/canonicalize.ts @@ -27,6 +27,8 @@ export type CanonicalizeResult = { htmlIslandOccurrences: HtmlIslandOccurrence[]; htmlIslandDistinctCount: number; htmlIslandOccurrencesTruncated: boolean; + /** Invalid blocks kept as written because their re-save drops content. */ + preservedBlocks: string[]; degraded: boolean; }; @@ -271,12 +273,48 @@ function commentAttrsAltered( // attributes (content etc.) never appear in comment attrs, so author intent // can't clobber them; createBlock() drops keys unknown to the current schema. // +// The words and media sources a reader sees in serialized markup, counted. +function visibleContent(markup: string): Map { + const counts = new Map(); + const sources = Array.from(markup.matchAll(/\ssrc="([^"]+)"/gu), (match) => `src:${match[1]}`); + const words = markup + .replace(//gu, ' ') + .replace(/<[^>]+>/gu, ' ') + .split(/\s+/u) + .filter(Boolean); + for (const item of [...sources, ...words]) counts.set(item, (counts.get(item) ?? 0) + 1); + return counts; +} + +function dropsContent(before: string, after: string): boolean { + const kept = visibleContent(after); + for (const [item, count] of visibleContent(before)) { + if ((kept.get(item) ?? 0) < count) return true; + } + return false; +} + +type BlockFixer = { + getBlockAttributes: WordpressBlocks['getBlockAttributes']; + createBlock: WordpressBlocks['createBlock']; + serialize: WordpressBlocks['serialize']; + preserved: string[]; +}; + +// +// An invalid block's re-save is rebuilt from its attributes and inner +// blocks, so anything else its markup held is lost (a container's raw inner +// HTML, for one). When that re-save would drop words or media the block +// showed, the block is kept as written: serialize() emits an invalid block's +// original source, which is what the editor itself does until a person +// chooses to recover it. +// function fixBlockRecursively( block: ParsedBlock, rawBlock: RawBlock | undefined, - getBlockAttributes: WordpressBlocks['getBlockAttributes'], - createBlock: WordpressBlocks['createBlock'], + fixer: BlockFixer, ): { block: ParsedBlock; wasFixed: boolean } { + const { getBlockAttributes, createBlock, serialize } = fixer; const fixedInnerBlocks = []; if (block.innerBlocks && block.innerBlocks.length > 0) { @@ -295,12 +333,7 @@ function fixBlockRecursively( } const rawInnerBlock = rawIndex < rawInner.length ? rawInner[rawIndex++] : undefined; - const result = fixBlockRecursively( - innerBlock, - rawInnerBlock, - getBlockAttributes, - createBlock, - ); + const result = fixBlockRecursively(innerBlock, rawInnerBlock, fixer); fixedInnerBlocks.push(result.block); } } @@ -328,6 +361,11 @@ function fixBlockRecursively( fixedInnerBlocks.length > 0 ? fixedInnerBlocks : undefined, ); + if (block.isValid === false && dropsContent(serialize([block]), serialize([fixedBlock]))) { + fixer.preserved.push(block.name); + return { block, wasFixed: false }; + } + return { block: fixedBlock, wasFixed: true }; } @@ -464,6 +502,7 @@ export function canonicalize(markup: string): CanonicalizeResult { const fixedIssues: string[] = []; collectIssues(blocks, fixedIssues); + const fixer: BlockFixer = { getBlockAttributes, createBlock, serialize, preserved: [] }; let rawIndex = 0; const fixedBlocks = blocks.map((block) => { while ( @@ -474,12 +513,7 @@ export function canonicalize(markup: string): CanonicalizeResult { } const rawBlock = rawIndex < rawBlocks.length ? rawBlocks[rawIndex++] : undefined; - return fixBlockRecursively( - block, - rawBlock, - getBlockAttributes, - createBlock, - ).block; + return fixBlockRecursively(block, rawBlock, fixer).block; }); let fixedHtml = serialize(fixedBlocks); @@ -512,6 +546,7 @@ export function canonicalize(markup: string): CanonicalizeResult { changed: wasChanged, fixedIssues, ...inventory, + preservedBlocks: fixer.preserved, degraded: false, }; } catch (error) { @@ -527,6 +562,7 @@ export function canonicalize(markup: string): CanonicalizeResult { htmlIslandOccurrences: [], htmlIslandDistinctCount: 0, htmlIslandOccurrencesTruncated: false, + preservedBlocks: [], degraded: true, }; }