WordPress.org

Make WordPress Core

Opened 20 months ago

Closed 20 months ago

Last modified 20 months ago

#24830 closed defect (bug) (fixed)

No notifications should be shown when filter 'show_post_locked_dialog' returns false

Reported by: DH-Shredder Owned by: aaroncampbell
Milestone: 3.6 Priority: normal
Severity: normal Version: 3.6
Component: Autosave Keywords: commit
Focuses: Cc:

Description

At the moment, when the filter 'show_post_locked_dialog' returns false, the initial post lock dialog is silenced, but not the 'taken over' dialog, essentially leaving in the 'user is booted' behavior.

We should change this so that instead when the filter is set, post locking functions more similarly to 3.5, by stopping the heartbeat lock/kickout and notice.

Steps to Reproduce:

  • 'show_post_locked_dialog' filter set to return false in a plugin
  • User1 Opens Post for Editing
  • User2 Opens Post for Editing, and does not see a "post is locked" notice
  • After Heartbeat, User1 sees a post is locked notice

Expected Behavior:
No notices are be shown when 'show_post_locked_dialog' returns false.

Attachments (4)

24830.patch (2.0 KB) - added by azaozz 20 months ago.
24830-2.patch (2.6 KB) - added by azaozz 20 months ago.
24830-3.patch (1.8 KB) - added by azaozz 20 months ago.
24830-4.patch (1.9 KB) - added by DH-Shredder 20 months ago.
Refreshed patch to trunk, changed names to: 'post_locked_dialog' and 'lock_taken_over_dialog'

Download all attachments as: .zip

Change History (14)

comment:1 @nacin20 months ago

  • Milestone changed from Awaiting Review to 3.6

comment:2 @azaozz20 months ago

Perhaps it would be better to remove this filter completely. Plugins have several other options to manage post locking, including adding a "Close" button to the dialog, changing it from a modal to a standard warning (above the title field), etc.

To stop showing post locked dialogs completely, it is better to remove the html for the modal. Then locks won't be checked with heartbeat too. Example:

add_action( 'dbx_post_sidebar', 'my_disable_post_locks' );
function my_disable_post_locks() {
  if ( ... )
    remove_action( 'admin_footer', '_admin_notice_post_locked' );
}

Alternatively the filter can be moved to wp_refresh_post_lock(). Then a plugin can decide whether to show the "Post taken over" part of the dialog, i.e. if a user will be "booted" when another user/superadmin takes over the lock. This can also be done by hooking into the existing 'heartbeat_send' filter and removing the wp-refresh-post-lock array key from the response, but it will be running after we generate that response.

A better improvement would be to have separate actions for adding text to either the "Post is currently locked" or "Post lock taken over" parts of the dialog. Currently there is one action 'post_lock_text' fired in both.

Last edited 20 months ago by azaozz (previous) (diff)

@azaozz20 months ago

comment:3 @azaozz20 months ago

In 24830.patch:

  • Remove the show_post_locked_dialog filter.
  • Rename the post_lock_text action to post_locked_text and add post_lock_taken_over_text for the "taken over" part of the modal (suggestions for better names welcome).

comment:4 @markjaquith20 months ago

  • Keywords commit added

I'm good with 24830.patch.

@azaozz20 months ago

@azaozz20 months ago

comment:5 @azaozz20 months ago

24830-2.patch (added for reference)​ add the show_post_locked_dialog filter to wp_refresh_post_lock() that handles the response through heartbeat. This filter can be used to stop showing the "Post lock taken over" dialog, i.e. not "boot" a user when somebody else edits the same post. However that would not be enough to handle cases where this might be needed. It would cause arbitrary overwriting and the users would "fight" who gets the lock.

24830-3.patch keeps show_post_locked_dialog in _admin_notice_post_locked(). The change is it returns early so no html is outputted. This effectively disables showing the dialogs and checking post locks.

@DH-Shredder20 months ago

Refreshed patch to trunk, changed names to: 'post_locked_dialog' and 'lock_taken_over_dialog'

comment:6 @DH-Shredder20 months ago

In 24830-4.patch:

  • Quick refresh to trunk from 24830-3.patch.
  • After chats with @azaozz, changed the names of the actions to 'post_locked_dialog' and 'lock_taken_over_dialog'.

comment:7 @aaroncampbell20 months ago

  • Owner set to aaroncampbell
  • Resolution set to fixed
  • Status changed from new to closed

In 24883:

Make the show_post_locked_dialog filter hide both the post locked and post taken over dialogs

props azaozz, DH-Shredder. Fixes #24830 for trunk.

comment:8 @nacin20 months ago

In 24884:

Rename post_lock_text hook to post_locked_dialog, and lock_taken_over_dialog to post_lock_lost_dialog. fixes #24830 for trunk.

comment:9 @nacin20 months ago

[24883] missed a filter rename. After a discussion with DH-Shredder and aaroncampbell (in person), the second filter ended up with a slightly different name, for [24884].

comment:10 @aaroncampbell20 months ago

In 24886:

Make the show_post_locked_dialog filter hide both the post locked and post taken over dialogs

props azaozz, DH-Shredder. Fixes #24830 for 3.6.

Note: See TracTickets for help on using tickets.