Make WordPress Core

Opened 8 months ago

Closed 5 months ago

Last modified 5 months ago

#59839 closed enhancement (fixed)

Twenty Twenty-Four: register_block_pattern_category()'s slug/$category_name should be prefixed

Reported by: acosmin's profile acosmin Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 6.5 Priority: normal
Severity: normal Version:
Component: Bundled Theme Keywords: good-first-bug has-patch commit
Focuses: Cc:

Description

https://themes.trac.wordpress.org/browser/twentytwentyfour/1.0/functions.php#L197

Plugins can change the proprieties (label/description) even by mistake.

Attachments (1)

59839.patch (5.5 KB) - added by shailu25 6 months ago.
Patch Added

Download all attachments as: .zip

Change History (16)

This ticket was mentioned in Slack in #core by jorbin. View the logs.


8 months ago

#2 @poena
6 months ago

  • Keywords needs-patch good-first-bug added
  • Milestone changed from Awaiting Review to 6.5

To clarify, register_block_attern_category() accepts the parameter $category_name, which in Twenty Twenty-Four is 'page':

register_block_pattern_category(
			'page',
			array(
				'label'       => _x( 'Pages', 'Block pattern category', 'twentytwentyfour' ),
				'description' => __( 'A collection of full page layouts.', 'twentytwentyfour' ),
	)
);

If a plugin also registers the category name 'page' in an init hook that runs after the theme's, it can override the label and description unintentionally.

With a prefix, extenders can still override it, but it would be intentional.
Adding a prefix now will stop any existing overrides from working, intentional or not: either way, it is a minor problem since it would only affect a label and description.

The proposed fix is to prefix parameter similarly to this:

register_block_pattern_category(
			'twentytwentyfour_page',
			array(
				'label'       => _x( 'Pages', 'Block pattern category', 'twentytwentyfour' ),
				'description' => __( 'A collection of full page layouts.', 'twentytwentyfour' ),
	)
);

The theme patterns with

 * Categories: page

Would need to be updated to use the new prefixed parameter:

 * Categories: twentytwentyfour_page

@shailu25
6 months ago

Patch Added

#3 @shailu25
6 months ago

  • Keywords has-patch added; needs-patch removed

I have added patch as per suggested in above comment

#4 @poena
6 months ago

  • Keywords needs-testing added

#5 @harshgajipara
6 months ago

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://core.trac.wordpress.org/attachment/ticket/59839/59839.patch

Environment

OS: Windows
PHP: 8.1.9
WordPress: 6.4.2
Browser: Chrome
Theme: Twenty Twenty-Four
Plugins: None activated

Actual Results:

After patch: twentytwentyfour_page pattern category is prefix now.(not changeable / override via plugin / custom code )
Backend: https://prnt.sc/4IXCfFOyOOwu
Frontend:

  1. https://prnt.sc/MZKtbvcbo79B
  2. https://prnt.sc/xxq7ykUu3m2q

#6 @poena
5 months ago

  • Keywords commit added; 2nd-opinion needs-testing removed

I'm trying to think of if there are any side effects, but no, I can't think of any. This works well in my testing, thank you.

-When creating a new custom pattern, the Pages category is available, working correctly.

Last edited 5 months ago by poena (previous) (diff)

#7 @swissspidy
5 months ago

The categories in Twenty Twenty-Two are also not prefixed (see twentytwentytwo_register_block_patterns()). Should the same change be made there?

All other default themes just use their theme name as the category name.

#8 @SergeyBiryukov
5 months ago

In 57555:

Twenty Twenty-Four: Prefix the block pattern category name.

If a plugin also registers the category name page in an init hook that runs after the theme's, it can override the label and description unintentionally.

With a prefix, extenders can still override it, but it would be intentional.

Props shailu25, poena, acosmin, harshgajipara, swissspidy.
See #59839.

#9 @SergeyBiryukov
5 months ago

Thanks for the patch!

Keeping the ticket open for now to also address comment:7.

#10 @poena
5 months ago

Good find.

I have now had a look at the categories in Twenty Twenty-two:
I found that featured, footer, header, and query are also registered by WordPress:
https://core.trac.wordpress.org/browser/trunk/src/wp-includes/block-patterns.php#L61
If the slug is updated, these categories show as duplicates on supported WordPress versions.
We also can't remove them because then the categories would be missing on older WordPress versions.

But we can update the final slug, pages.

@shailu25 commented on PR #6066:


5 months ago
#12

Hi @carolinan

Thanks for Patch.

You have added font file(OpenSans-Regular.ttf). it should be remove from the patch

There 3 more files which have Pages category it also should be changed

  1. /inc/patterns/page-about-large-image-and-buttons.php
  2. /inc/patterns/page-about-links-dark.php
  3. /inc/patterns/page-about-links.php

@poena commented on PR #6066:


5 months ago
#13

Thank you for the review!

#14 @SergeyBiryukov
5 months ago

  • Owner set to SergeyBiryukov
  • Resolution set to fixed
  • Status changed from new to closed

In 57569:

Twenty Twenty-Two: Prefix the pages block pattern category name.

If a plugin also registers the category name pages in an init hook that runs after the theme's, it can override the label and description unintentionally.

With a prefix, extenders can still override it, but it would be intentional.

Props poena, swissspidy, shailu25.
Fixes #59839.

@SergeyBiryukov commented on PR #6066:


5 months ago
#15

Thanks for the PR! Merged in r57569.

Note: See TracTickets for help on using tickets.