Block Supports: Add (failing) surrounding HTML tests

[sirreal]

Description

Add tests that block content with appended, prepended, and surrounding HTML is preserved and block classes are correctly added to the block content. This includes a test added and fixed in #25028 that will fail. It includes two additional tests for prepended and surrounding HTML that will fail and demonstrate some issues with the gutenberg_apply_block_supports function.

It raises an interesting question about render_callback with regards to the block supports functionality. Can block supports functionality work well with arbitrary render_callback output?

The failing tests (note that 1 is expected to be fixed with #25028):

1) Block_Supported_Styles_Test::test_render_block_includes_appended_html
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<p class="wp-block-example">Hello from the block content!</p><div>Appended</div>'
+'<p class="wp-block-example">Hello from the block content!</p>'

2) Block_Supported_Styles_Test::test_render_block_includes_prepended_html
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<div>Prepended</div><p class="wp-block-example">Hello from the block content!</p>'
+'<div class="wp-block-example">Prepended</div>'

3) Block_Supported_Styles_Test::test_render_block_includes_surrounding_html
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'<div>Prepended</div><p class="wp-block-example">Hello from the block content!</p><div>Appended</div>'
+'<div class="wp-block-example">Prepended</div>'

[github-actions]

Size Change: 0 B

Total Size: 1.17 MB

ℹ️ View Unchanged

Filename	Size	Change
build/a11y/index.js	1.14 kB	0 B
build/annotations/index.js	3.67 kB	0 B
build/api-fetch/index.js	3.41 kB	0 B
build/autop/index.js	2.82 kB	0 B
build/blob/index.js	620 B	0 B
build/block-directory/index.js	7.99 kB	0 B
build/block-directory/style-rtl.css	953 B	0 B
build/block-directory/style.css	952 B	0 B
build/block-editor/index.js	128 kB	0 B
build/block-editor/style-rtl.css	11.1 kB	0 B
build/block-editor/style.css	11.1 kB	0 B
build/block-library/editor-rtl.css	8.57 kB	0 B
build/block-library/editor.css	8.57 kB	0 B
build/block-library/index.js	137 kB	0 B
build/block-library/style-rtl.css	7.47 kB	0 B
build/block-library/style.css	7.47 kB	0 B
build/block-library/theme-rtl.css	741 B	0 B
build/block-library/theme.css	742 B	0 B
build/block-serialization-default-parser/index.js	1.88 kB	0 B
build/block-serialization-spec-parser/index.js	3.1 kB	0 B
build/blocks/index.js	47.7 kB	0 B
build/components/index.js	200 kB	0 B
build/components/style-rtl.css	15.5 kB	0 B
build/components/style.css	15.5 kB	0 B
build/compose/index.js	9.67 kB	0 B
build/core-data/index.js	12.3 kB	0 B
build/data-controls/index.js	1.29 kB	0 B
build/data/index.js	8.55 kB	0 B
build/date/index.js	5.38 kB	0 B
build/deprecated/index.js	772 B	0 B
build/dom-ready/index.js	568 B	0 B
build/dom/index.js	4.48 kB	0 B
build/edit-navigation/index.js	11.7 kB	0 B
build/edit-navigation/style-rtl.css	1.16 kB	0 B
build/edit-navigation/style.css	1.16 kB	0 B
build/edit-post/index.js	305 kB	0 B
build/edit-post/style-rtl.css	6.26 kB	0 B
build/edit-post/style.css	6.25 kB	0 B
build/edit-site/index.js	17.1 kB	0 B
build/edit-site/style-rtl.css	3.06 kB	0 B
build/edit-site/style.css	3.06 kB	0 B
build/edit-widgets/index.js	12 kB	0 B
build/edit-widgets/style-rtl.css	2.46 kB	0 B
build/edit-widgets/style.css	2.45 kB	0 B
build/editor/editor-styles-rtl.css	492 B	0 B
build/editor/editor-styles.css	493 B	0 B
build/editor/index.js	45.6 kB	0 B
build/editor/style-rtl.css	3.81 kB	0 B
build/editor/style.css	3.81 kB	0 B
build/element/index.js	4.65 kB	0 B
build/escape-html/index.js	733 B	0 B
build/format-library/index.js	7.71 kB	0 B
build/format-library/style-rtl.css	547 B	0 B
build/format-library/style.css	548 B	0 B
build/hooks/index.js	2.13 kB	0 B
build/html-entities/index.js	621 B	0 B
build/i18n/index.js	3.57 kB	0 B
build/is-shallow-equal/index.js	710 B	0 B
build/keyboard-shortcuts/index.js	2.52 kB	0 B
build/keycodes/index.js	1.94 kB	0 B
build/list-reusable-blocks/index.js	3.12 kB	0 B
build/list-reusable-blocks/style-rtl.css	476 B	0 B
build/list-reusable-blocks/style.css	476 B	0 B
build/media-utils/index.js	5.32 kB	0 B
build/notices/index.js	1.79 kB	0 B
build/nux/index.js	3.4 kB	0 B
build/nux/style-rtl.css	671 B	0 B
build/nux/style.css	668 B	0 B
build/plugins/index.js	2.56 kB	0 B
build/primitives/index.js	1.41 kB	0 B
build/priority-queue/index.js	789 B	0 B
build/redux-routine/index.js	2.85 kB	0 B
build/rich-text/index.js	13.9 kB	0 B
build/server-side-render/index.js	2.77 kB	0 B
build/shortcode/index.js	1.7 kB	0 B
build/token-list/index.js	1.27 kB	0 B
build/url/index.js	4.06 kB	0 B
build/viewport/index.js	1.85 kB	0 B
build/warning/index.js	1.14 kB	0 B
build/wordcount/index.js	1.17 kB	0 B

_{compressed-size-action}

[ockham]

It raises an interesting question about render_callback with regards to the block supports functionality. Can block supports functionality work well with arbitrary render_callback output?

Maybe if we can find some sort of invariant 🤔 Can we assume that the 'actual' block wrapper always has a class name that identifies it? (wp-block-example in your example.) This might be available from $attributes, and we could maybe use it to select the 'root' element (and infer what constitutes the 'surrounding' HTML).

[ockham]

It raises an interesting question about render_callback with regards to the block supports functionality. Can block supports functionality work well with arbitrary render_callback output?

Maybe if we can find some sort of invariant Can we assume that the 'actual' block wrapper always has a class name that identifies it? (wp-block-example in your example.) This might be available from $attributes, and we could maybe use it to select the 'root' element (and infer what constitutes the 'surrounding' HTML).

Hmm, seems like we're injecting that class name right there. So that might be a dead end.

But there is some aspect of atomicity to blocks; after all, I think we're basically iterating over them (both client and server side, IIUC). So there's a point where we should be able to get the block (without any prepended/appended stuff) -- and I think that point is before render_callback is run. 🤔

Edit: Meh, OTOH, a lot of dynamic blocks really use render_callback for the actual block (nothing prepended or appended there) 🙁

[ockham]

It raises an interesting question about render_callback with regards to the block supports functionality. Can block supports functionality work well with arbitrary render_callback output?

Maybe if we can find some sort of invariant Can we assume that the 'actual' block wrapper always has a class name that identifies it? (wp-block-example in your example.) This might be available from $attributes, and we could maybe use it to select the 'root' element (and infer what constitutes the 'surrounding' HTML).

Hmm, seems like we're injecting that class name right there. So that might be a dead end.

Actually, maybe it isn't. I think we can potentially query by class name on $block_content before applying any of our modifications 🤔 This might be a rather reliable invariant (tho it's possible that it's not properly represented in our unit test fixtures).

[ockham]

It raises an interesting question about render_callback with regards to the block supports functionality. Can block supports functionality work well with arbitrary render_callback output?

Maybe if we can find some sort of invariant Can we assume that the 'actual' block wrapper always has a class name that identifies it? (wp-block-example in your example.) This might be available from $attributes, and we could maybe use it to select the 'root' element (and infer what constitutes the 'surrounding' HTML).

Hmm, seems like we're injecting that class name right there. So that might be a dead end.

Actually, maybe it isn't. I think we can potentially query by class name on $block_content before applying any of our modifications This might be a rather reliable invariant (tho it's possible that it's not properly represented in our unit test fixtures).

The main problem there could be that we have a method to automatically add an auto-generated block class name (if block_supports for className is set to true). That is at odds with requiring a unique way to identify the 'actual' block from the block author.

We might re-consider providing this helper to ensure that we invariably have a block class name available to select the block by.

cc/ @youknowriad

[youknowriad]

Here's how I see things:

There's no hard-rule saying that a block content is just a single element (wrapper), a block can render multiple elements or none as well both in the client and the server.

That said, for block-supports we can be more opinionated as these are helpers:

1- We can say, they hooks apply to all top-level elements of the block
2- We can also say, they only apply to the first element

I believe right now, the frontend does 1 while the backed does 2 (need to be verified) but for me, 2 seems the most logical.

[ockham]

Here's how I see things:

There's no hard-rule saying that a block content is just a single element (wrapper), a block can render multiple elements or none as well both in the client and the server.

That said, for block-supports we can be more opinionated as these are helpers:

1- We can say, they hooks apply to all top-level elements of the block
2- We can also say, they only apply to the first element

I believe right now, the frontend does 1 while the backed does 2 (need to be verified) but for me, 2 seems the most logical.

Yeah, that might make sense (though for consistency's sake, we might want to enforce the same constraints on all blocks, rather than just the ones that are affected by block-supports; so it's e.g. easier for an existing block to adopt block-supports).

The backend is special in that it can append scripts etc. The problem with 1. on the backend is then that we use the same logic to insert a bunch of classnames and styles that we don't really want in e.g. a <script /> tag.

[youknowriad]

The backend is special in that it can append scripts etc. The problem with 1. on the backend is then that we use the same logic to insert a bunch of classnames and styles that we don't really want in e.g. a <script /> tag.

I think frontend save functions can also append script and styles though less likely to happen.

[mcsf]

That said, for block-supports we can be more opinionated as these are helpers:

1- We can say, they hooks apply to all top-level elements of the block
2- We can also say, they only apply to the first element

I'd like to second this. And put this third option up for consideration:

3. These hooks apply to blocks that only output one top-level element.

Yeah, that might make sense (though for consistency's sake, we might want to enforce the same constraints on all blocks, rather than just the ones that are affected by block-supports; so it's e.g. easier for an existing block to adopt block-supports).

Riad recently suggested — and I think it's worth mentioning it here — that, now with apiVersion: 2 finalised as a signal for "light blocks", maybe we should require blocks interested in leveraging block-supports to conform to apiVersion: 2.