Regression in saving posts with Block Editor in 6.8

[margolisj]

I believe this to be an issue on the save portion of the rest API. When saving the raw post data in the visual editor, we often have empty JS/JSON objects in our data; when this is saved, they are being transformed into arrays. I believe this is being done via the save post rest end point as checking the data after saving a post shows the post content having arrays. We have also noticed this before when saving post data when the user saving the post is not an admin. With WP 6.8 this also happening with users that have admin privilege.

Example:

<!-- wp:nectar-blocks/button {"blockId":"block-RX3iloq4aK","bgColor":{"desktop":{"type":"solid","solidValue":"#2c80af","gradientValue":""},"tablet":{},"mobile":{},"hover":{}}} -->
<div class="wp-block-nectar-blocks-button nectar-blocks-button nectar-font-label" id="block-RX3iloq4aK"><a href="#" class="nectar__link nectar-blocks-button__inner"><span class="nectar-blocks-button__text"></span></a></div>
<!-- /wp:nectar-blocks/button -->

Which on save is being transformed into:

<!-- wp:nectar-blocks/button {"blockId":"block-RX3iloq4aK","bgColor":{"desktop":{"type":"solid","solidValue":"#af2c2c","gradientValue":""},"tablet":[],"mobile":[],"hover":[]}} -->
<div class="wp-block-nectar-blocks-button nectar-blocks-button nectar-font-label" id="block-RX3iloq4aK"><a href="#" class="nectar__link nectar-blocks-button__inner"><span class="nectar-blocks-button__text"></span></a></div>
<!-- /wp:nectar-blocks/button -->

If this is the expected behavior, is there a way to disable it, or otherwise filter out this functionality? This is fundamentally changing the data we send to the backend and then expect to be returned.

[wildworks]

Thanks for the report.

I found the same issue is submitted on the Gutenberg repo: ​https://github.com/WordPress/gutenberg/issues/69959

I'm going to close this ticket as "reported-upstream".

Trac ticket: https://core.trac.wordpress.org/ticket/63325
​https://github.com/WordPress/gutenberg/issues/69959

[Bernhard Reiter]

@wildworks Wouldn't it make sense to keep the ticket open? The fix will likely need to be made in Core, and it might help to have the ticket around so folks are aware in case it's decided that it should go into 6.8.x...

Thank you for working on a fix for this!

Since I apparently introduced this in https://core.trac.wordpress.org/changeset/59523, it might be sufficient to fix apply_block_hooks_to_content to respect the array or object nature of block attributes -- especially since AFAICS it might be really hard to fix serialize_block_attributes to return the correct kind of attribute even if it doesn't have sufficient context.

I'll see if I can figure out a fix for apply_block_hooks_to_content.

[wildworks]

Wouldn't it make sense to keep the ticket open?

That's right, I initially closed this because I thought this issue was directly related to the Block Hooks API, but it actually looks like it needs to be fixed in core, so I'm reopening it.

[desrosj]

With 6.8.1 RC1 due out in a few hours, I'm going to punt this as there's still more work required.

In serialize_attributes, if the value of an attribute is an empty array(), it will always be JSON-encoded into [] even though it could be an associative array (which should be encoded into {}) — but that distinction just doesn’t exist for _empty_ arrays in PHP. In other words, PHP simply doesn’t have enough (meta) information to know what it should encode an empty array() into.

So our best bet is to infer the attribute type from the block definition. This isn't great, since serialize_attributes is a low-level function that was previously able to operate without any knowledge about higher-level block semantics, but alas.

Note that this also violates function signature parity of serialize_attributes (PHP) and serializeAttributes (JS), as the PHP version needs to no the block name in order to look up its block definition. Maybe the violation is permissible here, but I’d like to hear @dmsnell’s optionion before proceeding. Alternatively, we can move the logic I’m introducing here one level up, i.e. into get_comment_delimited_block_content. This should still be sufficient for all practical purposes; it is also currently the only callsite of serialize_attributes in the entire Core codebase.

Trac ticket: https://core.trac.wordpress.org/ticket/63325
Gutenberg issue: ​https://github.com/WordPress/gutenberg/issues/69959
Prior art: ​https://github.com/WordPress/wordpress-develop/pull/8735

Right, so this is a tricky one; I haven’t found a way to tackle it at apply_block_hooks_to_content level.

Here's an attempt based on inferring the attribute type from the block definition: ​https://github.com/WordPress/wordpress-develop/pull/8789

Alternatively, we can move the logic I’m introducing here one level up, i.e. into get_comment_delimited_block_content. This should still be sufficient for all practical purposes; it is also currently the only callsite of serialize_attributes in the entire Core codebase.

That sounds more compelling to me as this would leave serialize_attributes with the old signature, helping to avoid adding a bit confusing behavior. I was looking at the code, and it doesn't look it would be straightforward to map an empty array to an empty object during block parsing. It feels like that would be the best place to ensure proper type recognition.

BTW this still doesn't solve the problem for empty arrays that are nested inside objects, which was the original example in ​https://github.com/WordPress/gutenberg/issues/69959, e.g.

<div class="wp-block-nectar-blocks-button nectar-blocks-button nectar-font-label" id="block-RX3iloq4aK"><a href="#" class="nectar__link nectar-blocks-button__inner"><span class="nectar-blocks-button__text"></span></a></div>

Note that bgColor is an object; its tablet, mobile, and hover properties are all empty objects ({}). These will currently get transformed into []s, as block.json doesn't contain "full" schema information about object-type attributes.

The only piece of information that we _might_ be able to use if the default, e.g. ​https://github.com/WordPress/wordpress-develop/blob/3f9317c2aa2ba650c0d930acaca1bb840c95a6da/src/wp-includes/blocks/query/block.json#L14-L32

Might be material for a follow-up, though, as it gets us into slightly more "heuristic" territory.

This should still be sufficient for all practical purposes; it is also currently the only callsite of serialize_attributes in the entire Core codebase.

If I understand correctly, if consumers do not specify the optional second argument, the return value will be the same as before, right? Because this function seems to be used by some plugins.

​https://wpdirectory.net/search/01JTTHDK6PXHAWX44Z2HXE3SEH

Thanks for tackling this — it's a subtle edge case with real consequences, especially for block attributes that depend on accurate type preservation.
I agree that introducing $block_name into serialize_attributes() complicates its previously low-level, stateless nature. Moving this logic into get_comment_delimited_block_content() seems like a cleaner solution and keeps serialize_attributes() generic and reusable.
Also appreciate the note about nested empty arrays — definitely sounds like it warrants a follow-up, maybe with enhancements to how block schemas are defined or parsed (especially from block.json).

BTW this still doesn't solve the problem for empty arrays that are nested inside objects, which was the original example in ​https://github.com/WordPress/gutenberg/issues/69959, e.g.

One way to solve it would be to introduce another field similar to enum that would allow defining the JSON schema describing the shape of the objects and arrays. In fact, it wouldn't differ that much from what enum offers today, but with greater flexibility.

The root cause lies in how block attributes gets decoded from the serialized JSON:

​https://github.com/WordPress/wordpress-develop/blob/dcba8735453a87c04026468eed4f809c630738c0/src/wp-includes/class-wp-block-parser.php#L280

In effect, empty arrays and empty objects become indistinguishable in PHP.

The same case in JavaScript gets handled differently:

​https://github.com/WordPress/gutenberg/blob/918d152f25248a53e3fedd1ec3af2eadeb038e9d/packages/block-serialization-default-parser/src/index.js#L415

​https://github.com/WordPress/gutenberg/blob/918d152f25248a53e3fedd1ec3af2eadeb038e9d/packages/block-serialization-default-parser/src/index.js#L373

Example:

BTW this still doesn't solve the problem for empty arrays that are nested inside objects, which was the original example in ​WordPress/gutenberg#69959, e.g.

One way to solve it would be to introduce another field similar to enum that would allow defining the JSON schema describing the shape of the objects and arrays. In fact, it wouldn't differ that much from what enum offers today, but with greater flexibility.

Indeed. But that sounds like more of a longer-term solution to me, as it'll require both updating the block.json schema (to allow a JSON schema for its attribute types); Core blocks to be updated to specify that attribute type schema where appropriate; and 3rd party block authors to adopt it as well.

What happens if we skip serializing empty arrays? I’m unnerved for the same reasons you are about reading the block registry on serialization:

• it doesn’t solve this problem; it only helps in some cases where the block attribute definitions are available and where the attributes are at the top-level
• it couples a behavior that works universally with one that’s now highly dependent on the runtime environment, because my system will serialize blocks differently than yours and my system might even serialize differently based on what plugins are activated, meaning this is different now than it was an hour ago

I’d be interested in approaching this from the other side: let’s declare a hidden attribute on all parsed map types and use the presence of that attribute upon serialization. Though, this is still incomplete because people will be running things like $block['attrs']['settings'] = []

// decoding attributes on parse
foreach ( walk_parsed_attributes() as $name => &$value ) {
   if ( $value instance stdClass ) {
      $value['__wp_meta__is_js_map'] = true;
   }
}

// serializing
if ( is_array( $value ) && isset( $value['__wp_meta__is_js_map'] ) ) {
   unset( $value['__wp_meta__is_js_map'] );
   $value = (stdClass) $value;
}

@dmsnell Interesting idea 🤔 I was trying to find a way to add that sort of meta information (i.e. array or object?) but didn't think of that. I like it, it's pretty straight-forward!

I'll give that a try 👍 (Not sure when I'll get around to; I'm about to wrap up my week and will be pretty busy next week.)

Very interesting idea. Unless I misunderstood what you proposed, my only concern is that the problem exists at the JSON parsing level in PHP, so this meta information would have to be serialized in HTML on the client. In effect, it would also need to get removed during parsing on the client before passing the data to the block editor. It might require very detailed testing to ensure it doesn't break anything when this special property leaks somewhere.