Broken Deprecation on core/block deletes templateLock when false

[kraftner]

Description

The latest deprecation migration for a core/group block discards the templateLock set on that block if the value isn't one of the string values but the also valid false.

Step-by-step reproduction instructions

1. Start a new post
2. Switch to the code editor
3. Paste the following:

   <!-- wp:group {"templateLock":false} -->
   <div class="wp-block-group"></div>
   <!-- /wp:group -->
   

4. Click outside the code editor to apply the deprecation migrations and see the templateLock gets removed instantly turning this into:

   <!-- wp:group -->
   <div class="wp-block-group"></div>
   <!-- /wp:group -->
   

---

If you want to see a more real world flow:

1. Create a post with a core/group block that doesn't have layout set but a templateLock value of false.
   There are two ways how to do this:
   • Create the block in an old WP version prior to the default layout, I think 6.0 should already do.
   • Manually edit the post_content in the database so it looks like this:

   <!-- wp:group {"templateLock":false} -->
   <div class="wp-block-group"></div>
   <!-- /wp:group -->
   

2. Open the post in WP 6.1.
3. Switch to Code Editor and see post_content now looks like this:

   <!-- wp:group -->
   <div class="wp-block-group"></div>
   <!-- /wp:group -->
   

Screenshots, screen recording, code snippet

Video demonstrating the issue:

templateLock.webm

---

The problem seems to be the deprecation migration to add the default layout, specifically the fact that the templateLock attribute is wrongly defined as string only:

gutenberg/packages/block-library/src/group/deprecated.js

Lines 52 to 54 in b7c693b

	templateLock: {
	type: 'string',
	},

while it can also be bool:

gutenberg/packages/block-library/src/group/block.json

Lines 15 to 18 in b7c693b

	"templateLock": {
	"type": [ "string", "boolean" ],
	"enum": [ "all", "insert", "contentOnly", false ]
	}

This then leads to the bool attribute being dropped since it is considered invalid but any other string value like all does indeed survive:

templateLock2.webm

---

This is further proven by the fact that adding this snippet does resolve the issue at hand:

wp.hooks.addFilter(
	'blocks.registerBlockType',
	'fix/template-lock',
	(settings, name) => {
		if(
			name==='core/group'
			&&
			settings.deprecated.length === 5
			&&
			settings.deprecated[0].attributes?.templateLock?.type === 'string'
		){
			settings.deprecated[0].attributes.templateLock.type = [ "string", "boolean" ];
			settings.deprecated[0].attributes.templateLock.enum =  [ "all", "insert", "contentOnly", false ];
		}
		return settings;
	}
);

Environment info

• WordPress 6.1.1, without Gutenberg as well as with Gutenberg 15.3.1 enabled
• Firefox
• Desktop with Ubuntu 22.04

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[kraftner]

Looking some more I think these (2918b80, #36264) are related in that the string/bool for the templateLock attribute mix was already causing confusion elsewhere but nobody did think of the deprecations as well.

I also wonder if the deprecation migration for the even older version of the group block with the double-div might also be affected.

[kraftner]

Yeah, same problem with the second migration. Updated code also fixes it for that:

wp.hooks.addFilter(
	'blocks.registerBlockType',
	'fix/template-lock',
	(settings, name) => {
		if(
			name==='core/group'
			&&
			settings.deprecated.length === 5
			&&
			settings.deprecated[0].attributes?.templateLock?.type === 'string'
		){
			settings.deprecated[0].attributes.templateLock.type = [ "string", "boolean" ];
			settings.deprecated[0].attributes.templateLock.enum =  [ "all", "insert", "contentOnly", false ];
		}
		if(
			name==='core/group'
			&&
			settings.deprecated.length === 5
			&&
			settings.deprecated[1].attributes?.templateLock?.type === 'string'
		){
			settings.deprecated[1].attributes.templateLock.type = [ "string", "boolean" ];
			settings.deprecated[1].attributes.templateLock.enum =  [ "all", "insert", "contentOnly", false ];
		}
		return settings;
	}
);

(Yeah, I know this could be written better but since it is just a proof of concept...)

[kraftner]

@Mamaduka Looking at it again today I think the fix in #49205 was incomplete since for the earlier migration the type is still wrong:

gutenberg/packages/block-library/src/group/deprecated.js

Lines 138 to 140 in b93fd88

	templateLock: {
	type: 'string',
	},

This is what I mentioned in the comment above.

[Mamaduka]

Hi, @kraftner

I saw the original comment, but I think changes weren't required for that deprecation.

The deprecation attributes are a "snapshot" of block attributes at a (deprecation) creation time. Looking at the PR introducing deprecation #29335, you can see that the "templateLock" type is still a string.

[kraftner]

I am aware how they work, but looking at #36264 (where the type got changed) I got the impression that might have been an error itself.

Or are you saying that the bug in #36264 means at that point it wasn't even possible to save false so the migration doesn't need to consider it? Or am I understanding this wrong?

Sorry if I am wrong, just double-checking. 😄

[Mamaduka]

Oh, you're right. When the deprecation was created, it copied the attribute of the wrong type, which later got fixed for the block but not for the deprecation.

I'll create a new PR later today or tomorrow.

Thanks, @kraftner!