Customize: Heartbeat doesn't refresh changeset lock when branching is enabled

[dlh]

Heartbeat requests from the Customizer will create a WP_Customize_Manager instance without a changeset_uuid. However, when changeset branching is active and when the manager is instantiated without a UUID, WP_Customize_Manager::establish_loaded_changeset() will generate a new UUID.

Without the UUID from the client, WP_Customize_Manager::check_changeset_lock_with_heartbeat() won't have a post ID whose lock should be refreshed.

The attached patch would attempt to address this by including the current UUID with the heartbeat request. The patch then has check_changeset_lock_with_heartbeat() use the heartbeat UUID to find the changeset post ID before falling back to WP_Customize_Manager::changeset_post_id().

[westonruter]

Good catch!

[dd32]

Bumping to 4.9.3 due to 4.9.2s release

[westonruter]

git diff -w --color-words

[westonruter]

Just wanted to share 42658.diff.png​ as a demonstration of how useful git diff -w --color-words is when looking at a patch like 42658.diff​ to see that it's not as large as it would at first seem.

[westonruter]

@dlh one thing that I think needs to be improved with 42658.diff​: it's checking if current_user_can( 'customize' ) but it's not checking whether the current user can actually edit the specific $changeset_post_id. In other words, I think it needs to include a condition like: current_user_can( get_post_type_object( 'customize_changeset' )->cap->edit_post, $changeset_post_id ).

It's too bad that merely data.changeset_uuid = api.settings.changeset.uuid doesn't cause the Customizer to initialize with the UUID supplied there. The reason is that _wp_customize_include() is looking for a customize_changeset_uuid or changeset_uuid input var, but Heartbeat sends it as data[changeset_uuid]. The only way to get around that would be to opt to inject the customize_changeset_uuid param by some other means, such as by jQuery.ajaxPrefilter, which we are currently doing to add the customize_preview_nonce: ​https://github.com/WordPress/wordpress-develop/blob/026cd70/src/wp-admin/js/customize-controls.js#L7784-L7792

I don't know if that's better than just improving the cap checking for locking as it exists in core today.

[westonruter]

@dlh This needs to be included in 4.9.3 now because, if I'm not mistaken, the change in #42975 will mean the lock will get lost when a user is sitting in the Customizer but doesn't make any changes. If they make a change, the autosave will cause the lock to be regained, but heartbeat won't be maintaining their lock otherwise. Right?

[dlh]

42658.2.diff​ makes the current_user_can() check more specific as suggested in comment:8.

The only way to get around that would be to opt to inject the customize_changeset_uuid param by some other means...I don't know if that's better than just improving the cap checking for locking as it exists in core today.

I'm not sure I entirely follow. I agree that it would be nice if the Customizer automatically initialized with the UUID from the heartbeat request. But, as far as I can tell, that wouldn't cause the Customizer to start running any new checks for current_user_can( get_post_type_object( 'customize_changeset' )->cap->edit_post, $changeset_post_id ). So, adding that check in check_changeset_lock_with_heartbeat() seems useful either way.

the change in #42975 will mean the lock will get lost when a user is sitting in the Customizer but doesn't make any changes. If they make a change, the autosave will cause the lock to be regained, but heartbeat won't be maintaining their lock otherwise. Right?

It's true that \WP_Customize_Manager::establish_loaded_changeset() will no longer call set_changeset_lock() during heartbeat requests because $pagenow is admin-ajax.php: ​https://github.com/WordPress/wordpress-develop/blob/026cd70e25b90f04c27a21eaf634048945848398/src/wp-includes/class-wp-customize-manager.php#L643.

However, for users in "linear" changeset mode, establish_loaded_changeset() should still be able to find the current changeset post ID, and \WP_Customize_Manager::check_changeset_lock_with_heartbeat() can use that post ID to proceed with its logic. Users with changeset branching enabled are still in the same boat they're in now. So I'm not sure #42975 changes the situation in this ticket.

Of course, let me know if I've misunderstood what you meant on either point.

[westonruter]

In 42612:

Customize: Ensure heartbeat keeps changeset locked when in branching mode.

Props dlh.
See #42024.
Fixes #42658.

[SergeyBiryukov]

In 42623:

Customize: Ensure heartbeat keeps changeset locked when in branching mode.

Props dlh.
See #42024.
Merges [42612] to the 4.9 branch.
Fixes #42658.