WordPress.org

Make WordPress Core

Opened 11 months ago

Closed 10 months ago

Last modified 10 months ago

#39103 closed defect (bug) (fixed)

Customize: menus aren't deleted

Reported by: celloexpressions Owned by: westonruter
Milestone: 4.7.1 Priority: normal
Severity: normal Version: 4.7
Component: Customize Keywords: has-patch commit fixed-major
Focuses: Cc:

Description

In trunk, deleting a menu in the customizer appears to be successful. However, the next time you open or reload the customizer, any deleted menus reappear. This may cause larger issues with conflicting menu names, for example, because the menu seems to be deleted but actually isn't.

Attachments (2)

39103.0.dff (1.3 KB) - added by westonruter 11 months ago.
39103.1.dff (4.0 KB) - added by westonruter 11 months ago.

Download all attachments as: .zip

Change History (12)

@westonruter
11 months ago

#1 @westonruter
11 months ago

  • Keywords has-patch added; needs-patch removed

The reason for the failure to delete a menu is that the preview filters are being added for the nav menu settings even during a customize_save request. The result is that when attempting to call wp_delete_nav_menu() for the deleted nav menu ID, it will in turn call wp_get_nav_menu_object() which has its return value modified by the wp_get_nav_menu_object filter, which the setting's preview uses to supply false. When this function returns false, then the wp_delete_nav_menu() logic is short-circuited.

This was accounted for in widgets in the \WP_Customize_Widgets::customize_register() be checking to see if the customize_save ajax request is being made.

But, this is short sighted nevertheless. It will fail to work when implementing the changesets REST API endpoint (#38900). There should be a way to initialize WP_Customize_Manager without calling preview on all of the settings.

Aside: We'll need to refactor these to not use the customize_save Admin ajax as the signal but rather to check if doing_action( 'publish_customize_changeset' )

Last edited 11 months ago by westonruter (previous) (diff)

@westonruter
11 months ago

#2 @westonruter
11 months ago

39103.1.dff introduces a new saving property on WP_Customize_Manager and when it is set then the manager and any components should not call preview on the settings.

The JS change fixes a related issue where emptying a nav menu location was assigning it to NaN as opposed to an empty value.

#3 @westonruter
11 months ago

  • Keywords needs-testing added

This ticket was mentioned in Slack in #core-customize by celloexpressions. View the logs.


10 months ago

#5 @celloexpressions
10 months ago

  • Keywords commit added; needs-testing removed

Confirmed that 39103.0.dff works. Let's commit that to trunk and the 4.7 branch, then continue working on 39103.1.dff for a broader approach in a future release.

#6 @westonruter
10 months ago

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

In 39558:

Customize: Fix inability to delete nav menus by preventing preview filters from being added during customize_save admin ajax request.

Also prevent setting nav_menu_locations[...] values to NaN which gets sent as null.

Amends [38810].
See #30937.
Fixes #39103.

#7 @westonruter
10 months ago

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

Reopening for 4.7.1

#8 @westonruter
10 months ago

(See #39221 for follow-up.)

#9 @dd32
10 months ago

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

In 39570:

Customize: Fix inability to delete nav menus by preventing preview filters from being added during customize_save admin ajax request.

Also prevent setting nav_menu_locations[...] values to NaN which gets sent as null.

Props westonruter.
See #30937, [38810].
Merges [39558] to the 4.7 branch.
Fixes #39103.

#10 @westonruter
10 months ago

#39303 was marked as a duplicate.

Note: See TracTickets for help on using tickets.