Inactive sidebar box can be on the wrong place in one situation

[ocean90]

• Clean install.
• Twenty Eleven as default theme.
• Change sidebar content: ​http://cl.ly/BQBN
• Switch to another theme (in my case Toolbox). Screenshot of widget screen now: ​http://cl.ly/BOn9
• Now open functions.php from Twenty Eleven and remove the last sidebar ('id' => 'sidebar-5',), save.
• After that switch back to Twenty Eleven and you will get this: ​http://cl.ly/BPEZ

Scenario could be, that Toolbox is a maintenance theme so that you can change the old theme.

Related: #17979

[azaozz]

Looks like the last sidebar has the wrong name after switching back to Twenty Eleven. It is in the right place and has the right widget(s). Tweaking the theme's functions.php would do that.

[ocean90]

Did you try to reprodouce it?

The last sidebar shouldn't be a sidebar anymore, because I have removed it.

It is in the right place and has the right widget(s).

Right widgets is right, but the place is IMO wrong, should be on the left side, see second screenshot above.

[SergeyBiryukov]

Replying to azaozz:

Looks like the last sidebar has the wrong name after switching back to Twenty Eleven. It is in the right place and has the right widget(s).

If it's not registered by the theme any more, shouldn't it be considered orphaned? Adding a patch.

[ocean90]

If it's not registered by the theme any more, shouldn't it be considered orphaned? Adding a patch.

Right, this is my expected behaviour. Patch works for me.

[azaozz]

We save the widgets for the previous theme and then restore them on switching back to it. This seems to be a side effect: we trust that the theme matches the saved data.

Seems like a very rare case when the number of the theme's registered sidebars would change before switching back. In any case, it's too late to be able to test the patch now.

[ocean90]

nacin: So I just switched themes and found like eight empty "Intactive sidebars (from previous them"
nacin: That is terrible.
nacin: The text is cut off no matter what I scale it to.
nacin: And the theme only has like two sidebars.

[nacin]

I have a feeling that when you introduce child themes into the mix, things get messy quickly.

I made a theme change on a site in the wild, and suddenly I had a bunch of inactive sidebar boxes, but all of them but one of them were empty.

Also, the inactive sidebar description needs some work, it doesn't read right, and the title doesn't fit, so all you see is "Inactive Sidebar (from previous them".

[dougwrites]

Is this a good place to ask when Inactive Sidebar boxes disappear for a theme? I'm assuming they go away for the first theme upon activation of a third theme.

Asking because the help tab for Widgets is still out (one of many in #19020) and needs more work.

[ocean90]

when Inactive Sidebar boxes disappear for a theme?

When you remove the widgets out of the sidebar.

[dougwrites]

ocean90: would those Inactive Sidebar boxes otherwise stick around permanently and/or through many theme changes?

[azaozz]

Replying to nacin:

I have a feeling that when you introduce child themes into the mix, things get messy quickly.

Agreed, changing to another theme and then back to the previous theme but this time with a child theme (that changes the number of sidebars) would make the saved data bad. Perhaps we should invalidate the data in this case and threat the old theme + child theme as a new/different theme.

I made a theme change on a site in the wild, and suddenly I had a bunch of inactive sidebar boxes, but all of them but one of them were empty.

This sounds like another bug. As far as I see it there shouldn't be any empty inactive sidebars at all.

Also, the inactive sidebar description needs some work, it doesn't read right, and the title doesn't fit, so all you see is "Inactive Sidebar (from previous them".

Maybe make the "(from previous theme)" a title/tooltip on "Inactive Sidebar"? We need to shorten it.

[lancewillett]

Possibly related: #19274.

[azaozz]

In [19332]:

When restoring sidebars after switching back to an old theme, make sure the saved data matches the theme's current sidebars, fixes #19092

[SergeyBiryukov]

[19332] breaks the sidebars mapping feature. When switching from Twenty Eleven to Twenty Ten, all widgets end up as inactive.

Also causes a notice:

NOTICE: wp-includes/widgets.php:1244 - Undefined index: wp_inactive_widgets

Wouldn't 19092.patch​ be the proper solution here?

The wp_inactive_widgets and orphaned_widgets check is based on the one from wp-admin/widgets.php:
​http://core.trac.wordpress.org/browser/trunk/wp-admin/widgets.php#L367

[SergeyBiryukov]

Refreshed the patch.

[lancewillett]

Replying to SergeyBiryukov:

Refreshed the patch.

Refreshed patch fixes the PHP notice and tests out. I'd suggest going with it.

[lancewillett]

Replying to lancewillett:

Replying to SergeyBiryukov:

Refreshed the patch.

Refreshed patch fixes the PHP notice and tests out. I'd suggest going with it.

See also #19291. To capture lost widgets to orphans.

Relies on this patch being committed.

[azaozz]

Replying to SergeyBiryukov:

[19332] breaks the sidebars mapping feature. When switching from Twenty Eleven to Twenty Ten, all widgets end up as inactive.

Hmm, don't see that happening. All it does is check whether all currently registered sidebars match the stored data for the theme and ignores any extra data.

Also causes a notice:

NOTICE: wp-includes/widgets.php:1244 - Undefined index: wp_inactive_widgets

Seems the notice comes from the fact that 'wp_inactive_widgets' hasn't been registered yet. Can add that by hand to $sidebars. Strangely didn't see that notice while testing.

Wouldn't 19092.patch​ be the proper solution here?

19092.2.patch threats the saved data as "main" and looks for orphaned widgets in it. Not sure that's the proper behavior. If the saved data for a theme doesn't match the current theme's setup, think we should be discarding it, not appending to orphaned.

User case:

• Twenty Ten with 5 widgets in each sidebar,
• user installs theme X with only 4 sidebars leaving 2 orphaned sidebars,
• after some time user modifies Twenty Ten and enables only 2 sidebars,
• when activating the modified Twenty Ten, user ends up with 2 orphaned sidebars from theme X plus 4 orphaned sidebars from the saved data for Twenty Ten, where two of the sidebars are exactly the same as the previous orphaned sidebars.

[lancewillett]

Replying to azaozz:

Also causes a notice:

NOTICE: wp-includes/widgets.php:1244 - Undefined index: wp_inactive_widgets

Seems the notice comes from the fact that 'wp_inactive_widgets' hasn't been registered yet. Can add that by hand to $sidebars. Strangely didn't see that notice while testing.

I saw the error after your commit in r19332, and the latest patch fixes it.

Wouldn't 19092.patch​ be the proper solution here?

19092.2.patch threats the saved data as "main" and looks for orphaned widgets in it. Not sure that's the proper behavior. If the saved data for a theme doesn't match the current theme's setup, think we should be discarding it, not appending to orphaned.

I disagree. This goes against the point of the Orphaned widgets idea. Nothing should go into Inactive unless it's moved there explicitly.

The consensus in #17979 was that we'd save the data for previous themes indefinitely -- and that data trumps anything new when you switch back.

See notes in ​http://core.trac.wordpress.org/ticket/19291#comment:5.

At that time I'd argued to remove the stored data after a week. But that was overruled by the core team. :)

I think on WP.com we might end up only storing it for a week. If a theme is switched back after that time, we'll just treat it as a new theme, not on old one you're going back to.

[SergeyBiryukov]

Replying to azaozz:

[19332] breaks the sidebars mapping feature. When switching from Twenty Eleven to Twenty Ten, all widgets end up as inactive.

Hmm, don't see that happening. All it does is check whether all currently registered sidebars match the stored data for the theme and ignores any extra data.

Can't reproduce now. I probably forgot to clear theme mods that time. Sorry for the false alarm.

[azaozz]

In [19340]:

Prioritize the saved data over the current data when restoring widget positions, props SergeyBiryukov, see #19092

[azaozz]

Replying to SergeyBiryukov:

No worries :) We have to use the stored data anyways, going with your patch.

[azaozz]

This seems fixed with [19340].