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

Parsing Issues with classic editor and php rendered blocks. #9968

Closed
jrmd opened this issue Sep 17, 2018 · 4 comments
Closed

Parsing Issues with classic editor and php rendered blocks. #9968

jrmd opened this issue Sep 17, 2018 · 4 comments
Labels
[Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f [Type] Regression Related to a regression in the latest release

Comments

@jrmd
Copy link

jrmd commented Sep 17, 2018

Describe the bug
in Gutenberg 3.8 where the classic editor doesnt show in the backend if a php rendered block is added just after the classic editor block.

It seems to be something to do with how the comments work. For example my php rendered block would be stored in the database like this:
<!-- wp:my-blocks/block-custom /-->

This causes the classic editor not to render, however if you change the comment to
<!-- wp:my-blocks/block-custom --><!-- /wp:my-blocks/block-custom -->
Everything will render in the editor correctly

Going back through the commits - it seems that it was caused by the new parser ( #8083 )

If my description you can see the conversation here in slack - https://wordpress.slack.com/archives/C02QB2JS7/p1537188435000100

To Reproduce
Steps to reproduce the behavior:

  1. Add Classic editor block add some content
  2. Directly after add the latest posts block (its a php rendered block)
  3. Save the post
  4. Refresh to see that the classic editor is not visible even though the content is in the database.

Expected behavior
Classic editor should be visible

Additional context

  • Gutenberg 3.8
@aduth aduth added [Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f [Type] Regression Related to a regression in the latest release labels Sep 17, 2018
@dmsnell
Copy link
Member

dmsnell commented Sep 17, 2018

Strange…thanks for reporting! I haven't tried reproducing with those steps yet but I tried adding a new test to the parser just to see what's happening and I think we'll need to dig in more to find out what's up.

diff --git a/packages/block-serialization-default-parser/test/index.js b/packages/block-serialization-default-parser/test/index.js
index 68a46b39c..90eaf5cc9 100644
--- a/packages/block-serialization-default-parser/test/index.js
+++ b/packages/block-serialization-default-parser/test/index.js
@@ -24,4 +24,11 @@ describe( 'block-serialization-spec-parser', () => {
 			},
 		] );
 	} );
+
+	test( 'treats void blocks and empty blocks identically', () => {
+		const voidBlock = parse( '<!-- wp:block /-->' );
+		const emptyBlock = parse( '<!-- wp:block --><!-- /wp:block -->' );
+
+		expect( voidBlock ).toEqual( emptyBlock );
+	} );
 } );

I'll check in later but maybe this has something to do with another oddity? @jrmd I'll try and reproduce later with the steps you added, but in the meantime if you could post here a copy of the test document that produced the error that would be helpful (for example, the minimal document showing the bug).

Maybe there's something funny with newlines or whitespace…

@jrmd
Copy link
Author

jrmd commented Sep 17, 2018

@dmsnell

here is the entire post content to make this issue occur

<p>Hello World</p>

<!-- wp:latest-posts /-->

@chrisvanpatten
Copy link
Contributor

I was able to reproduce this. You can also see in Code Editor mode that if you paste in the content @jrmd provided, the classic block content is immediately deleted.

dmsnell added a commit that referenced this issue Sep 18, 2018
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.
@dmsnell
Copy link
Member

dmsnell commented Sep 18, 2018

Found it! this one was staring us down - I'm not sure why I missed it but the fix is simple. It was the case where we had leading HTML in front of a void block and we weren't pushing it to the output list.

I've submitted a patch in #9984 and it's ready for review. If you are able to confirm that this fixes the problem for you that'd be awesome to have the additional confirmation.

dmsnell added a commit that referenced this issue Sep 18, 2018
* 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.
mcsf pushed a commit that referenced this issue Sep 21, 2018
* 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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

No branches or pull requests

4 participants