Skip to content
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/wp-includes/widgets.php
Original file line number Diff line number Diff line change
Expand Up @@ -1350,6 +1350,13 @@ function retrieve_widgets( $theme_changed = false ) {
$sidebars_widgets = _wp_remove_unregistered_widgets( $sidebars_widgets, $registered_widgets_ids );
$sidebars_widgets = wp_map_sidebars_widgets( $sidebars_widgets );

// Replace null values inside the array with an empty array.
foreach ( $sidebars_widgets as $key => $value ) {
if ( null === $value ) {
Copy link
Member

Choose a reason for hiding this comment

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

What if it was set to something other than an array or null, like a string, boolean, integer, or object? Wouldn't this be more appropriate:

Suggested change
if ( null === $value ) {
if ( ! is_array( $value ) ) {

Copy link
Author

Choose a reason for hiding this comment

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

moving forward with this approach

$sidebars_widgets[ $key ] = array();
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively:

Suggested change
// Replace null values inside the array with an empty array.
foreach ( $sidebars_widgets as $key => $value ) {
if ( null === $value ) {
$sidebars_widgets[ $key ] = array();
}
}
$sidebars_widgets = array_filter( $sidebars_widgets, 'is_array' );

Copy link
Author

Choose a reason for hiding this comment

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

I think checking for is_array will be a better solution and is a drop in replacement because it preserves sidebar keys and guarantees any downstream operations receive arrays, this is consistent with how older PHP version like 7.4 handle array_merge and array_search.

Using array_filter($sidebars_widgets, 'is_array') will remove entries that are not arrays, which can drop keys and change control flow in the downstream code that expects those keys to exist.


// Find hidden/lost multi-widget instances.
$shown_widgets = array_merge( ...array_values( $sidebars_widgets ) );
$lost_widgets = array_diff( $registered_widgets_ids, $shown_widgets );
Expand Down Expand Up @@ -1511,6 +1518,13 @@ function wp_map_sidebars_widgets( $existing_sidebars_widgets ) {

$old_sidebars_widgets = _wp_remove_unregistered_widgets( $old_sidebars_widgets );

// Replace null values inside the array with an empty array.
foreach ( $new_sidebars_widgets as $key => $value ) {
if ( null === $value ) {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if ( null === $value ) {
if ( ! is_array( $value ) ) {

$new_sidebars_widgets[ $key ] = array();
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Alternatively:

Suggested change
// Replace null values inside the array with an empty array.
foreach ( $new_sidebars_widgets as $key => $value ) {
if ( null === $value ) {
$new_sidebars_widgets[ $key ] = array();
}
}
$new_sidebars_widgets = array_filter( $new_sidebars_widgets, 'is_array' );


if ( ! empty( $old_sidebars_widgets ) ) {

// Go through each remaining sidebar...
Expand Down
55 changes: 55 additions & 0 deletions tests/phpunit/tests/widgets.php
Original file line number Diff line number Diff line change
Expand Up @@ -1359,4 +1359,59 @@ public function test_sidebar_guessing_one_menu_per_sidebar() {
);
$this->assertSameSetsWithIndex( $expected_sidebars, $new_next_theme_sidebars );
}

/**
* Ensures null sidebar values are converted to empty arrays in retrieve_widgets().
*
* @covers ::retrieve_widgets
* @ticket 57469
*/
public function test_retrieve_widgets_converts_null_sidebar_to_empty_array() {
global $sidebars_widgets;
wp_widgets_init();
$this->register_sidebars( array( 'primary', 'secondary', 'wp_inactive_widgets' ) );

$sidebars_widgets = array(
'primary' => null,
'secondary' => array( 'text-1' ),
'extra_sidebar' => array( 'unregistered_widget-1' ),
'wp_inactive_widgets' => array(),
);

$result = retrieve_widgets( true );
$this->assertArrayHasKey( 'primary', $result );
$this->assertIsArray( $result['primary'], 'Primary sidebar should be an array after normalization.' );
$this->assertEmpty( $result['primary'], 'Primary sidebar should be an empty array when original value was null.' );
$this->assertArrayNotHasKey( 'extra_sidebar', $result, 'Unregistered sidebar should be removed.' );
}

/**
* Ensures wp_map_sidebars_widgets() normalizes null sidebar widget arrays.
*
* @covers ::wp_map_sidebars_widgets
* @ticket 57469
*/
public function test_wp_map_sidebars_widgets_converts_null_sidebar_to_empty_array() {
$this->register_sidebars( array( 'primary', 'wp_inactive_widgets' ) );
// Theme data containing a null so the normalization loop runs.
set_theme_mod(
'sidebars_widgets',
array(
'time' => time(),
'data' => array(
'primary' => null,
),
)
);

$prev_theme_sidebars = array(
'primary' => null,
'wp_inactive_widgets' => array(),
);

$new_sidebars = wp_map_sidebars_widgets( $prev_theme_sidebars );
$this->assertArrayHasKey( 'primary', $new_sidebars );
$this->assertIsArray( $new_sidebars['primary'], 'Primary sidebar should be an array after normalization.' );
$this->assertEmpty( $new_sidebars['primary'], 'Primary sidebar should be an empty array when original value was null.' );
}
}
Loading