WordPress.org
Get WordPress

Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#53612 closed defect (bug) (fixed)

Widgets not loaded after deleting multiple widgets at once

Reported by: walbo's profile walbo Owned by: timothyblynjacobs's profile TimothyBlynJacobs
Milestone: 5.8 Priority: normal
Severity: normal Version: 5.8
Component: Widgets Keywords: has-patch has-unit-tests fixed-major commit dev-reviewed
Focuses: rest-api Cc:

Description

When deleting multiple widgets at once, no widgets is loaded in the editor. The widgets I didn't delete do show in the customizer widget sceen and frontend.

It doesn't happend if you delete the last widgets, only if you delete from the beginning or in the middle of the array.

Steps to reproduce:

  1. Create a few widgets in an area and save. (or from a fresh dev install you already have them)
  2. Select the 2 first widgets and delete them. Then save.
  3. Reload the widget editor and no widgets in loaded.

Attachments (1)

delete_multiple_widgets.gif​ (447.5 KB) - added by walbo 3 years ago.

Download all attachments as: .zip

Change History (16)

@walbo
3 years ago

#1 @walbo
3 years ago

  • Keywords needs-patch added

#2 @walbo
3 years ago

  • Focuses rest-api added

Before deleting the sidebars rest-api return an array:

widgets: ["block-2", "block-3", "block-4", "block-5", "block-6"]

After save its a object:

widgets: {2: "block-4", 3: "block-5", 4: "block-6"}

#3 @desrosj
3 years ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to 5.8

#4 follow-up: @TimothyBlynJacobs
3 years ago

Looks like we need an array_values call when preparing the widgets property.

#5 @noisysocks
3 years ago

  • Owner set to TimothyBlynJacobs
  • Status changed from new to assigned

This ticket was mentioned in ​PR #1480 on ​WordPress/wordpress-develop by ​walbo.
3 years ago #6

  • Keywords has-patch added; needs-patch removed

Trac ticket: https://core.trac.wordpress.org/ticket/53612

#7 in reply to: ↑ 4 @walbo
3 years ago

Replying to TimothyBlynJacobs:

Looks like we need an array_values call when preparing the widgets property.

Yes. Adding array_values brings them back.

I can't find a good explanation why it only happens when deleting multiple widgets and not just one.

#8 @desrosj
3 years ago

  • Keywords needs-unit-tests added

Can we get a unit test or two in place for this as well?

#9 @walbo
3 years ago

  • Keywords has-unit-tests added; needs-unit-tests removed

Added a unit test to the PR.

​TimothyBJacobs commented on ​PR #1480:
3 years ago #10

This looks great, thanks for working on it @walbo! Will commit shortly.

#11 @TimothyBlynJacobs
3 years ago

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

In 51377:

REST API: Ensure a sidebar's widgets property is a list.

When a widget is removed from a sidebar, if it was removed from the middle of the list, the widgets property would become an object with numeric keys.

The sidebars controller now forces the widgets property to be a list.

Props walbo.
Fixes #53612.

#12 @TimothyBlynJacobs
3 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for backport.

#14 @desrosj
3 years ago

  • Keywords commit dev-reviewed added; needs-testing removed

[51377] looks good to backport.

#15 @desrosj
3 years ago

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

In 51385:

REST API: Ensure a sidebar's widgets property is a list.

When a widget is removed from a sidebar, if it was removed from the middle of the list, the widgets property would become an object with numeric keys.

The sidebars controller now forces the widgets property to be a list.

Props walbo, timothyblynjacobs.
Merges [51377] to the 5.8 branch.
Fixes #53612.

Note: See TracTickets for help on using tickets.