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

Parser: Fix default PHP parser to cast inner blocks as arrays #11678

Merged
merged 1 commit into from
Nov 10, 2018

Conversation

dmsnell
Copy link
Member

@dmsnell dmsnell commented Nov 9, 2018

Since the introduction of the default parsers we have had a bug in the
PHP version whereby inner blocks were being popped onto the output
stack as PHP classes instead of as simple associated arrays. Blocks
that weren't inner blocks were being properly type-casted as arrays.

This patch adds the cast in where it's needed in order to fix this
inconsistent behavior.

So far this hasn't caused any troubles or exposed itself but while
working on other PRs like #11434 it became evident in test code.

Testing

Hopefully you will see by inspection how limited this change is.
Nothing internally should be relying on the WP_Block_Parser_Block
classes so this change should simply be removing that leak.

Load up Gutenberg, try some things - I estimate that a smoke-test
should uncover problems.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

Since the introduction of the default parsers we have had a bug in the
PHP version whereby inner blocks were being popped onto the output
stack as PHP classes instead of as simple associated arrays. Blocks
that weren't inner blocks were being properly type-casted as arrays.

This patch adds the cast in where it's needed in order to fix this
inconsistent behavior.

So far this hasn't caused any troubles or exposed itself but while
working on other PRs like #11434 it became evident in test code.
@dmsnell dmsnell added the [Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f label Nov 9, 2018
@dmsnell dmsnell requested review from mcsf and pento November 9, 2018 16:08
@@ -457,7 +457,7 @@ function add_freeform( $length = null ) {
*/
function add_inner_block( WP_Block_Parser_Block $block, $token_start, $token_length, $last_offset = null ) {
$parent = $this->stack[ count( $this->stack ) - 1 ];
$parent->block->innerBlocks[] = $block;
$parent->block->innerBlocks[] = (array) $block;
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

@gwwar gwwar left a comment

Choose a reason for hiding this comment

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

Thanks @dmsnell. Code changes here seem pretty safe. I've also smoke tested a bit without noticing anything out of the ordinary.

Let's wait for core folks to chime in, but I think this looks pretty good to me 👍

Copy link
Member

@pento pento left a comment

Choose a reason for hiding this comment

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

I did the same thing in core while I was looking the recursive block parsing problem. 🙂

@dmsnell dmsnell merged commit 50380b3 into master Nov 10, 2018
@dmsnell dmsnell deleted the parser/fix-inner-block-array-cast branch November 10, 2018 01:43
@mtias mtias added Internationalization (i18n) Issues or PRs related to internationalization efforts and removed Internationalization (i18n) Issues or PRs related to internationalization efforts labels Nov 12, 2018
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants