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.

Sign up for GitHub

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

Return inner HTML before and after inner blocks when parsing and fix … #8760

Closed
wants to merge 1 commit into from
Closed

Return inner HTML before and after inner blocks when parsing and fix … #8760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion core-blocks/test/fixtures/core__column.parsed.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
"innerHTML": "\n\t<p>Column One, Paragraph Two</p>\n\t"
}
],
"innerHTML": "\n<div class=\"wp-block-column\">\n\t\n\t\n</div>\n"
"innerHTML": "\n<div class=\"wp-block-column\">\n\t\n\t\n</div>\n",
"innerHTMLBeforeInnerBlocks": "\n<div class=\"wp-block-column\">\n\t",
"innerHTMLAfterInnerBlocks": "\n\t\n</div>\n"
},
{
"attrs": {},
Expand Down
10 changes: 10 additions & 0 deletions core-blocks/test/fixtures/core__column.rendered.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@

<div class="wp-block-column">

<p>Column One, Paragraph One</p>

<p>Column One, Paragraph Two</p>


</div>

12 changes: 9 additions & 3 deletions core-blocks/test/fixtures/core__columns.parsed.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,9 @@
"innerHTML": "\n\t\t<p>Column One, Paragraph Two</p>\n\t\t"
}
],
"innerHTML": "\n\t<div class=\"wp-block-column\">\n\t\t\n\t\t\n\t</div>\n\t"
"innerHTML": "\n\t<div class=\"wp-block-column\">\n\t\t\n\t\t\n\t</div>\n\t",
"innerHTMLBeforeInnerBlocks": "\n\t<div class=\"wp-block-column\">\n\t\t",
"innerHTMLAfterInnerBlocks": "\n\t\t\n\t</div>\n\t"
},
{
"blockName": "core/column",
Expand All @@ -41,10 +43,14 @@
"innerHTML": "\n\t\t<p>Column Three, Paragraph One</p>\n\t\t"
}
],
"innerHTML": "\n\t<div class=\"wp-block-column\">\n\t\t\n\t\t\n\t</div>\n\t"
"innerHTML": "\n\t<div class=\"wp-block-column\">\n\t\t\n\t\t\n\t</div>\n\t",
"innerHTMLBeforeInnerBlocks": "\n\t<div class=\"wp-block-column\">\n\t\t",
"innerHTMLAfterInnerBlocks": "\n\t\t\n\t</div>\n\t"
}
],
"innerHTML": "\n<div class=\"wp-block-columns has-3-columns\">\n\t\n\t\n</div>\n"
"innerHTML": "\n<div class=\"wp-block-columns has-3-columns\">\n\t\n\t\n</div>\n",
"innerHTMLBeforeInnerBlocks": "\n<div class=\"wp-block-columns has-3-columns\">\n\t",
"innerHTMLAfterInnerBlocks": "\n\t\n</div>\n"
},
{
"attrs": {},
Expand Down
24 changes: 24 additions & 0 deletions core-blocks/test/fixtures/core__columns.rendered.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@

<div class="wp-block-columns has-3-columns">

<div class="wp-block-column">

<p>Column One, Paragraph One</p>

<p>Column One, Paragraph Two</p>


</div>

<div class="wp-block-column">

<p>Column Two, Paragraph One</p>

<p>Column Three, Paragraph One</p>


</div>


</div>

7 changes: 7 additions & 0 deletions lib/blocks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,13 @@ function gutenberg_render_block( $block ) {
}
}

if ( isset( $block['innerBlocks'] ) && count( $block['innerBlocks'] ) ) {
$raw_content = $block['innerHTMLBeforeInnerBlocks'];
foreach ( $block['innerBlocks'] as $inner_block ) {
$raw_content .= gutenberg_render_block( $inner_block );
Copy link
Member

Choose a reason for hiding this comment

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

although we shouldn't expect deeply nested blocks we may want to consider the possibility of their existence. suppose someone made a post with 1000 levels of nesting - that could theoretically cause WordPress to crash on account of overflowing the call stack.

I'd recommend considering one of two changes to the behavior here:

  • set a limit on the recursion and abort with some defined behavior when we're too deep
  • flip the recursion on its head and recurse using a trampoline structure to track the recursion in an alternate data structure or control structure to eliminate the risk of stack overflow

it should be noted that this work overlaps what's happening in #8083. if we get a fast internal PHP parser we might dramatically rewrite this function.

Copy link
Author

Choose a reason for hiding this comment

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

@dmsnell Is it worth continuing with this considering you might change the PHP parser. The intention of this change is to fix rendering of inner blocks that doesn't happen at all right now.

}
$raw_content .= $block['innerHTMLAfterInnerBlocks'];
}
if ( $raw_content ) {
return $raw_content;
}
Expand Down
31 changes: 27 additions & 4 deletions lib/parser.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,14 +266,37 @@ private function peg_f3($blockName, $attrs) {
);
}
private function peg_f4($s, $children, $e) {
list( $innerHTML, $innerBlocks ) = peg_array_partition( $children, 'is_string' );

return array(
$innerHTMLBeforeInnerBlocks = array();
$innerHTMLAfterInnerBlocks = array();
$innerBlocks = array();

$inner_found = false;
foreach ( $children as $child ) {
if( is_string( $child ) ) {
if ( $inner_found ) {
$innerHTMLAfterInnerBlocks[] = $child;
} else {
$innerHTMLBeforeInnerBlocks[] = $child;
}
} else {
$innerBlocks[] = $child;
$inner_found = true;
}
}


$block = array(
'blockName' => $s['blockName'],
'attrs' => $s['attrs'],
'innerBlocks' => $innerBlocks,
'innerHTML' => implode( '', $innerHTML ),
'innerHTML' => implode( '', $innerHTMLBeforeInnerBlocks ) . implode( '', $innerHTMLAfterInnerBlocks ),
);

if( $inner_found ) {
$block['innerHTMLBeforeInnerBlocks'] = implode( '', $innerHTMLBeforeInnerBlocks );
$block['innerHTMLAfterInnerBlocks'] = implode( '', $innerHTMLAfterInnerBlocks );
}
return $block;
Copy link
Member

Choose a reason for hiding this comment

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

hm…it seems like we are trying to manually edit automatically-generated code here. @brucepearson did you get a chance to look at the spec grammar which is run through php-pegjs to generate this file?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks @dmsnell. Is the concept OK that I've applied here? I can then translate to the grammar file.

}
private function peg_f5($blockName, $attrs) {
return array(
Expand Down
43 changes: 43 additions & 0 deletions phpunit/class-render-test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php
/**
* gutenberg_render_block tests
*
* @package Gutenberg
*/

class Render_Test extends WP_UnitTestCase {
protected static $fixtures_dir;

/**
* @test
* @dataProvider inner_block_files
*/
public function it_renders_inner_blocks( $blocks_file, $rendered_file ) {
self::$fixtures_dir = dirname( dirname( __FILE__ ) ) . '/core-blocks/test/fixtures';

require_once dirname( dirname( __FILE__ ) ) . '/lib/blocks.php';

$blocks = json_decode( self::strip_r( file_get_contents( self::$fixtures_dir . $blocks_file ) ), true );
$expected = self::strip_r( file_get_contents( self::$fixtures_dir . $rendered_file ) );

$content = '';
foreach ( $blocks as $block ) {
$content .= gutenberg_render_block( $block );
}

$this->assertEquals( $expected, $content );

}

function strip_r( $input ) {
return str_replace( "\r", '', $input );
}

public function inner_block_files() {
return array(
array( '/core__column.parsed.json', '/core__column.rendered.html' ),
array( '/core__columns.parsed.json', '/core__columns.rendered.html' ),
);
}

}