Make WordPress Core

Opened 7 years ago

Closed 7 years ago

#19274 closed defect (bug) (fixed)

Widgets shift when new sidebars added/removed

Reported by: batmoo Owned by: azaozz
Milestone: 3.3 Priority: normal
Severity: normal Version: 3.3
Component: Widgets Keywords: has-patch needs-testing
Focuses: Cc:


When an *empty sidebar* is removed or a *new sidebar* is adding next to an empty sidebar, widgets will shift down one sidebar.

Steps to reproduce:

  1. Add a bunch of sidebars, sidebar-1, sidebar-2, sidebar-3, sidebar-4 (using register_sidebar)
  2. Add text widgets to all of them (title them 1, 2, 3, 4 respectively)
  3. Remove the widget from sidebar-2 (so that it’s completely empty)
  4. Remove sidebar-2 (remove or comment out the register_sidebar call)
  5. The widgets will “shift” (sidebar-3 will be empty, sidebar-4 will have text widget titled “3″)

You can also reproduce the issue by adding a new sidebar-1.5 (instead of removing) at step 4.

It's a bit of an edge-case but is particularly a problem when you have dynamically generated sidebars (based on custom post types and taxonomies, for example) or during theme development. The first use case is becoming more common, for displaying sidebars specific to different single and archive pages.

Attachments (3)

functions.php (339 bytes) - added by batmoo 7 years ago.
functions.php with register_sidebar calls for testing
19274.diff (374 bytes) - added by batmoo 7 years ago.
19274-alternate.diff (625 bytes) - added by batmoo 7 years ago.

Download all attachments as: .zip

Change History (11)

7 years ago

functions.php with register_sidebar calls for testing

7 years ago

#1 @batmoo
7 years ago

  • Keywords has-patch added

Patch attached.

#2 follow-up: @batmoo
7 years ago

Alternate patch also works. Addresses the use of array_shift, which assumes that registered sidebars are the same as the sidebars in options.

#3 @lancewillett
7 years ago

  • Cc lancewillett added

See also #19092.

#4 @ocean90
7 years ago

  • Component changed from General to Widgets
  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to 3.3

#5 @azaozz
7 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In [19334]:

Prevent errors in assigning widgets to sidebars for themes with dynamic sidebars, props batmoo, fixes #19274

#6 in reply to: ↑ 2 @azaozz
7 years ago

Replying to batmoo:

Changed the test for existing sidebar from empty() to !isset() to avoid processing sidebars that are really empty, could you see if that still works as expected.

Alternate patch also works.

We may need to add this patch too. If a user switches themes but doesn't go to the widgets page (as we advise), the sidebars may remain messed up. This case needs more testing.

#7 @nacin
7 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#8 @azaozz
7 years ago

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

Chatted with @batmoo, works as expected now.

Note: See TracTickets for help on using tickets.