-
Notifications
You must be signed in to change notification settings - Fork 826
Widget Visibility: Fix thrown PHP error under specific block conditions #45087
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
base: trunk
Are you sure you want to change the base?
Widget Visibility: Fix thrown PHP error under specific block conditions #45087
Conversation
…ck (#45080) * Added logic to process $instance['content'] as an array, ensuring compatibility with nested structures and serialization of block arrays. * Improved validation to return early if content is not a valid string after processing.
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Jetpack plugin: No scheduled milestone found for this plugin. If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
Code Coverage SummaryCoverage changed in 1 file.
|
* Updated logic to ensure proper processing of $instance['content'], including normalization to string and improved validation for block visibility. * Enhanced comments for clarity on content structure handling.
* Updated the visibility condition to check for empty content more effectively, ensuring that the logic correctly identifies invalid content scenarios. * Improved overall robustness of content handling in widget visibility checks.
* Updated assertions in Jetpack_Widget_Conditions_Test to utilize PHPUnit\Framework\Assert for consistency. * Added new test cases to verify handling of parsed blocks arrays and unknown array shapes in filter_widget method. * Enhanced test coverage for visibility rules with various block configurations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't test, but the code looks like it should address the issue. Thanks!
@@ -826,8 +826,29 @@ public static function filter_widget( $instance ) { | |||
return $instance; | |||
} | |||
return false; | |||
} elseif ( ! empty( $instance['content'] ) && has_blocks( $instance['content'] ) ) { | |||
$scanner = Block_Scanner::create( $instance['content'] ); | |||
} elseif ( ! empty( $instance['content'] ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could invert this and return early, de-nesting the rest of the block.
Also, thanks for adding test coverage! |
Fixes HOG-336: Block Scanner: PHP Fatal TypeError in Widget Visibility Conditions
Proposed changes:
$instance['content']
to a string before callinghas_blocks()
inJetpack_Widget_Conditions::filter_widget
.serialize_blocks()
to only run on parsed block arrays (checks forblockName
), and bail safely on unknown/empty array shapes.has_blocks()
is invoked with an array.Jetpack product discussion
N/A
Does this pull request change what data or activity we track or use?
N/A
Testing instructions:
Prereqs:
wp-config.php
if not already:define( 'WP_DEBUG', true );
define( 'WP_DEBUG_LOG', true );
define( 'WP_DEBUG_DISPLAY', false );
wp-content/debug.log
while testing.Manual verification:
wp-content/debug.log
.Simulate "array content" code paths using a temporary MU plugin:
wp-content/mu-plugins/test-widget-visibility-content-shapes.php
, or use an existing01-snippets.php
:With your test widget in place:
?force_arr_content=1
→ no errors; visibility rules respected.?force_nested_content=1
→ no errors; visibility rules respected.?force_unknown_shape=1
→ no errors; widget displays (safe fallback).Check
wp-content/debug.log
for any TypeErrors/notices; expect none.Unit tests:
Before / After:
has_blocks()
could be called with an array, causing a fatal error on PHP 8+ under certain block conditions.content
is normalized first;has_blocks()
only receives strings; array shapes are handled safely or bailed out conservatively.