Fix missing properties in block arrays#10735
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
|
Not sure why https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/class-wp-block-parser-block.php is not present in trunk? |
|
@mukeshpanchal27 Looks like that's an effect of 22294af for https://core.trac.wordpress.org/ticket/64393. |
| 'attrs' => $attributes, | ||
| 'innerHTML' => '', | ||
| 'innerContent' => array(), | ||
| $block = (array) new WP_Block_Parser_Block( |
There was a problem hiding this comment.
This is a clever way to ensure that any future properties automatically get included. However, if private properties are ever included, then this will result in some unexpected array keys being returned. See https://3v4l.org/JNfY7#veol
So maybe get_object_vars() would be a bit safer?
There was a problem hiding this comment.
That's a great point. While it would be safer, I also observe (and I didn't realize this until right now) that WP_Block_Parser itself also uses the casting approach (example). Better to be consistent here? I don't have a strong preference, just mentioning it.
|
@westonruter As I was doing some other work, I happened to notice that the |
Why it didn't show any error then? |
It might cause errors, I don't know. I'm not using hooked blocks currently in my own code. This is just an observation from reading the function. |
Probably best to get @dmsnell's thoughts on this. |
dlh01
changed the title
dmsnell
left a comment
dmsnell
left a comment
There was a problem hiding this comment.
Thanks for your understanding and flexibility. I think this is an appropriate tactical change to prevent issues without involving broader questions.
I did end up tossing something together on Friday but didn’t push it out. It does indeed raise some new questions, but I think it does have a role. Just it would be better to get it out on its proper timeline than to rush it for this change.
There have been a few places in WordPress where a method creates “an array representation simulating the output of parse_blocks”, but the array is missing one or more keys. The missing keys cause issue with downstream code, which either treats a block without, for example, `innerBlocks` as invalid input, or more commonly, assumes that `innerBlocks` is present and accesses it unconditionally. This can even lead to crashes. This patch adds the missing properties so that the code behaves as expected. Since the array values are empty by default, adding these missing keys does not present any intentional behavioral changes — only a safer default. Developed in: #10735 Discussed in: https://core.trac.wordpress.org/ticket/64505 Follow-up to [47360], [57354]. Props dlh, dmsnell, mukesh27, westonruter. Fixes #64505. git-svn-id: https://develop.svn.wordpress.org/trunk@62039 602fd350-edb4-49c9-b593-d223f7449a82
There have been a few places in WordPress where a method creates “an array representation simulating the output of parse_blocks”, but the array is missing one or more keys. The missing keys cause issue with downstream code, which either treats a block without, for example, `innerBlocks` as invalid input, or more commonly, assumes that `innerBlocks` is present and accesses it unconditionally. This can even lead to crashes. This patch adds the missing properties so that the code behaves as expected. Since the array values are empty by default, adding these missing keys does not present any intentional behavioral changes — only a safer default. Developed in: WordPress/wordpress-develop#10735 Discussed in: https://core.trac.wordpress.org/ticket/64505 Follow-up to [47360], [57354]. Props dlh, dmsnell, mukesh27, westonruter. Fixes #64505. Built from https://develop.svn.wordpress.org/trunk@62039 git-svn-id: http://core.svn.wordpress.org/trunk@61321 1a063a9b-81f0-0310-95a4-ce76da25c4cd
4 participants
Trac ticket: https://core.trac.wordpress.org/ticket/64505