Make WordPress Core

Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#39103 closed defect (bug) (fixed)

Customize: menus aren't deleted

Reported by: celloexpressions's profile celloexpressions Owned by: westonruter's profile 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 7 years ago.
39103.1.dff (4.0 KB) - added by westonruter 7 years ago.

Download all attachments as: .zip

Change History (12)

@westonruter
7 years ago

#1 @westonruter
7 years 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 7 years ago by westonruter (previous) (diff)

@westonruter
7 years ago

#2 @westonruter
7 years 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
7 years ago

  • Keywords needs-testing added

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


7 years ago

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

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

Reopening for 4.7.1

#8 @westonruter
7 years ago

(See #39221 for follow-up.)

#9 @dd32
7 years 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
7 years ago

#39303 was marked as a duplicate.

Note: See TracTickets for help on using tickets.