Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#42603 closed defect (bug) (fixed)

Widgets Warning after activating theme and on dashboard widgets page

Reported by: ionvv's profile ionvv Owned by: obenland's profile obenland
Milestone: 4.9.2 Priority: normal
Severity: normal Version: 4.9
Component: Widgets Keywords: has-patch
Focuses: administration Cc:

Description

This happens only on websites that were updated from 4.8.x to 4.9
This happens to custom widget areas (in my case, sidebar-3, sidebar-4, sidebar-5)

Recorded issue here: https://www.youtube.com/watch?v=Dg3JPGWaCwU

To replicate:

  1. Install WP 4.8.x
  2. Install a theme that has custom widget areas
  3. Activate theme, don't add any widgets
  4. Update WP to 4.9
  5. Activate another theme
  6. Activate again the theme that has custom widgets - the warning will be displayed
  7. Go to dashboard, widgets. Warnings will be displayed -> https://i.imgur.com/v7qU1M0.png

This happens because of the "_wp_remove_unregistered_widgets" function added to 4.9 version. It returns null value instead of empty array ( https://i.imgur.com/dfNhQKS.png last 3 items in array ).

To fix the issue, the function "_wp_remove_unregistered_widgets" should be changed from

foreach ( $sidebars_widgets as $sidebar => $widgets ) {
    if ( is_array( $widgets ) ) {
	$sidebars_widgets[ $sidebar ] = array_intersect( $widgets, $whitelist );
    }
}

to

foreach ( $sidebars_widgets as $sidebar => $widgets ) {
    if ( is_array( $widgets ) ) {
	$sidebars_widgets[ $sidebar ] = array_intersect( $widgets, $whitelist );
    } else {
	$sidebars_widgets[ $sidebar ] = array();
    }
}

Attachments (2)

42603.diff (674 bytes) - added by chetan200891 6 years ago.
I've attached the patch 42603.diff.
42603.2.diff (1.5 KB) - added by obenland 6 years ago.

Download all attachments as: .zip

Change History (13)

#1 @westonruter
6 years ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to 4.9.1
  • Owner set to obenland
  • Status changed from new to assigned

I believe this is a follow up on #39693. @obenland would you review?

Last edited 6 years ago by westonruter (previous) (diff)

@chetan200891
6 years ago

I've attached the patch 42603.diff.

#2 @chetan200891
6 years ago

  • Keywords has-patch added; needs-patch removed

#3 follow-up: @obenland
6 years ago

@ionvv Thanks for the report! I tried to follow the steps you outlined above but couldn't reproduce it. Can anyone else?
Are the themes you're using open source? I couldn't find either jora or spiffy in the theme directory.

This ticket was mentioned in Slack in #core-customize by obenland. View the logs.


6 years ago

#5 in reply to: ↑ 3 @ionvv
6 years ago

Replying to obenland:

@ionvv Thanks for the report! I tried to follow the steps you outlined above but couldn't reproduce it. Can anyone else?
Are the themes you're using open source? I couldn't find either jora or spiffy in the theme directory.

@obenland please send me an email at ion [at] pixolette [dot] com and I’ll send you the themes

Last edited 6 years ago by ionvv (previous) (diff)

@obenland
6 years ago

#6 @obenland
6 years ago

I was able to recreate the issue with the themes shown in the youtube video. For the bug to occur, the following set of conditions have to apply:

  1. A theme had to be switched to from a theme that had fewer registered sidebars.
  2. It had to be active during the upgrade to 4.9.
  3. It has to be switched away from.
  4. It has to be activated again, with the previously active theme having at least one registered sidebar that does not have the same slug as a sidebar in the theme being switched to.

Congrats @ionvv for finding a bug that I'd consider pretty rare and hard to find :)
Attached test—when run without the fix—will surface the bug.

Fixing it is thankfully more straight forward than reproducing it, and we even get a small performance improvement out of this. By removing empty sidebars prior to mapping, we fix the bug and also cut down on the amount of mapping iterations we need to do.

@ionvv & @chetan200891 would you mind double-checking 42603.2.diff?

#7 @johnbillion
6 years ago

  • Milestone changed from 4.9.1 to 4.9.2

#8 @chetan200891
6 years ago

@obenland I was able to recreate issue. What I did.

Installed WordPress 4.8.1
Activate Twentysixteen Theme
Updated to WordPress 4.9.2
Activate TwentySeventeen Theme
I got same warnings on Widgets page mentioned by @ionvv

What I did again.

Installed WordPress 4.8.1
Activate Twentysixteen Theme
Updated to WordPress 4.9.2
Made changes to wp-includes/widgets.php as per patch 42603.2.diff
Activate TwentySeventeen Theme
No more warning shown on Widgets page. :)

So for me 42603.2.diff works!

#9 @edge22
6 years ago

This has been reported by a handful of our theme users as well. Some are just activating a child theme and getting the warnings.

#10 @obenland
6 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 42362:

Widgets: Don't try mapping empty sidebars.

Fixes a bug where the mapping logic would try mapping empty sidebars, resulting in PHP warnings.

Props ionvv, chetan200891 for initial patch.
Fixes #42603.

#11 @obenland
6 years ago

In 42363:

Widgets: Don't try mapping empty sidebars.

Fixes a bug where the mapping logic would try mapping empty sidebars, resulting in PHP warnings.

Props ionvv, chetan200891 for initial patch.
See #42603.

Merges [42362] to the 4.9 branch.

Note: See TracTickets for help on using tickets.