WordPress.org

Make WordPress Core

Opened 10 years ago

Closed 10 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:

Description

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 10 years ago.
functions.php with register_sidebar calls for testing
19274.diff (374 bytes) - added by batmoo 10 years ago.
19274-alternate.diff (625 bytes) - added by batmoo 10 years ago.

Download all attachments as: .zip

Change History (11)

@batmoo
10 years ago

functions.php with register_sidebar calls for testing

@batmoo
10 years ago

#1 @batmoo
10 years ago

  • Keywords has-patch added

Patch attached.

#2 follow-up: @batmoo
10 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
10 years ago

  • Cc lancewillett added

See also #19092.

#4 @ocean90
10 years ago

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

#5 @azaozz
10 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
10 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
10 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#8 @azaozz
10 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.