Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ticket 56780 #3851

Open
wants to merge 10 commits into
base: trunk
Choose a base branch
from
Open

Ticket 56780 #3851

wants to merge 10 commits into from

Conversation

petitphp
Copy link

Update block_template_part to expand shortcodes in template parts

Add set of typical actions run on the_content to the block_template_part function. This replicate the behavior of the template part block's render method.

Trac ticket: https://core.trac.wordpress.org/ticket/56780


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@ironprogrammer
Copy link

Thanks for this, @petitphp! Because this PR goes beyond handling do_shortcode() (original issue), it would be great if the unit tests accounted for the other changes. If you have time to expand those further, perhaps we can nudge this back into the current release cycle 🤞🏻

There are still a couple of early scrubs left, so I don't think the door is closed quite yet if this keeps up momentum.

@petitphp
Copy link
Author

@ironprogrammer Thanks for taking the time to do another round of testing! As suggested, I've expanded the test cases for the change based on the ones you mentioned on your report.

Add set of typical actions run on `the_content` to the `block_template_part` function.
This replicate the behavior of the template part block's render method.
@costdev costdev requested a review from spacedmonkey October 7, 2023 04:45
$content = wptexturize( $content );
$content = convert_smilies( $content );
$content = shortcode_unautop( $content );
$content = wp_filter_content_tags( $content );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@felixarntz Do you have any idea how this will effect fetch priority or other image

@youknowriad
Copy link
Contributor

I think this PR introduces some subtle security issues. For example an anonymous user can use a shortcode in a comment to render private data in the frontend. (comments being rendered by a block comments block)

@spacedmonkey
Copy link
Member

I think this PR introduces some subtle security issues. For example an anonymous user can use a shortcode in a comment to render private data in the frontend. (comments being rendered by a block comments block)

I created a test shortcode and adding to comment text. I am not seeing the shortcode render there.
Screenshot 2023-10-09 at 12 02 06

@youknowriad
Copy link
Contributor

The template part block does something like this:

	// Run through the actions that are typically taken on the_content.
	$content                       = shortcode_unautop( $content );
	$content                       = do_shortcode( $content );
	$seen_ids[ $template_part_id ] = true;
	$content                       = do_blocks( $content );
	unset( $seen_ids[ $template_part_id ] );
	$content = wptexturize( $content );
	$content = convert_smilies( $content );
	$content = wp_filter_content_tags( $content, "template_part_{$area}" );

	// Handle embeds for block template parts.
	global $wp_embed;
	$content = $wp_embed->autoembed( $content );

I wonder if there's a way to reuse that code in both places, one could use the other or rely on a common function. (It might require the changes to be made in the Gutenberg repository)

@youknowriad
Copy link
Contributor

I created a test shortcode and adding to comment text. I am not seeing the shortcode render there.

Did you use a theme that uses the function above to render a "template part" that contains the comments block?

@spacedmonkey
Copy link
Member

+1 render_block_core_template_part and block_template_part should use the same logic. It is confusing why they do not.

@Mai-Saad
Copy link

@petitphp Thanks for the PR. Can you please check the conflicts 🙏

Copy link

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 props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @[email protected].

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

Core Committers: Use this line as a base for the props when committing in SVN:

Props petitphp, mukesh27, spacedmonkey, ironprogrammer, youknowriad, mai21.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants