Skip to content

Commit

Permalink
Parser (Fix): Output freeform content before void blocks (#9984)
Browse files Browse the repository at this point in the history
* Parser (Fix): Output freeform content before void blocks

Resolves #9968

It was noted that a classic block preceding a void block would
disappear in the editor while if that same classic block preceded
the long-form non-void representation of an empty block then things
would load as expected.

This behavior was determined to originate in the new default parser
in #8083 and the bug was that with void blocks we weren't sending
any preceding HTML soup/freeform content into the output list.

In this patch I've duplicated some code from the block-closing
function of the parser to spit out this content when a void block
is at the top-level of the document.

This bug did not appear when void blocks are nested because it's
the parent block that eats HTML soup. In the case of the top-level
void however we were immediately pushing that void block to the
output list and neglecting the freeform HTML.

I've added a few tests to verify and demonstrate this behavior.
Actually, since I wasn't sure what was wrong I wrote the tests first
to try and understand the behaviors and bugs. There are a few tests
that are thus not entirely essential but worthwhile to have in here.
  • Loading branch information
dmsnell authored Sep 18, 2018
1 parent 050e32d commit 38e1ae0
Show file tree
Hide file tree
Showing 4 changed files with 74 additions and 7 deletions.
3 changes: 3 additions & 0 deletions packages/block-serialization-default-parser/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
## 1.0.1 (unreleased)
- Fix: Include freeform content preceding void blocks (#9984)

## 1.0.0

- Initial release.
20 changes: 16 additions & 4 deletions packages/block-serialization-default-parser/parser.php
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,13 @@ function parse( $document ) {
* @return bool
*/
function proceed() {
list( $token_type, $block_name, $attrs, $start_offset, $token_length ) = $this->next_token();
$next_token = $this->next_token();
list( $token_type, $block_name, $attrs, $start_offset, $token_length ) = $next_token;
$stack_depth = count( $this->stack );

// we may have some HTML soup before the next block
$leading_html_start = $start_offset > $this->offset ? $this->offset : null;

switch ( $token_type ) {
case 'no-more-tokens':
// if not in a block then flush output
Expand Down Expand Up @@ -239,6 +243,17 @@ function proceed() {
* in the top-level of the document
*/
if ( 0 === $stack_depth ) {
if ( isset( $leading_html_start ) ) {
$this->output[] = array(
'attrs' => array(),

This comment has been minimized.

Copy link
@mcsf

mcsf Sep 19, 2018

Contributor

In this same file I'm seeing conflicting shapes for attrs:

'attrs' => array(), // here and in `add_block_from_stack`
'attrs' => new stdClass(), // in `add_freeform`
'innerHTML' => substr(
$this->document,
$leading_html_start,
$start_offset - $leading_html_start
),
);
}

$this->output[] = new WP_Block_Parser_Block( $block_name, $attrs, array(), '' );
$this->offset = $start_offset + $token_length;
return true;
Expand All @@ -254,9 +269,6 @@ function proceed() {
return true;

case 'block-opener':
// we may have some HTML soup before the next block
$leading_html_start = $start_offset > $this->offset ? $this->offset : null;

// track all newly-opened blocks on the stack
array_push( $this->stack, new WP_Block_Parser_Frame(
new WP_Block_Parser_Block( $block_name, $attrs, array(), '' ),
Expand Down
12 changes: 9 additions & 3 deletions packages/block-serialization-default-parser/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ function proceed() {
const [ tokenType, blockName, attrs, startOffset, tokenLength ] = next;
const stackDepth = stack.length;

// we may have some HTML soup before the next block
const leadingHtmlStart = ( startOffset > offset ) ? offset : null;

switch ( tokenType ) {
case 'no-more-tokens':
// if not in a block then flush output
Expand Down Expand Up @@ -74,6 +77,12 @@ function proceed() {
// easy case is if we stumbled upon a void block
// in the top-level of the document
if ( 0 === stackDepth ) {
if ( null !== leadingHtmlStart ) {
output.push( {
attrs: {},
innerHTML: document.substr( leadingHtmlStart, startOffset - leadingHtmlStart ),
} );
}
output.push( Block( blockName, attrs, [], '' ) );
offset = startOffset + tokenLength;
return true;
Expand All @@ -89,9 +98,6 @@ function proceed() {
return true;

case 'block-opener':
// we may have some HTML soup before the next block
const leadingHtmlStart = ( startOffset > offset ) ? offset : null;

// track all newly-opened blocks on the stack
stack.push(
Frame(
Expand Down
46 changes: 46 additions & 0 deletions packages/block-serialization-default-parser/test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,4 +24,50 @@ describe( 'block-serialization-spec-parser', () => {
},
] );
} );

test( 'treats void blocks and empty blocks identically', () => {
expect( parse(
'<!-- wp:block /-->'
) ).toEqual( parse(
'<!-- wp:block --><!-- /wp:block -->'
) );

expect( parse(
'<!-- wp:my/bus { "is": "fast" } /-->'
) ).toEqual( parse(
'<!-- wp:my/bus { "is": "fast" } --><!-- /wp:my/bus -->'
) );
} );

test( 'should grab HTML soup before block openers', () => {
[
'<p>Break me</p><!-- wp:block /-->',
'<p>Break me</p><!-- wp:block --><!-- /wp:block -->',
].forEach( ( input ) => expect( parse( input ) ).toEqual( [
expect.objectContaining( { innerHTML: '<p>Break me</p>' } ),
expect.objectContaining( { blockName: 'core/block', innerHTML: '' } ),
] ) );
} );

test( 'should grab HTML soup before inner block openers', () => {
[
'<!-- wp:outer --><p>Break me</p><!-- wp:block /--><!-- /wp:outer -->',
'<!-- wp:outer --><p>Break me</p><!-- wp:block --><!-- /wp:block --><!-- /wp:outer -->',
].forEach( ( input ) => expect( parse( input ) ).toEqual( [
expect.objectContaining( {
innerBlocks: [ expect.objectContaining( { blockName: 'core/block', innerHTML: '' } ) ],
innerHTML: '<p>Break me</p>',
} ),
] ) );
} );

test( 'should grab HTML soup after blocks', () => {
[
'<!-- wp:block /--><p>Break me</p>',
'<!-- wp:block --><!-- /wp:block --><p>Break me</p>',
].forEach( ( input ) => expect( parse( input ) ).toEqual( [
expect.objectContaining( { blockName: 'core/block', innerHTML: '' } ),
expect.objectContaining( { innerHTML: '<p>Break me</p>' } ),
] ) );
} );
} );

0 comments on commit 38e1ae0

Please sign in to comment.