#42457 closed defect (bug) (fixed)
Scheduled changeset won't publish if Customizer is opened first after missing schedule
Reported by: | LittleBigThing | Owned by: | westonruter |
---|---|---|---|
Milestone: | 4.9 | Priority: | normal |
Severity: | normal | Version: | 4.9 |
Component: | Customize | Keywords: | has-patch commit dev-reviewed |
Focuses: | Cc: |
Description
There seems to be a bug with publishing scheduled changes for some Customizer settings if the Customizer is opened first after missing the scheduled date.
If I understand correctly from the blog post introducing the changes in WP 4.9, by default, scheduled changesets are autoloaded when the Customizer is loaded (/wp-admin/customize.php). This works correctly. However, when the site has not been visited after the scheduled date is passed and the Customizer is then loaded, the message appears that "Your scheduled changes just published". If then I click through to the Customizer or visit the site the changes are not present for some Customizer settings. This was the case for the site title, description, (text) widgets and the home page setting (blog or page).
However, it seemed to work correctly for changes for the menus, colors and the header image. Sorry for not testing "everything".
When I looked in the posts table in the database, the changes seemed properly cued as a future changeset before opening the Customizer (post_status = 'future'). After opening the Customizer, the changeset had status 'trash'. (Note that I am not aware of the correct structure for the post_content of changesets.)
To reproduce:
- go to the Customizer
- change a setting (e.g. site title)
- schedule the change for a (near) future date (just a couple of minutes)
- close the Customizer
- let scheduled time pass
- open the Customizer (you'll receive the message that scheduled changes are published)
- go further to the Customizer or check the site directly for the changes
I tested locally with WP 4.9-RC2 using the themes Twenty Fifteen, Sixteen en Seventeen as administrator.
Sorry if I'm missing something and this is no bug...
Attachments (3)
Change History (17)
#1
@
7 years ago
- Keywords has-patch needs-testing added
- Milestone changed from Awaiting Review to 4.9
#2
@
7 years ago
I tested the above settings (incl. options, but also theme_mods) and it works fine now.
I did some testing without define( 'DISABLE_WP_CRON', true )
, too, OK as well.
#4
@
7 years ago
If ALTERNATE_WP_CRON
is enabled, then spawn_cron()
will redirect the user back to the Customizer. However, WP_Customize_Manager
calls remove_action( 'init', 'wp_cron' )
.
In my testing, my changes were not published in this scenario, but maybe I didn't replicate the scenario correctly? ALTERNATE_WP_CRON
might not be too common, either.
#5
@
7 years ago
- Keywords needs-patch added; has-patch dev-feedback removed
@dlh Thanks very much for testing that. I forgot about ALTERNATE_WP_CRON
.
Regardless, if the loopback request fails (which is a common cause for scheduling problems) then spawn_cron()
won't do anything either.
The simplest thing I can think of is to open an Ajax request to invoke the customize save method like when hitting Publish in the Customizer. Can you think of a better way?
#6
@
7 years ago
I can't think of anything more straightforward than an Ajax request so far, especially at this point in the 4.9 cycle.
The only alternative I can think of, although I'm not really sold on it, is to take advantage of the fact that the changeset post ID is available before each setting's preview()
method is called here: https://github.com/WordPress/wordpress-develop/blob/9f77ec13ffcf1ab14c8699fdb68e728709eb6bb5/src/wp-includes/class-wp-customize-manager.php#L923.
Because the manager knows the changeset ID, it could perform the same "missed schedule" logic that's in customize.php
and skip calling preview()
if the changeset has missed the schedule, or even call wp_publish_post()
itself.
But, apart from being more code than the Ajax request, that approach seems like it would conflict with the idea behind the settings_previewed
argument to begin with.
Thinking out loud a bit, maybe in the future WP_Customize_Manager
(or WP_Customize_Changeset
) could have a "missed schedule" property that _wp_customize_include()
could use to switch WP_Customize_Manager::$settings_previewed
as needed.
#7
@
7 years ago
- Keywords has-patch needs-testing added; needs-patch removed
@dlh @LittleBigThing Please test 42457.1.diff. It uses the Ajax approach. It's also not perfect, as a user could be coming to the Customizer who lacks the capability to publish the changeset. But for 4.9 I think the liklihood of an under-permissioned user visiting the Customizer while also a schedule has been missed to be quite rare, and something that we can look into further after 4.9.
#8
@
7 years ago
Tested it. Works OK.
About the problem with an underpermissioned user: isn't that a theoretical problem? The planned publish date is already passed here. So if anyone (even without permissions) visited the site, the changes would have been published already.
But maybe I am missing some (future) complexity in how changesets are implemented (like in branched mode).
#9
@
7 years ago
- Keywords dev-feedback added; needs-testing removed
@LittleBigThing yes, it is somewhat of a theoretical problem. In fact, a user visiting the site usually causes WP Cron to spawn. It just seems that we should make sure that it is actually getting published when we tell the user that the changes have been published. For example, if DISABLE_WP_CRON
is enabled and system cron pings WordPress every minute, then the changeset may not get published for another 59 seconds while waiting for WP Cron to get spawned by system cron. So that's why I think it's important to proactively publish the changes immediately. Another reason for this is that if there is a delay, then if the user clicks “Customize New Changes” then there would be a potential for them to encounter the same missed schedule message again.
#11
@
7 years ago
- Keywords dev-reviewed added; dev-feedback removed
42457.1.diff works as expected and looks good for commit. Creative. +1 :)
I took a stab at tweaking the docs a bit in 42457.diff. No changes to code were made.
#12
@
7 years ago
42457.diff works as expected for me, too.
I can reproduce this. For anyone who wants to test, it is particularly helpful to add
define( 'DISABLE_WP_CRON', true );
to yourwp-config.php
.The problem is that
wp_publish_post( $changeset_post->ID )
is getting incustomize.php
called after the settings have already been previewed (after the filters have been added to apply the changeset's values to WordPress). In the case of options particularly, calls toupdate_option()
will short-circuit if the value being updated is the same value that it finds when doing aget_option()
call. Since the filters would be supplying the changeset's previewed value at that point, then none of the options would get saved becauseupdate_option()
would short-circuit for each one. This problem is what we introduced thesettings_previewed
arg for in #39221. However, incustomize.php
this argument is not set tofalse
like it is in_wp_customize_publish_changeset()
which is normally called when WP Cron publishes a scheduled changeset.So the fix here in 42457.0.diff is simply to replace the call to
wp_publish_post()
with a call tospawn_cron()
. This will result in a loopback request towp-cron.php
being opened up, and in that requestWP_Customize_Manager
will not be instantiated withsettings_previewed=true
, allowing option settings to be saved successfully.I noticed that all of the settings you described failing to save were in fact of the
option
type. So that confirms my suspicion.@LittleBigThing Please test the patch.
This may be the same issue as reported at https://wordpress.org/support/topic/beta-v4-9-twenty-seventeen-does-not-make-any-scheduled-changes-to-customizer/