Make WordPress Core

Opened 10 years ago

Last modified 3 years ago

#31436 assigned enhancement

Handle conflicts in concurrent Customizer sessions

Reported by: westonruter's profile westonruter Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.4
Component: Customize Keywords: needs-patch
Focuses: javascript Cc:

Description (last modified by westonruter)

If two users open the Customizer at the same time and modify the same settings, the user who saves their changes last will win out, and the person who saves first will have their changes lost. (The frequency of the problem was reduced in #29983 since now only dirty settings now get POSTed.) The Customizer needs Heartbeat integration to add the “Post Locking” functionality. We don't need to lock the entire Customizer, however, from concurrent users: we need to add locking for individual settings in the Customizer. When a setting becomes dirty, we need to broadcast via Heartbeat to other users that the setting has been changed and thus any controls for this setting should be marked as "locked", with any changes prevented. This will become increasingly important as more and more settings are added to the Customizer and users go there more often to make changes.

The locking UI could provide a button to copy the other user's change into the other Customizer session, and this could result in the control being editable again, with subsequent changes pushed out to other users as well, who would then also get the corresponding setting automatically updated if it was dirty, but if it was not dirty then it would remain in its clean state but with a locking notification added.

This also should apply when a setting is modified by some means other than the Customizer: if someone is in the Customizer and another user changes a setting via an admin page or via XML-RPC or REST API, then this setting update should also be illustrated in the Customizer to note that the settings are stale and should be refreshed. This refresh could be done inline, without having to reload the entire page.

For the issue of conflicting auto-incremented widget IDs across user sessions, see #32183.

For the previously-reported issue specific for handling conflicts between editing widgets on the widget admin page, see #12722.

For the introduction of concurrency locking for options pages (settings API), see #32202.

Some enhancements for a feature plugin: The Customizer UI would benefit from having a list of users currently in the Customizer appearing somewhere, with a list of the changes each user has made. If someone left their Customizer session open, this list would also allow an administrator to sign the user out, using something like the User Session Control plugin; or the Customizer UI could provide a way to boot a user from the Customizer.

For use of Heartbeat to keep nonces fresh, see #31897.

See also #42191 (Customize: Selectively merge settings from autosave revisions)

Change History (24)

#1 @westonruter
10 years ago

  • Description modified (diff)

#2 @westonruter
10 years ago

  • Owner set to westonruter
  • Status changed from new to accepted

In regards to widgets, I've started hacking on a prototype for this. I'm trying out an implementation where a new post type is used for widget instances, allowing a widget ID to be derived from a post ID created at the time that the widget is added. This also has the benefit of widgets being versioned via post revisions, and also they could be drafted or scheduled. They're also much more scalable since all of the instance data is not crammed into a WP option as a serialized array. (I shudder to think what could happen on a site with Memcached Object Cache where widget instances of a given type grow larger than 1MB when serialized.)

#4 @westonruter
9 years ago

  • Milestone changed from Future Release to 4.3

#5 @westonruter
9 years ago

  • Description modified (diff)

#6 @adamsilverstein
9 years ago

Interesting proposal. I love the idea of heartbeat integration/locking!

Can you elaborate on how individual settings locking would work? If two users are editing different settings at the same time and each try saving, wouldn’t the save of one user overwrite the others? Seems like this could lead to race conditions? Locking users out of customizer entirely would be much simpler to implement and might fulfil the goal of this ticket; it would certainly be a good start, although I agree that as the customizer is used more and more, individual settings locking would be useful.

#7 @westonruter
9 years ago

  • Description modified (diff)

This ticket was mentioned in Slack in #core by westonruter. View the logs.


9 years ago

#9 @westonruter
9 years ago

  • Description modified (diff)

#10 @westonruter
9 years ago

  • Description modified (diff)

This ticket was mentioned in Slack in #core by helen. View the logs.


9 years ago

#12 @ocean90
9 years ago

  • Milestone changed from 4.3 to Future Release
  • Type changed from defect (bug) to enhancement

No patch yet.

#13 follow-up: @azaozz
9 years ago

Locking the customizer when in use is a really good idea. Locking individual settings in the customizer seems too much. The customizer, and the rest of the settings screens are not used frequently. Also settings usually require an admin level user to change, so collisions are likely to occur only on sites with many admins (which are quite rare).

In that terms a general per-user lock for any settings screen, including the customizer, seems best.

#14 in reply to: ↑ 13 @celloexpressions
9 years ago

Replying to azaozz:

Locking the customizer when in use is a really good idea. Locking individual settings in the customizer seems too much. The customizer, and the rest of the settings screens are not used frequently. Also settings usually require an admin level user to change, so collisions are likely to occur only on sites with many admins (which are quite rare).

In that terms a general per-user lock for any settings screen, including the customizer, seems best.

In terms of thinking towards the future, and particularly potential future uses of the Customizer API potentially including content, having per-setting locking would be awesome.

This is certainly a forward-looking proposal and may take a few releases to get right, but if we can go all the way and do it per-setting we'll set ourselves up really nicely for future additions and use-cases.

#15 @westonruter
9 years ago

  • Milestone changed from Future Release to 4.4

This ticket was mentioned in Slack in #core by sergey. View the logs.


9 years ago

#17 @westonruter
9 years ago

  • Milestone changed from 4.4 to Future Release

Punting. Will need to be part of feature plugin first.

#18 @westonruter
8 years ago

  • Owner changed from westonruter to lgedeon
  • Status changed from accepted to assigned

Work is being resumed here: https://github.com/xwp/wp-customize-concurrency/pull/4

Feature plugin will be released and then after community review it will be prepared for a core patch.

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


8 years ago

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


8 years ago

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


7 years ago

#22 @westonruter
7 years ago

In 41839:

Customize: Add changeset locking in Customizer to prevent users from overriding each other's changes.

  • Customization locking is checked when changesets are saved and when heartbeat ticks.
  • Lock is lifted immediately upon a user closing the Customizer.
  • Heartbeat is introduced into Customizer.
  • Changes made to user after it was locked by another user are stored as an autosave revision for restoration.
  • Lock notification displays link to preview the other user's changes on the frontend.
  • A user loading a locked Customizer changeset will be presented with an option to take over.
  • Autosave revisions attached to a published changeset are converted into auto-drafts so that they will be presented to users for restoration.
  • Focus constraining is improved in overlay notifications.
  • Escape key is stopped from propagating in overlay notifications, and it dismisses dismissible overlay notifications.
  • Introduces changesetLocked state which is used to disable the Save button and suppress the AYS dialog when leaving the Customizer.
  • Fixes bug where users could be presented with each other's autosave revisions.

Props sayedwp, westonruter, melchoyce.
See #31436, #31897, #39896.
Fixes #42024.

#23 @westonruter
7 years ago

  • Description modified (diff)
  • Owner lgedeon deleted

#24 @celloexpressions
3 years ago

It would be interesting to see some prototyping here for Gutenberg phase 3 - concurrent editing. The customize API, and the types of settings that it includes, are modular enough that there's a good baseline for enabling concurrency and locking relatively quickly. Previous work (linked above) looks like a good start for implementation.

This would be separate from whatever mechanism blocks and the block editor end up with, but could work for widget-blocks in the customizer and bring this improvement to non-block-based themes. And it could offer insights into a better overall API approach for the block version's build out.

Note: See TracTickets for help on using tickets.