Customize: Prevent setting previewing logic when settings will be saved

[westonruter]

When a setting is previewed this may very well cause a save to fail because WP may detect that the value is already set in the database (as in the case of options) and thus no-op. Or in the case of deleting a nav menu in the customizer, the preview filter causes the nav menu object to fail to be returned if the preview filter is applied (see #39103).

The way this has been accounted for in core up until now is to prevent calling WP_Customize_Setting::preview() when a customize_save admin ajax request is being done. This is not ideal, however, as it is too closely bound to the existing customize.php app and is not generic enough for other applications which may be built for live preview, such as REST API (#38900), although to be sure it is unlikely that the customizer preview logic would be booted when manipulating a changeset endpoint.

See https://core.trac.wordpress.org/attachment/ticket/39103/39103.1.dff for one possible approach, where a non-previewing WP_Customize_Manager instance could be made.

[celloexpressions]

This can probably be considered a bug, as its manifestations will be bugs and the underlying issue here should be fixed.

[westonruter]

I think that this actually won't be necessary per:

it is unlikely that the customizer preview logic would be booted when manipulating a changeset endpoint

@stubgo @dlh thoughts re: ​https://github.com/WP-API/wp-api-customize-endpoints

[stubgo]

@westonruter When manipulating the changeset via an endpoint the WP_Customize_Manager is initialized per each changeset and also the WP_Customize_Setting::preview() is called for relevant settings. It seems that when a changeset is created for deleting a menu and then this changeset is published, the menu is not actually deleted.

Seems that it does need preventing previewing logic. (Thanks for tagging me here.)

[westonruter]

@stubgo excellent. Great feedback. I wonder what the best solution would be for this. Maybe there should be a skip_setting_preview argument that is added to the the WP_Customize_Manager constructor args that when supplied would be checked for whether or not preview should be called on each setting instead of hackily looking at the current Ajax action? The argument could default to false, but then in _wp_customize_include() it could then look at supplying the skip_setting_preview arg if doing the ajax action or something. Then in the changeset endpoint, where you are directly instantiating WP_Customize_Manager you would then explicitly pass in skip_setting_preview=>true. Seems like that would work but I'm not sure it is the most elegant. Thoughts?

Milestoning for 4.8.1 to match #38900.

[stubgo]

@westonruter Sounds like adding an additional argument is reasonable. Maybe it would also make sense to create a method WP_Customize_Manager::maybe_preview_settings() (or something similar) and instead of making a check separately in WP_Customize_Nav_Menus and WP_Customize_Widgets etc. just call $this->manager->maybe_preview_settings( $settings )?

Couldn't think of a better alternative to adding an additional argument to WP_Customize_Manager for skipping the preview.

[westonruter]

Yeah, either that or perhaps it would be easier to add a method like WP_Customize_Manager::should_preview_settings() which could be checked before a component calls WP_Customize_Setting::preview().

[stubgo]

Note: starting working on the issue to implement the discussed approach.

[stubgo]

@westonruter Created the initial PR:
​https://github.com/xwp/wordpress-develop/pull/231

Github diff:
​https://github.com/xwp/wordpress-develop/pull/231.diff

Tested that it works in Customizer and for the customize changeset endpoint as well.

[stubgo]

Updated with a fix.

[utkarshpatel]

Tested works fine.

[westonruter]

Anne Louise Currie found a defect in core related to this, specifically in publishing a changeset from outside the Customizer, such as via the edit post screen as provided by the Customize Snapshots plugin. In particular, publishing changes to options will fail due to the preview filters being added.

I have amended the PR with a fix: ​https://github.com/xwp/wordpress-develop/pull/231/commits/3e4dea479df35a8e3b046a2509566c4a95657a91

[westonruter]

​https://github.com/xwp/wordpress-develop/pull/231

[westonruter]

39221.2.diff​ incorporates unit test from ​https://github.com/xwp/wp-customize-snapshots/pull/137

Any last feedback/tweaks before I commit?

[stubgo]

Weston: no additional comments from me, thank you for the fix.

[westonruter]

In 41205:

Customize: Introduce settings_previewed arg and getter on WP_Customize_Manager which controls whether WP_Customize_Setting::preview() should be called on settings.

The settings_previewed property eliminates the need for the Customizer components from having to look at global doing_ajax state. This is in particular needed when saving settings, as some settings will short-circuit the update operation if they detect no changes are introduced. This is also needed for low-level integrations with the Customizer, such as in REST API endpoints under development.

Props stubgo, westonruter, utkarshpatel for testing.
See #38900.
Fixes #39221.

[westonruter]

In 42138:

Customize: Fix reliability of just-in-time publishing for changesets that miss their schedule when visiting customize.php.

When just doing wp_publish_post() for the changeset from customize.php, any option-based settings will fail to get saved because WP_Customize_Manager would have already been loaded with settings_previewed, resulting in update_option() calls being short-circuited. So an admin-ajax request to customize_save is used to work around this.

Props westonruter, jeremyfelt, dlh for testing, LittleBigThing for testing.
Amends [41626].
See #28721, #39221.
Fixes #42457 for trunk.

[westonruter]

In 42139:

Customize: Fix reliability of just-in-time publishing for changesets that miss their schedule when visiting customize.php.

When just doing wp_publish_post() for the changeset from customize.php, any option-based settings will fail to get saved because WP_Customize_Manager would have already been loaded with settings_previewed, resulting in update_option() calls being short-circuited. So an admin-ajax request to customize_save is used to work around this.

Props westonruter, jeremyfelt, dlh for testing, LittleBigThing for testing.
Amends [41626].
See #28721, #39221.
Fixes #42457 for 4.9.