Make WordPress Core

Opened 2 years ago

Last modified 9 days ago

#56780 assigned defect (bug)

shortcode block in block-based template part in a classic theme does not get expanded

Reported by: pbiron's profile pbiron Owned by: costdev's profile costdev
Milestone: Future Release Priority: high
Severity: normal Version: 6.1
Component: Editor Keywords: has-testing-info has-screenshots has-patch has-unit-tests changes-requested needs-testing early
Focuses: Cc:

Description

While testing the new feature in 6.1 (w/ 6.1-beta3) for a classic theme to include block-based template parts, I discovered that shortcode blocks in such template parts are not expanded (their render callbacks are not called).

The details are in my comment on the Dev Note announcing this new feature.

Note: creating the ticket here in Trac, rather than the GB GitHub, because that's where @mamaduka asked me to open in

Attachments (6)

test-56780.zip (2.1 KB) - added by pbiron 2 years ago.
minimal classic theme that uses block-based template part with a shortcode block
test-56780_updated.zip (2.8 KB) - added by mai21 15 months ago.
updated theme
after-patch-3851.png (8.5 KB) - added by mai21 15 months ago.
after patch#3851
before-patch-3851.png (7.8 KB) - added by mai21 15 months ago.
before path#3851
ticket_56780_before_patch_3851.png (327.9 KB) - added by petitphp 13 months ago.
Screenshot of a single post with block template part before patch 3851
ticket_56780_after_patch_3851.png (302.5 KB) - added by petitphp 13 months ago.
Screenshot of a single post with block template part after patch 3851

Download all attachments as: .zip

Change History (84)

#1 @pbiron
2 years ago

  • Summary changed from shortcode block in block-based template part in a class theme does not be expanded to shortcode block in block-based template part in a class theme does not get expanded

#3 @desrosj
2 years ago

  • Milestone changed from Awaiting Review to 6.1

Moving to 6.1 milestone for investigation.

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


2 years ago

#5 @chaion07
2 years ago

  • Keywords needs-testing added

@pbiron thank you for reporting this. We reviewed this ticket during a recent bug-scrub session and based on the feedback we are adding the keyword needs-testing.

Cheers!

Props to @costdev & @mukesh27

@pbiron
2 years ago

minimal classic theme that uses block-based template part with a shortcode block

#6 @pbiron
2 years ago

test-56780.zip contains a minimal classic theme that can be used to replicate the problem.

  1. Download and activate the theme on a site running WP 6.1-beta1, 6.1-beta2, 6.1-beta3, or 6.1-RC1.
  2. Then view any post/page on the front-end.
  3. Expected results: the string 'hellow world' should display above any post/page content.
  4. Actual results: the stirng '[hello_world]' displays above any post/page content.

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


2 years ago

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


2 years ago

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


2 years ago

#10 @costdev
2 years ago

  • Summary changed from shortcode block in block-based template part in a class theme does not get expanded to shortcode block in block-based template part in a classic theme does not get expanded

#11 @pbiron
2 years ago

  • Keywords early added
  • Milestone changed from 6.1 to 6.2

#12 @audrasjb
2 years ago

As per today's bug scrub, we're going to move it to milestone 6.2 with high priority and early workflow keyword.

This is a very annoying bug, but if we try to fix it during RC cycle, we're probably going through a rabbit hole (see #54759, #55996).

#13 @audrasjb
2 years ago

  • Priority changed from normal to high

#14 @ironprogrammer
22 months ago

  • Keywords has-testing-info has-screenshots added; needs-testing removed

Reproduction Report

I was able to reproduce this issue.

Environment

  • Hardware: MacBook Pro Apple M1 Pro
  • OS: macOS 12.6.1
  • Browser: Safari 16.1
  • Server: nginx/1.23.2
  • PHP: 7.4.33
  • WordPress: 6.2-alpha-54642-src
  • Theme: test-56780 v0.1.0 (the provided attachment:test-56780.zip)
  • Active Plugins:
    • gutenberg v14.8.0-rc.1 (tested active/inactive)

Actual Results

  • ✅ Shortcodes are not processed when included in a block_template_part.

Additional Information

  • Shortcode was not processed in the template part, whether inside a <!-- wp:shortcode --> or standalone.
  • Added paragraph and shortcode blocks to post to demonstrate the shortcode was otherwise functional.
  • Applied the following adjustments to the provided test theme for added clarity:
    • in masthead.html, added <h3>masthead:</h3> before the shortcode block
    • correct typo in shortcode to [hello_world]
    • in functions.php, updated return value of shortcode to hello world!

Supplemental Artifacts

https://cldup.com/_M3upWbG1n.png

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


21 months ago

#16 @audrasjb
21 months ago

  • Milestone changed from 6.2 to Future Release

As per today's bug scrub and since this ticket doesn't have any patch yet, let's move it to Future Release.

This ticket was mentioned in PR #3851 on WordPress/wordpress-develop by @petitphp.


21 months ago
#17

  • Keywords has-patch has-unit-tests added; needs-patch removed

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

#18 @petitphp
21 months ago

Added a patch that replicate the behavior of the template part block's render method in block_template_part. Tested manually and with the newly created unit test.

@audrasjb I don't know if this ticket can be added back to the 6.2 milestone or if it's too late given it was tagged early.

#19 @ironprogrammer
21 months ago

Thanks for the patch, @petitphp! I've expanded the test cases to include the other processing added in PR 3851.

Test Report

Patch tested: https://github.com/WordPress/wordpress-develop/pull/3851

Environment

  • Hardware: MacBook Pro Apple M1 Pro
  • OS: macOS 12.6.2
  • Browser: Safari 16.2
  • Server: nginx/1.23.3
  • PHP: 7.4.33
  • WordPress: 6.2-alpha-54642-src
  • Theme: test-56780 v0.1.0 (same as comment:14 plus changes noted below)

Actual Results

block_template_part content is now processed by:

  • ✅ do_shortcode (original reported issue)
  • ✅ wptexturize (e.g. processed smart quotes)
  • ✅ convert_smilies
  • ✅ shortcode_unautop
  • wp_filter_content_tags (added decoding="async" loading="lazy" attributes to <img> tag)
  • ✅ $wp_embed->autoembed

Additional Notes

Supplemental Artifacts

Before Patch After Patch
https://cldup.com/OJ3-r45Gk4.png https://cldup.com/rSvlzt4B9V.png

@ironprogrammer commented on PR #3851:


21 months ago
#20

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 commented on PR #3851:


21 months ago
#21

@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.

#22 @davidbaumwald
19 months ago

  • Milestone changed from Future Release to 6.3

#23 @hellofromTonya
19 months ago

  • Keywords early removed

I don't think this ticket needs to be constrained by early.

Why?

Per the handbook:

This keyword should only be applied by committers. The keyword signals that the ticket is a priority, and should be handled early in the next release cycle.

The high priority level flags that it needs to be addressed high in the bugfix queue.

But early keyword has a time restriction that the bugfix must be completed early alpha release cycle, else it will be punted out of the release.

For this bugfix, I'm seeing a high risk to users or the project to drive needing it committed early in alpha. Is there a high risk?

IMO this could have been fixed up until RC1. But since it was flagged as an early candidate, once the early phase of the alpha cycle was past, it has to be punted.

To avoid punting it in 6.3, I suggest removing early but leaving the high priority. With this approach, the high priority bugfix can happen at any time between now and 6.3 RC1.

@audrasjb do you agree?

#24 @petitphp
17 months ago

In WP 6.2.1 a change was made to stop processing shortcodes in block templates.

This change is being discussed after some issues were reported for some site after the update to 6.2.1.

This ticket might become invalid if shortcode processing is removed, or the patch for this ticket will need to be updated to match the definitive solution.

Last edited 17 months ago by petitphp (previous) (diff)

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


17 months ago

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


16 months ago

#27 @oglekler
16 months ago

  • Keywords needs-testing added

I tested with 6.3-alpha-56078-src and Twenty Twenty-Three theme, placing inline shortcode into Header, Footer and Post Meta templates part — it is working everywhere.

So, it looks like the initial issue needs to be tested again to see if it was fully fixed during 6.2.2.

#28 @poena
16 months ago

I can reproduce the issue with 6.3-beta3-56130-src.

I have used the provided classic/hybrid test theme with some additions like the default gallery short code. I found that short codes are not rendered in the header part.

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


15 months ago

#30 @mai21
15 months ago

Test Report

Reproducible with the following steps based on https://core.trac.wordpress.org/ticket/56780#comment:14

1- Install and activate the following theme
https://core.trac.wordpress.org/attachment/ticket/56780/test-56780_updated.zip

2- Create a post and add short code

<!-- wp:shortcode -->
	[hello_world]
	<!-- /wp:shortcode -->

3- View the frontend of the post

Environment

  • OS: Ubuntu 23.04
  • Web Server: Nginx
  • PHP: 8.1.9
  • WordPress: 6.3-beta4-56228)
  • Browser: Chrome 114.0.5735.198
  • Theme: [test-56780_updated.zip]
  • Active Plugins:
    • Core rollback 1.3.5
    • WordPress Beta Tester 3.5.2

Actual Results

before path#3851

Additional Notes

  • Expected o/p:

masthead: hello_world!

Short code in post edit

hello_world!

Supplemental Artifacts

Last edited 15 months ago by mai21 (previous) (diff)

@mai21
15 months ago

updated theme

@mai21
15 months ago

after patch#3851

@mai21
15 months ago

before path#3851

This ticket was mentioned in Slack in #core-test by mai21. View the logs.


15 months ago

#32 @oglekler
15 months ago

  • Keywords close added; needs-testing removed

According to @mai21, this ticket was fixed on the related PR.

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


15 months ago

#34 @audrasjb
15 months ago

  • Keywords early added; close removed
  • Milestone changed from 6.3 to 6.4

As per today's bug scrub, and since WP 6.3 release candidate 1 is planned tomorrow, let's move this ticket to milestone 6.4 wirth early keyword, so it doesn't wait until the end of the beta cycle to be committed.

#35 @hellofromTonya
14 months ago

  • Keywords early removed

I've removed the early keyword. Why? See my comments above.

The handbook explains the keyword as:

This keyword should only be applied by committers. The keyword signals that the ticket is a priority, and should be handled early in the next release cycle.

early means the commits need to happen during the early part of the Alpha cycle.

Though it would be great to prepare the work for commit as early as possible, commits for this ticket should not be constrained to only happen during the early Alpha phase. Rather, defect commits can happen up to 6.4 RC 1.

To aid in 6.4 tracking and scrubs, I've removed the early keyword.

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


14 months ago

#37 follow-up: @deargrumpycats
14 months ago

Is there a chance in the next update to include Bard AI? I recently run a

Entertainment website and it would be
perfect if we can have AI. Thanks.

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


13 months ago

#39 @petitphp
13 months ago

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/3851

Environment

  • OS: macOS 13.4.1
  • Web Server: Nginx
  • PHP: 7.4.33
  • WordPress: 6.4-alpha-56267-src
  • Browser: Firefox 118.b2
  • Theme: Theme: test-56780 v0.1.0 (same as comment:14) with custom masthead.html content from ironprogrammer's gist
  • Active Plugins: ø

Actual Results

  • do_shortcode (original reported issue)
  • wptexturize (e.g. processed smart quotes)
  • convert_smilies
  • shortcode_unautop
  • wp_filter_content_tags (added decoding="async" loading="lazy" attributes to <img> tag)
  • $wp_embed->autoembed

Additional Notes

  • I was able to reproduce the original issue in trunk.
  • Prior to doing the testing, the patch has been refresh with latest change up until trunk@3f9bc4e.

Supplemental Artifacts

See attachments bellow for screenshots before and after applying the patch.

@petitphp
13 months ago

Screenshot of a single post with block template part before patch 3851

@petitphp
13 months ago

Screenshot of a single post with block template part after patch 3851

#40 @iammehedi1
13 months ago

Test Report

This report validates that the indicated patch fixed the issue.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/3851

Environment

Prior to Applying the Patch

  • Followed the provided guidelines of @mai21
  • ❌ The shortcode remained unprocessed within the template section, whether it was placed within a <!-- wp:shortcode --> block or used independently.

After Applying The Patch

  • ✅ The issue was fixed.

Supplemental Artifacts

Before patch:

https://i.ibb.co/Qn2zFY3/image.png

After Patch:

https://i.ibb.co/FKHKF1j/image.png

#41 @zunaid321
13 months ago

Test Report

This report validates that the indicated patch addresses the issue.

Patch tested: https://github.com/WordPress/wordpress-develop/pull/3851

Environment

  • OS: Windows 11 (22H2)
  • Web Server: nginx/1.25.1
  • PHP: 7.4.33
  • WordPress: 6.4-alpha-56267-src
  • Browser: Chrome Version 113.0.5672.126 (Official Build) (64-bit)
  • Theme: [test-56780_updated.zip]
  • Plugins: None activated

Before Applying The Patch

  • Followed the instructions of @mai21
  • ❌ shortcode was not applied.

After Applying The Patch

  • Followed the same instructions.
  • ✅ shortcode was applied properly.

Supplemental Artifacts

Before: https://i.imgur.com/GsMts5M.png
After: https://i.imgur.com/T2KG8sU.png

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


13 months ago

#43 @marybaum
13 months ago

@costdev has asked @SergeyBiryukov to run the patch against the test suite. If nothing breaks, we're ready to commit. @Clorith has also volunteered to help.

#44 @costdev
13 months ago

  • Owner set to costdev
  • Status changed from new to assigned

#45 @nicolefurlan
13 months ago

@costdev was testing for this patch successful? Just checking to make sure that this will be ready for inclusion in 6.4, as RC1 is in ~2 weeks. Thanks :)

#46 @costdev
13 months ago

Hi @nicolefurlan, this still needs to be checked against the security test suite. It's too close to 6.4 Beta 2 to get this done now, so I'll get this done before 6.4 Beta 3.

#47 @costdev
12 months ago

Update:

  • The PR passes against the security tests.
  • I've made some updates to the PR to document the $wp_embed global, improve test method docblocks, names and use get_echo() for brevity.
  • I've pinged @spacedmonkey to see if some performance checks are possible to ensure that this PR doesn't cause too much of impact.

This one should still be landable in 6.4, provided that the performance checks don't show a drastic impact.

#48 @spacedmonkey
12 months ago

  • Focuses performance added

As this may have effects on performance, I have added the performance focus so the performance team can support with benchmarks and profiles.

This ticket was mentioned in Slack in #core-performance by spacedmonkey. View the logs.


12 months ago

@youknowriad commented on PR #3851:


12 months ago
#50

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)

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


12 months ago

@spacedmonkey commented on PR #3851:


12 months ago
#52

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.
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/237508/de8cff77-fe51-4437-8077-ef228d7f9f33

@youknowriad commented on PR #3851:


12 months ago
#53

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 commented on PR #3851:


12 months ago
#54

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 commented on PR #3851:


12 months ago
#55

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

#56 @spacedmonkey
12 months ago

Is there an example theme that uses this function to help test?

#57 follow-up: @costdev
12 months ago

  • Keywords changes-requested needs-testing added
  • Milestone changed from 6.4 to 6.5

Update:

  • There are security concerns about the PR. This needs some additional testing.
  • To help with this testing, there is a request for a test theme that uses block_template_part() to render a template part containing a comments block.
    • @pbiron can you help with this bit?
  • There is a request for feedback from @flixos90 regarding wp_filter_content_tags().
  • There is a suggestion to abstract the logic for shared usage between render_block_core_template_part() and block_template_part() which may require changes in the Gutenberg repository.
    • @youknowriad Are you happy to follow up on your suggestion with a new issue on the GitHub repository? If so, can you drop a link to the issue in here as a reference when it's created?

This ticket needs more work and 6.4 Beta is scheduled for release tomorrow. To give us time to explore and resolve the above, I'm milestoning this ticket for 6.5.

#58 in reply to: ↑ 57 @pbiron
12 months ago

Replying to costdev:

  • To help with this testing, there is a request for a test theme that uses block_template_part() to render a template part containing a comments block.
    • @pbiron can you help with this bit?

A template part containing a comments block? No, I don't have one. But the test-56780.zip test theme attached to this ticket should be modifiable to have a comments block.

But since I've never built a site (in 13 years of WP development) that allows commenting I'm don't think I'm the best person to create a "realistic" test theme with that feature ;-)

#59 @youknowriad
12 months ago

Good summary @costdev there are two small issues that force us to adopt a different strategy:

  • The block_template_part function is now maintained in core and can't be modified in Gutenberg.
  • The template part block can't use any code that only lives in the Gutenberg plugin (because it's auto imported to core later)
  • The Gutenberg plugin supports two WP versions.

So the way we should go about this is the following:

  • Update block_template_part on Core to duplicate what the template part block does (while ensuring the function or the extracted code can be reused in the template part block).
  • We'd have to wait that the function has been released since two WP releases before making the change in the template part block in the Gutenberg plugin to reuse the function in the template part block. The other approach would be to use a Gutenberg specific function as a fallback in the meantime something like:
<?php
if ( function_exists( 'block_template_part' ) ) {
   gutenberg_block_template_part();
} else {
  block_template_part();
}

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


11 months ago

#61 @joemcgill
11 months ago

  • Focuses performance removed

I'm removing the performance focus, since this ticket is not primarily about addressing a performance concern. Even so, @costdev feel free to reach out to folks on the team if we can support in any way.

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


10 months ago

#63 in reply to: ↑ 37 @tastynifty
8 months ago

  • Type changed from defect (bug) to enhancement

Any chance in the next update to include AI module such as bard or gemini? I'm new to wp, I build a Blog and I guessed it would be awesome if we can have AI inside wp/theme. It will make the job easier. Thanks.

#64 @hellofromTonya
8 months ago

  • Type changed from enhancement to defect (bug)

Restoring this ticket to be a bug report.

@tastynifty this ticket is tracking a specific bug report for shortcode blocks not being expanded (their render callbacks are not called) in a classic theme when using block-based template part. Discussions for adding AI module or capabilities should happen in a different Trac ticket. If one is not currently open, feel free to open a new feature request Trac ticket.

#65 @poena
8 months ago

#60444 was marked as a duplicate.

#66 @swissspidy
8 months ago

  • Milestone changed from 6.5 to 6.6

#67 @hmbashar
4 months ago

Test Report

Patch tested: https://github.com/WordPress/wordpress-develop/pull/3851

Environment

  • WordPress: 6.6-alpha-57778-src
  • PHP: 8.3.7
  • Server: nginx/1.25.4
  • Database: mysqli (Server: 8.3.0 / Client: mysqlnd 8.3.7)
  • Browser: Chrome 125.0.0.0
  • OS: macOS
  • Theme: Test 56780 0.1.0
  • MU Plugins: None activated
  • Plugins:
    • Test Reports 1.1.0

Actual Results

Followed @mai21's instructions before and after applying the patch, but I see no changes.

https://i.ibb.co/yp1c1Py/Before-Patch-Apply.png
https://i.ibb.co/v1dsNgf/Screenshot-at-Jun-06-1-40-34-AM.png

@mai21 commented on PR #3851:


4 months ago
#68

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

This ticket was mentioned in Slack in #core-test by mai21. View the logs.


4 months ago

#70 @nazmul111
4 months ago

I could not find any proper scenario of what about need to be tested.

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


4 months ago

#72 @desrosj
4 months ago

  • Keywords early added
  • Milestone changed from 6.6 to 6.7

Unfortunately this one did not get the attention it needs during the 6.6 cycle, so I'm going to punt.

I'm adding the early tag as I think this would benefit from a greater period of testing in trunk to confirm there are no unintended side effects somewhere.

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


6 weeks ago

#74 @chaion07
6 weeks ago

We reviewed this ticket during a recent bug-scrub session. @costdev do you still want to continue owning this Ticket? If so are you able to share a status update? Thanks.

Are the existing Test Results still valid? If not, can we have contributors conduct updated tests? We believe this Ticket needs a bit more attention. Cheers!

Props to @mukesh27 for the discussion

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


3 weeks ago

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


3 weeks ago

#77 @joemcgill
3 weeks ago

  • Milestone changed from 6.7 to Future Release

During Dev Chat today, we reviewed this ticket and concluded that this should be punted. Given that this has rolled for a few releases without much traction, I'm setting it as Future Release rather than allowing it to continue rolling to future release milestones without momentum. If it picks up traction, it can be moved back to a milestone.

#78 @ecairol
9 days ago

For folks out there still having this issue, here's a workaround while the patch makes it to core.

At the top of your template, extract the block HTML to a variable and apply do_shortcode():

<?php
ob_start();
block_template_part( 'header' );
$block_header = do_shortcode( ob_get_clean() );

Make sure to include wp_head() after these lines, so that the proper CSS inline styles are embedded on the screen:

<?php
<head>
  <?php wp_head(); ?>

And then echo $block_header to actually render the block where you want it.

Note: See TracTickets for help on using tickets.