Full Site Editing: PHP Warning with incomplete presets

[jeherve]

In some scenarios, especially on sites that started experimenting with Full Site Editing with the Gutenberg plugin when the feature was still under development, and then switched to WordPress 5.9, one may run into the following warning on the frontend, when using a block-based theme:

Warning: Undefined array key "fontFamily" in /wp-includes/class-wp-theme-json.php on line 1093

I believe the attached patch should get rid of the warning.

For reference, the warning was the following when using the Gutenberg plugin:

Warning: Undefined array key "fontFamily" in /wp-content/plugins/gutenberg/lib/compat/wordpress-5.9/class-wp-theme-json-gutenberg.php

Let me know if I should create this patch against https://github.com/WordPress/gutenberg/blob/efafb0a561c8ef2e171e6cb6430c3dbc00709bc0/lib/compat/wordpress-5.9/class-wp-theme-json-5-9.php instead.

Thank you!

[jeherve]

Check if isset before to use.

[swissspidy]

Instead of isset( $preset_metadata['value_key'] ) && isset( $preset[ $preset_metadata['value_key'] ] ) you can just combine the isset() calls like so:

isset( $preset_metadata['value_key'], $preset[ $preset_metadata['value_key'] ] )

[SergeyBiryukov]

Replying to swissspidy:

you can just combine the isset() calls like so:

isset( $preset_metadata['value_key'], $preset[ $preset_metadata['value_key'] ] )

Technically true, but that doesn't seem to be a common pattern in core, we seem to generally check multiple values separately. 55161.diff​ looks a bit more readable to me as is :)

[jrf]

Replying to SergeyBiryukov:

Replying to swissspidy:

you can just combine the isset() calls like so:

isset( $preset_metadata['value_key'], $preset[ $preset_metadata['value_key'] ] )

Technically true, but that doesn't seem to be a common pattern in core, we seem to generally check multiple values separately. 55161.diff​ looks a bit more readable to me as is :)

The fact that it's not used much in Core is no reason not to use it. I concur with @swissspidy that doing the check in one call to isset() is more efficient and is a good pattern to introduce in Core.

[jeherve]

Simplify isset checks

[hellofromTonya]

I agree with using one isset().

55161-2.diff​ LGTM. Marking for commit.

[hellofromTonya]

In 52763:

Editor: Adds an additional check to guard against incompete presets.

Adds an additional guard to ensure the value of $preset_metadata['value_key'] actually exists as a key in the $preset array. Fixes Warning: Undefined array key error.

Intentionally adds the check into the existing isset() as it's native to PHP, more efficient, and a good pattern.

Follow-up [52049].

Props jeherve, swissspidy, sergeybiryukov, jrf.
Fixes #55161.

[hellofromTonya]

Reopening for backport to 5.9 branch for 5.9.1 inclusion.

[SergeyBiryukov]

[52763] looks good to backport.

[hellofromTonya]

In 52764:

Editor: Adds an additional check to guard against incompete presets.

Adds an additional guard to ensure the value of $preset_metadata['value_key'] actually exists as a key in the $preset array. Fixes Warning: Undefined array key error.

Intentionally adds the check into the existing isset() as it's native to PHP, more efficient, and a good pattern.

Follow-up [52049].

Props jeherve, swissspidy, sergeybiryukov, jrf.
Merges [52763] to the 5.9 branch.
Fixes #55161.

[hellofromTonya]

Let me know if I should create this patch against ​https://github.com/WordPress/gutenberg/blob/efafb0a561c8ef2e171e6cb6430c3dbc00709bc0/lib/compat/wordpress-5.9/class-wp-theme-json-5-9.php instead.

It would be great to also apply this same change in the Gutenberg repo. @jeherve would you like to create a PR for it?

[jeherve]

Replying to hellofromTonya:

It would be great to also apply this same change in the Gutenberg repo. @jeherve would you like to create a PR for it?

Sure thing. Done so here:
​https://github.com/WordPress/gutenberg/pull/38902