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

Gutenberg is wrapping p tags inside script tags since 4.6 #12530

Open
CreativeDive opened this Issue Dec 2, 2018 · 30 comments

Comments

Projects
None yet
8 participants
@CreativeDive
Copy link

CreativeDive commented Dec 2, 2018

In some cases I include a little script within a block content. Since 4.6. the gutenberg editor is breaking my scripts by adding p-tags inside the scripts. Before 4.6 everything is working fine with using scripts in combination with a custom block.

[UPDATE: this is being tracked as a WP Core Bug at https://core.trac.wordpress.org/ticket/45495.]

@designsimply

This comment has been minimized.

Copy link
Contributor

designsimply commented Dec 2, 2018

If possible, please include a code snippet to help illustrate where p-tags are getting added inside of scripts in your custom block.

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 3, 2018

@designsimply ... I'm using Advanced Custom Fields Pro to add custom Gutenberg blocks. In my case, this block can add a custom image carousel which is using OWL Carousel. For this reason I add a little inline script.

It works fine with Gutenberg 4.5, like this:

bildschirmfoto 2018-12-03 um 10 16 24

After updating to Gutenberg 4.6, the javascript is broken, because there are p-tags inside the script, like this:

bildschirmfoto 2018-12-03 um 10 17 57

With downgrading to Gutenberg 4.5. It works fine again. Is there a workaround or is this an bug of Gutenberg 4.6?

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 3, 2018

@designsimply ... Also other blocks which are build with HTML are broken after installing Gutenberg 4.6 by adding p-tags into the HTML structure of the custom block.

p-tags

This is bad practice for a new editor, which can build custom blocks with HTML.

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 3, 2018

By adding remove_filter( 'the_content', 'wpautop' ); it's solving the issue. But this is not the solution :-(

@youknowriad

This comment has been minimized.

Copy link
Contributor

youknowriad commented Dec 3, 2018

cc @brandonpayton if you have an idea as I believe you recently worked on something in this area.

@CreativeDive how are you adding your inline script?

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 3, 2018

@youknowriad I'm using the ACF method (https://www.advancedcustomfields.com/resources/acf_register_block/) to include custom blocks.

There is a "acf_register_block" function to register a custom block like:

bildschirmfoto 2018-12-03 um 12 32 55

And the register block function is loading a template for each custom block. In the example of the inline script, which have custom modifications, the script can be customize by php variables and will called after the HTML:

bildschirmfoto 2018-12-03 um 12 33 23

@youknowriad

This comment has been minimized.

Copy link
Contributor

youknowriad commented Dec 3, 2018

Try avoiding empty lines in your script, I think that's a Core issue where wpautop adds this <P> tags on empty lines regardless of whether it's a script or not. a trac ticket might be better.

I think Gutenberg was disabling this behavior for the whole post before 4.6 but that was creating other issues because content that was not inside blocks was still supposed to be passed through wpautop (classic block for instance).

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 3, 2018

@youknowriad I think to delete empty spaces in a template file is not a solution for the future of the new gutenberg editor. The editor is more flexible and more customizable than before and a lot of developers will work in the future with custom blocks by adding HTML, Styles or Scripts.

Is there no other solution for developers, to add custom blocks without wrapped or empty p-tags?

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 3, 2018

@youknowriad Is there no way to use wpautop only for classic blocks? I think by using wpautop in the gutenberg editor will cause so many issues in the future.

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 3, 2018

@youknowriad in this example template file I have deleted all empty lines:

bildschirmfoto 2018-12-03 um 13 08 52

But the issue is still there:

bildschirmfoto 2018-12-03 um 13 10 15

A lot of annoying p-tags and br-tags are inside the HTML structure and they destroy the design.

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 3, 2018

@youknowriad In the past, shortcodes have been the best way to include HTML content in editor content. There was a way to fix this problem by adding a filter to remove empty annoying or incorrectly wrapped p-tags. In this case, however, I see no way to solve this problem.

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 3, 2018

@youknowriad I have figured out, with the default custom HTML block from wordpress, p-tag or br-tags will filtered out, if I add custom HTML. It's possible to add a simular filter for to do the same thing with blocks by other developers?

@jsor

This comment has been minimized.

Copy link

jsor commented Dec 3, 2018

We're also seeing this for all server-side rendered blocks. We have temporarily fixed that by only applying wpautop for old post without blocks.

remove_filter('the_content', 'wpautop');
add_filter('the_content', function ($content) {
    if (has_blocks()) {
        return $content;
    }

    return wpautop($content);
});

Is there no way to use wpautop only for classic blocks?

If i'm not missing something, that isn't needed as content written with core/freeform (classic block) already contains the required <p>'s and <br>s.

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 3, 2018

@jsor Yes this is definitively a workaround to solve this issue 👍

But I wonder if that wpautop is really necessary for the gutenberg editor?

@brandonpayton

This comment has been minimized.

Copy link
Member

brandonpayton commented Dec 3, 2018

@CreativeDive which version of ACF are you using?

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 3, 2018

@brandonpayton I'm using the latest ACF PRO 5.8.0 Beta3 with Gutenberg 6.1. But I don't think it's an issue by ACF, I think it's a main issue by adding server-side rendered blocks.

@brandonpayton

This comment has been minimized.

Copy link
Member

brandonpayton commented Dec 3, 2018

Thanks, @CreativeDive. I'm wondering whether there is interaction between ACF and Gutenberg.

We used to remove WordPress's wpautop filter and replace it with a Gutenberg version gutenberg_wpautop that ran earlier for the_content filter. It would only apply wpautop to content that didn't contain blocks.

Changing the priority at which wpautop was run caused an issue with auto-embeds. This was also an issue in early 5.0 betas. In the latest version of Gutenberg, we do the same thing that is now done in core which is:

  • Leave the wpautop filter for the_content at the default priority.
  • Temporarily disable the wpautop filter when filtering a post with block content.

💡 I'm wondering whether ACF is modifying filters or doing something that assumed the previous wpautop behavior and leading to this issue.

I think it's a main issue by adding server-side rendered blocks.

I've tested this with simple server-side rendered blocks today and haven't been able to reproduce the issue. Is there any more info you could provide to help?

@brandonpayton

This comment has been minimized.

Copy link
Member

brandonpayton commented Dec 3, 2018

@jsor how are your server-side blocks created? Are you using ACF, the Gutenberg blocks API, or something else?

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 3, 2018

@brandonpayton thanks for your reply. I have added a ticket for the ACF developers and wait for a response. Let's see what they say about this issue.

@jsor

This comment has been minimized.

Copy link

jsor commented Dec 4, 2018

@brandonpayton After a little more investigation, in my case, this is caused by a server-side rendered block which itself runs apply_filters('the_content', $content) (it renders a custom post list).

The nested run of the the_content filter chain then triggers the _restore_wpautop_hook too early.

Moving the wpauto filter handling after the block rendering fixes that for me.

--- a/public/app/mu-plugins/gutenberg/lib/blocks.php
+++ b/public/app/mu-plugins/gutenberg/lib/blocks.php
@@ -176,10 +176,6 @@ if ( ! function_exists( 'do_blocks' ) ) {
        function do_blocks( $content ) {
                // If there are blocks in this content, we shouldn't run wpautop() on it later.
                $priority = has_filter( 'the_content', 'wpautop' );
-               if ( false !== $priority && doing_filter( 'the_content' ) && has_blocks( $content ) ) {
-                       remove_filter( 'the_content', 'wpautop', $priority );
-                       add_filter( 'the_content', '_restore_wpautop_hook', $priority + 1 );
-               }
 
                $blocks = gutenberg_parse_blocks( $content );
                $output = '';
@@ -188,6 +184,11 @@ if ( ! function_exists( 'do_blocks' ) ) {
                        $output .= gutenberg_render_block( $block );
                }
 
+               if ( false !== $priority && doing_filter( 'the_content' ) && has_blocks( $content ) ) {
+                       remove_filter( 'the_content', 'wpautop', $priority );
+                       add_filter( 'the_content', '_restore_wpautop_hook', $priority + 1 );
+               }
+
                return $output;
        }
@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 7, 2018

@brandonpayton

Now I've got feedback from the ACF developers. It is not an issue by the ACF plugin. That's why I looked deeper into my code and figured out the reason for this problem. I have a block that creates a post list. This block was called together with the block from the above example and caused the error. The post list block is using the excerpt by get_the_excerpt();.

By removing get_the_excerpt(); from the post list block, everything is working well. The same problem occurs with the_excerpt();.

I'm not sure, but maybe the wp_trim_excerpt() function is used in my case? wp_trim_excerpt() is using apply_filters('the_content', $text); ---> And this is applying the filter too early?

My post list will create within a foreach loop like this:

global $post;
foreach( $query as $post ) {
    setup_postdata( $post );

    ...

}
wp_reset_postdata();

@brandonpayton Do you have an idea, why the complete post content is broken, after output get_the_excerpt(); to the post list block?

Maybe the solution of @jsor is solving that issue?

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 7, 2018

@brandonpayton I've tested the code changes from @jsor and it is solving this issue completely. What do you think about?

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 7, 2018

@brandonpayton for using wp_strip_all_tags( $post->post_content ) instead of get_the_excerpt(); is a workaround, but this can not solve the issue for get_the_excerpt(); inside the post content. Is this a BUG, how WordPress is trigger the get_the_excerpt(); function?

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 7, 2018

@brandonpayton If I using a shortcode that also using get_the_excerpt(); with a simular functionality like my post list block, it's working well. Only in combination with an editor block the HTML structure is interrupted.

@CreativeDive

This comment has been minimized.

Copy link
Author

CreativeDive commented Dec 10, 2018

@brandonpayton are there any reactions to this issue?

@jonschr

This comment has been minimized.

Copy link

jonschr commented Jan 18, 2019

I'm experiencing what I believe to be the same issue, also running Gutenberg with ACF blocks with ACF PRO 5.8.0 Beta3 and WordPress 5.0.3. I've noticed this happening when any of these happen:

  • the_content() is used to output the field content
  • 'the_content' filter is applied
  • the_excerpt is used
  • the_field() ACF core function is used (the get_field() ACF core function works fine, which I found odd but couldn't explain, as the documentation says that the_field simply does echo get_field, which works.)
  • inside a shortcode which is inside the ACF field, in the output of the shortcode itself, any of the above bullets will break things as well.

When any of the above conditions are met, other ACF fields that are in different Gutenberg blocks on the same page fail to output properly, likely due to the

tags being added (I'm noticing it where I'm injecting field values into CSS and seeing it break there.

Just wanted to provide another data point and to see if this is likely to be changed.

I've seen the solution posted, but I doubt you want every plugin author out there to start doing this:

remove_filter( 'the_content', 'wpautop' );
add_filter( 'the_content', function ($content) {
    if (has_blocks()) {
        return $content;
    }

    return wpautop($content);
});
@hexagongirl

This comment has been minimized.

Copy link

hexagongirl commented Jan 30, 2019

I am also seeing the same issue, when creating a post grid block using ACF blocks. I'm using ACF PRO 5.8.0 Beta3 and WordPress 5.0.3. the_excerpt is the culprit in my case.

@jonschr

This comment has been minimized.

Copy link

jonschr commented Jan 30, 2019

@hexagongirl Please do update the thread if you find a better solution. Removing the wpautop filter ... leaves a lot to be desired.

@jSanchoDev

This comment has been minimized.

Copy link

jSanchoDev commented Jan 31, 2019

We have the same issue, but we didn't want to remove wpautop filter globally, so instead we call alternative function only for blocks render:

static function render_post_content($is_block_editor = false) {
    if (!$is_block_editor) {
	the_content();
	return;
    }
        
    $blocks_content_filter_current_priority = has_filter( 'the_content', '_restore_wpautop_hook' );
        
    if ($blocks_content_filter_current_priority) {
        remove_filter('the_content', '_restore_wpautop_hook', $blocks_content_filter_current_priority);
    }
        
    the_content();
        
    if ($blocks_content_filter_current_priority) {
        add_filter( 'the_content', '_restore_wpautop_hook', $blocks_content_filter_current_priority );
    }
}

I really hoped they'd fix this quickly, but the related Trac issue has been in Awaiting Review for 8 weeks now.

@brandonpayton

This comment has been minimized.

Copy link
Member

brandonpayton commented Feb 7, 2019

Hi @CreativeDive, I'm sorry for responding so late. Since this appears to be a core bug with a core ticket, let's see if a fix there resolves this specific issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment