Block Bindings: Preserve nested inner blocks when binding rich text#12113
Block Bindings: Preserve nested inner blocks when binding rich text#12113cbravobernal wants to merge 1 commit into
Conversation
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
1a18473 to
d095e16
Compare
The compat code in this PR is a plugin-only shim that preserves nested lists when binding list item content. It is not backported to Core: the Core fix is a different, general implementation in WordPress/wordpress-develop#12113, after which this workaround can be removed. Classify the PR with the `No Core Sync Required` label instead of a backport-changelog entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
4f3c8d1 to
b03fe11
Compare
The compat code in this PR is a plugin-only shim that preserves nested lists when binding list item content. It is not backported to Core: the Core fix is a different, general implementation in WordPress/wordpress-develop#12113, after which this workaround can be removed. Classify the PR with the `No Core Sync Required` label instead of a backport-changelog entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2f46385 to
9cf3cf6
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Code review
No high-confidence issues found. The core mechanism — recording byte offsets of inner blocks during render, clamping rich-text replacement at the first offset inside the matched element, and invalidating offsets once a replacement changes the markup — was checked for off-by-one errors, coordinate-space mismatches, stale-offset bugs, and regressions against the history of Two minor, non-blocking observations:
wordpress-develop/src/wp-includes/class-wp-block.php Lines 451 to 456 in edac6d1
wordpress-develop/tests/phpunit/tests/block-bindings/render.php Lines 722 to 725 in edac6d1 🤖 Generated with Claude Code (model: - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
There was a problem hiding this comment.
I'm not very familiar with block bindings, but I'll try to provide helpful feedback.
Is it possible somehow to produce a list item child in the replacement? As noted in the comment, that difficult is why set_inner_html does not yet exist, because if an <li is inserted in <li>[[set this inner HTML]] <ul><li>nested</li></ul></li>, then the outside HTML structure is altered and the contract is broken. That's the main danger of working with something like an LI, <li><li> is two adjacent LI, not nested.
| * Stop at the first inner block that renders inside this element so | ||
| * its markup is preserved. The block's own rich text always precedes | ||
| * its inner blocks, so replacing up to the first inner block offset | ||
| * replaces only that rich text. Offsets are recorded during render in | ||
| * the same byte coordinates as this fragment, and are in ascending | ||
| * order, so the first match is the earliest inner block. | ||
| * | ||
| * The lower bound is inclusive of `$start`: when an inner block | ||
| * begins immediately, with no leading rich text, the (empty) rich | ||
| * text is still replaced instead of the inner block markup. |
There was a problem hiding this comment.
Does this always hold? Specifically:
The block's own rich text always precedes its inner blocks, so replacing up to the first inner block offset replaces only that rich text.
Another way we might do this is to find all the text nodes from the block start to the first inner block and that becomes the replacement range. We could use a Tag Processor on that range to ensure that everything is, in fact, text.
There was a problem hiding this comment.
For core/list-item, yes. The block renders its innerContent in order: own rich-text first, then the inner-block placeholders. The offsets are recorded in that same render loop, so the first offset is probably where the rich text ends.
For an arbitrary block or manual markup, it's not guaranteed. I'll add a test.
On the Tag Processor idea
On finding the range by walking text nodes: the range isn't always text. It can contain raw block markup that should be replaced, right next to a real inner block that shouldn't, and they look identical once rendered. The offset is the only thing that knows which <ul> came from an actual inner block.
Still, I would need to add a test to be really sure about it.
Thanks for the review @sirreal ! You may not be block bindings familiar, but still a WordPress - HTML API expert 😄 . Yes, it is possible. Is already also possible in paragraphs, headings and button, which is already happening since 6.5. I guess we consider block bindings a developer tool, and it's developer's risk to add unbalanced tags that could break different blocks. Maybe is something we should work on on a different PR, so that way is only a fix for all blocks with this issue rather than coupling to this list-item only problem. |
4186b05 to
48a00b5
Compare
sirreal
left a comment
There was a problem hiding this comment.
Looks good. We've discussed a few risky scenarios, but they already exist and don't have good solutions right now.
48a00b5 to
085c57d
Compare
085c57d to
a176e2f
Compare
`WP_Block::replace_html()` replaced the entire element matched by a rich-text attribute's selector, dropping any markup produced by inner blocks rendered inside that same element (e.g. a List nested inside a List Item). Record, during render, the byte offset where each inner block's rendered output begins, and pass them to `replace_rich_text()` so the replacement stops at the first inner block inside the selector. A block's own rich text always precedes its inner blocks, so only that rich text is replaced and inner block output is preserved verbatim. Offsets are collected only when the block has bound attributes, and are dropped once a replacement shifts byte positions. The change is block-agnostic; tests cover it with an arbitrary block type and the rich-text processor, plus List Item integration cases (nested lists, multibyte fallbacks, ordered lists, pattern overrides and inner-block-before- text) that build on the List Item content binding. Props sauliusv, ockham, cbravobernal. See https://core.trac.wordpress.org/ticket/65406
a176e2f to
424ca0c
Compare
Trac ticket: https://core.trac.wordpress.org/ticket/65406
The inner-block fix.
WP_Block::replace_html()replaced the entire element matched by a rich-text attribute's selector, dropping any markup produced by inner blocks rendered inside that element (e.g. a List nested inside a List Item).List Item content binding enablement already landed in trunk (#12204), so this PR is now a clean, standalone fix — a single commit.
The fix
A block's own rich text always comes before its inner blocks, so we only need the byte offset where the first inner block begins.
WP_Block::render()already concatenatesinner_content(own HTML + each inner block's rendered output), so it records those offsets for free (only when the block has bound attributes). They are passed toreplace_rich_text(), which clamps the replacement to the first inner block inside the selector. A replacement that shifts byte positions drops the offsets, so later attributes fall back to offset-free replacement.The change is block-agnostic: tested with an arbitrary block type plus a rich-text processor unit test. List Item integration tests (nested lists, multibyte fallbacks, ordered lists, pattern overrides, inner-block-before-text) build on the enablement now in trunk.
Gutenberg counterpart: WordPress/gutenberg#78991 (runtime-gated compat workaround).