WordPress.org

Make WordPress Core

Opened 3 months ago

Closed 3 months ago

#53612 closed defect (bug) (fixed)

Widgets not loaded after deleting multiple widgets at once

Reported by: walbo Owned by: 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 months ago.

Download all attachments as: .zip

Change History (16)

@walbo
3 months ago

#1 @walbo
3 months ago

  • Keywords needs-patch added

#2 @walbo
3 months 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 months ago

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

#4 follow-up: @TimothyBlynJacobs
3 months ago

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

#5 @noisysocks
3 months 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 months ago

  • Keywords has-patch added; needs-patch removed

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

#7 in reply to: ↑ 4 @walbo
3 months 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 months ago

  • Keywords needs-unit-tests added

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

#9 @walbo
3 months ago

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

Added a unit test to the PR.

#10 @prbot
3 months ago

​TimothyBJacobs commented on ​PR #1480:

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

#11 @TimothyBlynJacobs
3 months 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 months ago

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

Reopening for backport.

#14 @desrosj
3 months ago

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

[51377] looks good to backport.

#15 @desrosj
3 months 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.