WordPress.org

Make WordPress Core

Opened 23 months ago

Closed 17 months ago

Last modified 3 months ago

#45312 closed defect (bug) (invalid)

parse_blocks return non-existing empty blocks

Reported by: Chouby Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.0
Component: Editor Keywords:
Focuses: Cc:

Description

Create a new post with 2 paragraphs. I created them in visual mode, but here is the content of the code mode:

<!-- wp:paragraph -->
<p>First paragraph</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>Second paragraph</p>
<!-- /wp:paragraph -->

When I apply parse_blocks() on this content, the function returns 3 blocks. It inserts an empty non existing block between the 2 existing paragraph blocks. Here the output:

Array
(
    [0] => Array
        (
            [blockName] => core/paragraph
            [attrs] => stdClass Object
                (
                )

            [innerBlocks] => Array
                (
                )

            [innerHTML] => 
<p>First paragraph</p>

        )

    [1] => Array
        (
            [blockName] => 
            [attrs] => Array
                (
                )

            [innerBlocks] => Array
                (
                )

            [innerHTML] => 


        )

    [2] => Array
        (
            [blockName] => core/paragraph
            [attrs] => stdClass Object
                (
                )

            [innerBlocks] => Array
                (
                )

            [innerHTML] => 
<p>Second paragraph</p>

        )

)

This is the smallest test case I could create. But if you have n blocks, the function returns something like 2n-1 blocks.

Change History (10)

#1 @birgire
23 months ago

@Chouby I think this could be intentional, keeping the "freeform blocks" in between blocks.

Now there's a core namespace, so one could also test:

<!-- wp:core/paragraph -->
<p>First paragraph</p>
<!-- /wp:core/paragraph -->

<!-- wp:core/paragraph -->
<p>Second paragraph</p>
<!-- /wp:core/paragraph -->

yielding the same result, with an extra freeform block from new lines.

It seems we could remove these new-lines-only freeform blocks, by checking if the trimmed sub-string of the document is non-empty, in the WP_Block_Parser_Block::add_block_from_stack(), before adding the freeform block to the output.

But then the WP_Test_Block_Render::test_do_blocks_removes_comments() will fail, because two new lines are being removed.

So at first glance these freeform blocks seems to be an important part of the block parsing and block rendering.

We can remove the freeform block with:

<!-- wp:core/paragraph -->
<p>First paragraph</p>
<!-- /wp:core/paragraph --><!-- wp:core/paragraph -->
<p>Second paragraph</p>
<!-- /wp:core/paragraph -->

I hope I'm reviewing it correctly :-)

Maybe you see it differently @Chouby ?

Last edited 23 months ago by birgire (previous) (diff)

#2 @Chouby
23 months ago

The editor does not let me save

<!-- wp:core/paragraph -->
<p>First paragraph</p>
<!-- /wp:core/paragraph --><!-- wp:core/paragraph -->
<p>Second paragraph</p>
<!-- /wp:core/paragraph -->

It automatically changes it back to

<!-- wp:paragraph -->
<p>First paragraph</p>
<!-- /wp:paragraph -->

<!-- wp:paragraph -->
<p>Second paragraph</p>
<!-- /wp:paragraph -->

I am not sure that this is intentional. But it's indeed probably a consequence of how the free form text is parsed as

<!-- wp:paragraph -->
<p>First paragraph</p>
<!-- /wp:paragraph -->

<p>Free form text</p>

<!-- wp:paragraph -->
<p>Second paragraph</p>
<!-- /wp:paragraph -->

is parsed in

Array
(
    [0] => Array
        (
            [blockName] => core/paragraph
            [attrs] => stdClass Object
                (
                )

            [innerBlocks] => Array
                (
                )

            [innerHTML] => 
<p>First paragraph</p>

        )

    [1] => Array
        (
            [blockName] => 
            [attrs] => Array
                (
                )

            [innerBlocks] => Array
                (
                )

            [innerHTML] => 

<p>Free form text</p>


        )

    [2] => Array
        (
            [blockName] => core/paragraph
            [attrs] => stdClass Object
                (
                )

            [innerBlocks] => Array
                (
                )

            [innerHTML] => 
<p>Second paragraph</p>

        )

)

Maybe not a big deal, but it surprised me having all these empty blocks in the array returned.

#3 @dmsnell
22 months ago

@birgire thanks for the response; it's accurate.

@Chouby we're seeing a smashing of abstraction layers here. the \n\n freeform segments exist in the string post_content so they in a sense of reality are truly there. The biggest surprise is that the visual editor doesn't show them and it won't let you get rid of them in the underlying content…

https://cldup.com/lw06BVN1B8.gif

…while the parsed response has them…

[
  {
    "blockName": "core/paragraph",
    "attrs": {},
    "innerBlocks": [],
    "innerHTML": "\n<p>One explicit block.</p>\n",
    "innerContent": [
      "\n<p>One explicit block.</p>\n"
    ]
  },
  {
    "blockName": null,
    "attrs": {},
    "innerBlocks": [],
    "innerHTML": "\n\n",
    "innerContent": [
      "\n\n"
    ]
  },
  {
    "blockName": "core/paragraph",
    "attrs": {},
    "innerBlocks": [],
    "innerHTML": "\n<p>Two explicit blocks.</p>\n",
    "innerContent": [
      "\n<p>Two explicit blocks.</p>\n"
    ]
  }
]

Right now I believe that the editor is actually manually filtering the returned blocks and skipping them if they are empty after autop( innerHTML ).trim(). This adds to the confusion because they are ripped from the document then implicitly recreated on serialization when the post undergoes HTML prettifying.

There are many different ways I'm thinking about this in my head on what's really wrong here or how we should resolve this. From the parser's perspective there is no problem - we decided that anything that isn't a block is freeform content and that's why they are there.

Could you help out and clarify what issues specifically this is causing you if it's breaking something? Are you noting here because something is difficult to interact with or mainly because it was surprising?

Thanks!

#4 @Chouby
22 months ago

  • Resolution set to invalid
  • Status changed from new to closed

@dmsnell It's not preventing me to do anything. I reported it because it surprised me when I started working with the function. It's ok for me to close as wontfix if there is no side effect for the core.

#5 @pento
22 months ago

  • Milestone Awaiting Review deleted

#6 @dmsnell
22 months ago

Thanks @Chouby - I appreciate you taking the time to raise the issue and provide concrete examples to illustrate your point. I know that I'll be thinking about this and maybe at some point we'll raise the core issue at another level, such as in a more global way of defining behavior for inter-block areas, or defining an optional newline as part of the block specification

#7 @leec87
17 months ago

  • Resolution invalid deleted
  • Status changed from closed to reopened

Can this please be re-opened (I Don't know why it was closed), and investigated and fixed?

I've created a visual example on the Gutenberg GitHub as an issue (as I Thought it was a Gutenberg related thing) - https://github.com/WordPress/gutenberg/issues/15040

This is a bug, that hasn't been answered as to why it's happening here, and I believe has been closed for no real reason.

Can this be investigated and fixed?

To give a real world example, in my foreach loop, I'm adding alternating classes to each, based on the array index being odd/even. This is making this screw up as it's adding extra array elements that shouldn't be there. (resulting in 'Notice: Undefined offset: 0' errors.

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

#8 @dmsnell
17 months ago

re-opened…investigated and fixed

The reason this was closed is because the behavior was surprising but not wrong. Specifically the parser is doing everything it should do and the source of the unexpected behavior comes from the way that the block editor prettifies the output when it saves. It prettifies output by adding empty freeform HTML content between blocks in order to make the post_content more readable.

To that end I don't see any warrant to reopen this issue as there's nothing to fix here. We can raise conversation about the editor's behavior but I think that needs to happen in a place where the discussion is focussed on the behavior in question.


I'm adding alternating classes to each, based on the array index being odd/even

I'd be curious to see what code you are using to do this. From how I am interpreting what you are describing I'm surprised you are getting errors. Would you share a small snippet demonstrating the error?

$blocks = parse_blocks( $post->post_content );
foreach ( $blocks as $index => $block ) {
        # ???
}

On an unrelated note I would be interested in seeing what specifically you are trying to accomplish through adding CSS classes. There may be other issues in the way even if these empty blocks weren't there (I did provide an example function in the GitHub PR as a way to filter out these blocks).

You may find block filtering useful if you are wanting to modify the output on page render.

add_filter( 'render_block', function( $content, $block ) {
        static $is_even = true;

        if ( ! do_I_want_to_modify_this_block( $block ) ) {
                return $content;
        }

        $is_even = ! $is_even;
        return sprint(
                '<div class="%s">%s</div>',
                $is_even ? 'is-even' : 'is-odd',
                $content
        );
} );

#9 @leec87
17 months ago

Sure, I can show what I'm trying to do, maybe you might be able to give me a better idea of doing it.

You could take a look at https://staging.bandbubble.com/features/, which shows you the blocks I'm talking about.

<?php
$blocks = parse_blocks($post->post_content);
$count = 0;
foreach($blocks as $feature) { ?>
  <div class="py-5 text-center">
    <div class="container">
      <div class="row align-items-center">
        <div class="col-12 col-md-3 offset-md-1 offset-lg-2 <?php echo (++$count%2 ? '' : 'order-md-2'); ?>">
          <?php echo $feature['innerHTML']; ?>
        </div>
        <div class="col-12 col-md-7 col-lg-5 <?php echo (++$count%2 ? '' : 'order-md-1'); ?>">
          <h2>
            <?php echo $feature['innerBlocks'][0]['innerContent'][0]; ?>
          </h2>
          <?php echo wpautop($feature['innerBlocks'][1]['innerContent'][0]); ?>
        </div>
      </div>
    </div>
  </div>
<?php } ?>

So essentially, a simple if/else statement if the current count is divisible by 2, to echo a Bootstrap class that changes the order. The effect I'm going for here, is to swap the order of the image and text around (in a media/text block).

This should work fine, if the empty blocks weren't returned by parse_blocks() (If I add the CSS class to one of the regular blocks, it appears fine, so this should work).

This should be a nice, simple way to bridge a gap between Wordpress blocks, and Bootstrap markup / styling, which is what I'm trying to achieve. I could easily hard code this in the page template file, but I like being able to edit content in Wordpress.

#10 @dmsnell
17 months ago

  • Resolution set to invalid
  • Status changed from reopened to closed

show what I'm trying to do

Thanks @leec87 for sharing that code. I'm seeing where the Undefined offset: 0 error is coming in and I do think here will be further problems beyond he empty blocks if you keep that code.

The root cause is hard-coding assumptions about what blocks will appear. It may be the case in the posts you are working with that you get blocks in the structure you want to modify but generally we'll run into problems when we access $block['innerBlocks'][ $any_index ] without verifying that such inner blocks exist.

In your case I'd consider using something along the example code I have here instead of in the GitHub issue as it should end up being more reliable for you as you process posts of different block types. Most blocks we encounter will have no inner blocks.

The method I'd recommend involves different parts: the process you take will largely hinge on what role those CSS classes play. I'm not sure what you mean when you say "swap the order of the image and text around." I'm going to assume that you want alternating columns where odd rows have the image on the left, text on the right but even rows get the image on the right, text on the left.

Note that we should avoid printing the output of parse_blocks() because it won't continue processing nested blocks and it won't render shortcodes. We'll use do_blocks() instead.

Assumptions:

  • your media and text are sitting inside a custom block whose name is my/media-text-pair
  • your media is a core/image block and the text is a core/paragraph block

You'll be able to answer questions I can't because of my limited exposure to your code.

function is_media_text_pair( $block ) {
        if ( 'my/media-text-pair' !== $block['blockName'] ) {
                return false;
        }

        if ( count( $block['innerBlocks'] ) < 2 ) {
                return false;
        }

        if ( 'core/image' !== $block['innerBlocks'][ 0 ][ 'blockName' ] ) {
                return false;
        }

        if ( 'core/paragraph' !== $block['innerBlocks'][ 1 ][ 'blockName' ] ) {
                return false;
        }

        return true;
}

function reverse_inner_blocks( $block ) {
        $blocksReversed = array_reverse( $block['innerBlocks'] );

        return array_merge( $block, [ 'innerBlocks' => $blocksReversed ] );
}

function swap_media_text_pairs( $block ) {
        return is_media_text_pair( $block )
                ? reverse_inner_blocks( $block )
                : $block;
}

function add_zebra_stripes( $content, $block ) {
        static $is_even = true;

        if ( ! is_media_text_pair( $block ) ) {
                return $block;
        }

        $is_even = ! $is_even;

        return sprintf( '<div class="%s">%s</div>', $is_even ? 'order-md-1' : 'order-md-2', $content );
}

add_filter( 'render_block_data', 'swap_media_text_pairs' );
add_filter( 'render_block', 'add_zebra_stripes' );
$output = do_blocks( $post->post_content );

Feel free to poke me in the WordPress Slack team - I'm @dmsnell. I'm going to close this issue again in the meantime.

Note: See TracTickets for help on using tickets.