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

When rendering template parts don't call wpautop() to avoid unwanted empty paragraphs (#26731) #26809

Closed
wants to merge 1 commit into from

Conversation

bobbingwide
Copy link
Contributor

Description

When rendering template parts the call to wpautop() could produce unwanted </p> tags.

I've removed the call to wpautop() from the routines run against the $content in (gutenberg_)render_block_core_template_part() to prevent this happening.

How has this been tested?

I created a simple test that demonstrated the problem, then fixed the problem, and retested.

Screenshots

For screenshots see the issue #26731

Types of changes

Fixes #26731

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

I haven't run any PHPUnit tests.

@youknowriad
Copy link
Contributor

This one can be a bit impactful. I'd love for us to "deprecated" autop in FSE templates and template parts but I'm not sure I understand the impact of something like that fully. I'd love thoughts from more folks here.

@bobbingwide
Copy link
Contributor Author

Here's the latest example of the problem.
Rendering Tag clouds in a template part produces a load of unwanted <br /> tags.
See bobbingwide/sb#5 (comment)

@bobbingwide
Copy link
Contributor Author

And another.... #26731 (comment)

@azaozz
Copy link
Contributor

azaozz commented Apr 8, 2021

I'd love for us to "deprecated" autop in FSE templates and template parts

A big +1!

Not only in FSE, also everywhere in the block editor even in the classic block. At this point autop should only be a "back-compat for older posts" there, really. It doesn't have any other role :)

A bit of history: autop was introduced very early in WP when the editor was just a plain textarea. There it made the Edit Post UX much more appealing by converting two /n to a <p>. Later on when TinyMCE was introduced (WP 2.0 I believe) autop continued to be used for the "Text" tab in the editor as quite a few users preferred it. Over the next 10-12 years the Text tab was used less and less but still remained the only way to fine-tune some HTML or add something specific, so autop continued to be needed. With the introduction of the block editor in WP 5.0 there's no more need of autop for the UX, it's needed mostly for back-compat.

@youknowriad
Copy link
Contributor

I'm not sure I understand all the implications of this PR but can we rebase it and try to land it?

@youknowriad
Copy link
Contributor

Closing in favor of #30552 which does the same but is more ready to land. Thanks @bobbingwide

@youknowriad youknowriad closed this Apr 9, 2021
@bobbingwide
Copy link
Contributor Author

Closing in favor of #30552 which does the same but is more ready to land. Thanks @bobbingwide

Oh good!

Now perhaps we can see if $content = shortcode_unautop($content); is still necessary.

@azaozz
Copy link
Contributor

azaozz commented Apr 12, 2021

we can see if $content = shortcode_unautop($content); is still necessary

Only needed for back-compat with $post->post_content. Should not be used for anything else.

Actually... Looking at https://github.com/WordPress/gutenberg/blob/trunk/packages/block-library/src/template-part/index.php what's the purpose of $content = do_shortcode( $content ); for "everything"? That seems to be adding slow, old, buggy, security challenged, and pretty pointless functionality to the brand-new template parts? :)

Investigating further...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Template Part Affects the Template Parts Block
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gutenberg_render_block_core_template_part() should not call wpautop()
4 participants