Elements: check value and whitelist before building style nodes#43622
Elements: check value and whitelist before building style nodes#43622
Conversation
Check for element name in the whitelist
| if ( isset( $theme_json['styles']['elements'] ) ) { | ||
| foreach ( self::ELEMENTS as $element => $selector ) { | ||
| if ( ! isset( $theme_json['styles']['elements'][ $element ] ) ) { | ||
| if ( ! isset( $theme_json['styles']['elements'][ $element ] ) || empty( static::ELEMENTS[ $element ] ) ) { |
There was a problem hiding this comment.
This will produce an Undefined index in PHP 5.6 - https://3v4l.org/vqKpS#v5.6.40, and we cannot use isset() since it will result in a fatal error.
So let's use array_key_exists, similar to #42567.
There was a problem hiding this comment.
Thanks @Mamaduka
Just checking, do you mean the isset( $theme_json['styles']['elements'][ $element ] ) or empty( static::ELEMENTS[ $element ] )?
Or isn't empty okay with constants either?
I'll update to
if ( ! isset( $theme_json['styles']['elements'][ $element ] ) || ! array_key_exists( $element, static::ELEMENTS ) ) {
There was a problem hiding this comment.
Sorry for not being clear; it's still early morning for me ☕
I mean - empty( static::ELEMENTS[ $element ] )
The isset and empty don't work as you expect with class constants in PHP 5.6.
There was a problem hiding this comment.
Ah great. I knew about isset, but TIL about empty. Thanks a lot for the nudge.
Updated.
There was a problem hiding this comment.
Thanks for the quick follow-up.
mcsf
left a comment
There was a problem hiding this comment.
Thanks for the quick fix :)
|
Thanks for taking care of this. |
|
Hey, while working on #46579 I realized this PR was not backported to WordPress 6.1. When we land that PR and then backport to WordPress 6.2 everything will be fine, but I wanted to raise in case you think this should be ported for a potential WordPress 6.1.X release. |
|
Thanks for the heads up @oandregal |
Core patch, just in case:
What?
In
WP_Theme_JSON::get_style_nodes()Context: #41160 (comment)
Why?
If an element does not exist in the
static::ELEMENTSwhitelist, PHP will complain aboutNotice: Undefined index.This won't fix the current notice in core, but is a preparatory patch for the 6.1 migration.
Testing Instructions
Check that the tests pass.
To check manually that things are still working, here's some test theme.json
{ "version": 2, "settings": { "appearanceTools": true, "layout": { "contentSize": "840px", "wideSize": "1100px" } }, "styles": { "elements": { "button": { "color": { "background": "green" } } } } }Using this JSON, your buttons should have a green background.