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

Paginated content #12218

Closed
Rahe opened this issue Nov 22, 2018 · 8 comments
Closed

Paginated content #12218

Rahe opened this issue Nov 22, 2018 · 8 comments
Labels
[Feature] Block API API that allows to express the block paradigm. [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended
Milestone

Comments

@Rahe
Copy link
Contributor

Rahe commented Nov 22, 2018

Describe the bug
When I have a dynamic bloc with a render_callback function and a paginated content (<--nextpage-->) the dynamic block are not rendered.

To Reproduce
Steps to reproduce the behavior:

  1. Go in admin
  2. Add an content
  3. Add content :
<!-- wp:heading -->
<h2>Test</h2>
<!-- /wp:heading -->

<!-- wp:nextpage -->
<!--nextpage-->
<!-- /wp:nextpage -->

<!-- wp:quote -->
<blockquote class="wp-block-quote"><p>TEST</p><cite>ETSTTSTT</cite></blockquote>
<!-- /wp:quote -->

<!-- wp:latest-posts /-->
  1. Go to front office, load the page 2

Expected behavior
I expect that the dynamic block is rendered on page 2, but this is not the case.

Desktop (please complete the following information):

  • OS: MACOS
  • Browser Chrome
  • Version 70.0.3538.77

Additional context

  • Version 4.5.1, but appears since 4.4.0 works fine in 4.3.0
@youknowriad youknowriad added the [Type] Bug An existing feature does not function as intended label Nov 22, 2018
@youknowriad
Copy link
Contributor

I suspect this being a parsing issue where the parsed string passed to do_blocks or something is starting with <!-- /wp:nextpage --> causing the issue.

cc @dmsnell @pento

Maybe we should just trim <!-- wp:nextpage --> and <!-- /wp:nextpage --> from the strings?

@swissspidy
Copy link
Member

Introduced in #1467. There were some discussions about changing the parser so these blocks would always be saved as <!--nextpage--> without <!-- wp:nextpage --> and <!-- /wp:nextpage -->, but that was reverted again.

@dmsnell
Copy link
Member

dmsnell commented Nov 22, 2018

If you plug in the example content into the parser explorer (you can load the spec grammar by clicking on "fetch") then you will see that it does what it's supposed to do and parse the document into a list of blocks.

(I had originally asked for follow-up information from @Rahe but since I believe I have found the more specific problem - "blocks aren't processed on non-first pages" - then I removed the request)

my guess is that we have some different code path for following pages in multi-page documents that skips the block processing altogether. @aduth I'm not sure who to ping but I think we only run content through do_blocks on the first page of a multi-page document.

check out the rendered HTML for page two of my test document…
screen shot 2018-11-22 at 8 47 34 am

I'm not sure where even to begin, but I can try and look into this. seems like a critical priority. cc: @mtias

@dmsnell dmsnell added [Priority] High Used to indicate top priority items that need quick attention [Feature] Block API API that allows to express the block paradigm. labels Nov 22, 2018
@mtias mtias added this to the WordPress 5.0 milestone Nov 22, 2018
@dmsnell
Copy link
Member

dmsnell commented Nov 22, 2018

Update

It seems like my assessment was wrong about not processing the successive pages. Instead, what's happening is that we're sending an invalid document to do_blocks() and that fails in the default parser in a different way than the spec parser.

Once WordPress strips out <!--nexpage--> it passes a document to do_blocks() that starts with <!-- /wp:nextpage --> which is invalid. The default parser fails and closes out the rest of the document.

I'll try and make at least one attempt at a quick-and-minimal fix from inside do_blocks(), but we know that the real fix is to update the default parser so that it's actually a PEG and conforms to the same failure modes as the spec parser.

@mcsf
Copy link
Contributor

mcsf commented Nov 22, 2018

Once WordPress strips out <!--nexpage--> it passes a document to do_blocks() that starts with <!-- /wp:nextpage --> which is invalid. The default parser fails and closes out the rest of the document.

Was starting to look at this and getting at the same conclusion. Looking forward for a quick patch. <3

@azaozz
Copy link
Contributor

azaozz commented Nov 22, 2018

Once WordPress strips out it passes a document to do_blocks() that starts with which is invalid. The default parser fails and closes out the rest of the document.

Heh, got there too few min ago... Parallel work :)

A very crude fix is

	
	if ( strpos( $content, '<!-- /wp:nextpage -->' ) === 0 ) {
		$content = preg_replace( '@^<!-- /wp:nextpage -->\s*@', '', $content );
	}

	if ( strrpos( $content, '<!-- wp:nextpage -->' ) === strlen( $content  ) - 20 ) {
		$content = preg_replace( '@\s*<!-- wp:nextpage -->$@', '', $content );
	}

in do_blocks() before $blocks = parse_blocks( $content );.

@mtias
Copy link
Member

mtias commented Nov 22, 2018

Cross-referencing ticket in core trac: https://core.trac.wordpress.org/ticket/45401

@youknowriad
Copy link
Contributor

I'm closing as this has been fixed upstream

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Block API API that allows to express the block paradigm. [Priority] High Used to indicate top priority items that need quick attention [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

7 participants