Skip to content

Fix: Hidden post-edit tabs still refresh the post-lock presence entry #78

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Intenzi wants to merge 2 commits into
base: main
Choose a base branch
from
+28 −23
Open

Fix: Hidden post-edit tabs still refresh the post-lock presence entry #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
48a0ae4
fix: delete post-lock payload when tab is hidden
Intenzi Jul 16, 2026
1a67dd6
test: assert wp-refresh-post-lock is deleted when editor tab is hidden
Intenzi Jul 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 26 additions & 22 deletions includes/heartbeat.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,32 +149,36 @@ function wp_presence_enqueue_heartbeat_ping() {
// Guards against duplicate leave() invocations.
let hasLeft = false;

$(document).on('heartbeat-send', function (event, data) {
// Skip while the document is hidden (background tab, minimized
// window, app switched away) so the existing entries expire via
// the default TTL. One early-return suppresses both presence-ping
// and presence-editor-ping, since the consolidated handler emits
// both.
if (document.visibilityState === 'hidden') {
return;
}

const ping = { screen: window.pagenow || 'front' };
if (frontContext) {
if (frontContext.title) {
ping.title = frontContext.title;
// Defer registration to document ready to ensure it runs after WP Core's post.js handler.
$(function () {
$(document).on('heartbeat-send', function (event, data) {
// Skip while the document is hidden (background tab, minimized
// window, app switched away) so the existing entries expire via
// the default TTL. One early-return suppresses both presence-ping
// and presence-editor-ping, since the consolidated handler emits
// both.
if (document.visibilityState === 'hidden') {
delete data['wp-refresh-post-lock'];
return;
}
if (frontContext.post_id) {
ping.post_id = frontContext.post_id;

const ping = { screen: window.pagenow || 'front' };
if (frontContext) {
if (frontContext.title) {
ping.title = frontContext.title;
}
if (frontContext.post_id) {
ping.post_id = frontContext.post_id;
}
}
}
data['presence-ping'] = ping;
data['presence-ping'] = ping;

if (editorPostId) {
data['presence-editor-ping'] = { post_id: editorPostId };
}
if (editorPostId) {
data['presence-editor-ping'] = { post_id: editorPostId };
}

hasLeft = false;
hasLeft = false;
});
});

function leave() {
Expand Down
3 changes: 2 additions & 1 deletion tests/e2e/presence-visibility.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ test.describe( 'Presence Visibility', () => {
expect( visible[ 'presence-ping' ].screen ).toBeTruthy();
} );

test( 'heartbeat-send omits presence-editor-ping while editor tab is hidden', async ( {
test( 'heartbeat-send omits presence-editor-ping and wp-refresh-post-lock while editor tab is hidden', async ( {
admin,
page,
requestUtils,
Expand All @@ -108,6 +108,7 @@ test.describe( 'Presence Visibility', () => {
await setVisibility( page, 'hidden' );
const hidden = await captureHeartbeatSend( page );
expect( hidden[ 'presence-editor-ping' ] ).toBeUndefined();
expect( hidden[ 'wp-refresh-post-lock' ] ).toBeUndefined();

@josephfusco josephfusco Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth adding a positive assertion on the visible side too, so a regression that always deletes the key doesn't slip through:

Suggested change
expect( visible[ 'wp-refresh-post-lock' ] ).toBeDefined();

@Intenzi Intenzi Jul 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @josephfusco, I'd love your input on the implementation direction here.

I went through with implementing the positive assertion and it has lead to a blocker.
After applying the positive assertion, the test was consistently failing when I ran the e2e tests locally.

Upon digging deeper into how Core handles post locking, I realised that the current negative assertion implementation done by me is also a false positive.

In Core's post.js, wp-refresh-post-lock is only attached if #post-lock-dialog exists in the DOM:

// wp-admin/js/post.js
	/**
	 * Heartbeat locks.
	 *
	 * Used to lock editing of an object by only one user at a time.
	 *
	 * When the user does not send a heartbeat in a heartbeat-time
	 * the user is no longer editing and another user can start editing.
	 */
	$(document).on( 'heartbeat-send.refresh-lock', function( e, data ) {
		var lock = $('#active_post_lock').val(),
			post_id = $('#post_ID').val(),
			send = {};

		if ( ! post_id || ! $('#post-lock-dialog').length )
			return;

		send.post_id = post_id;

		if ( lock )
			send.lock = lock;

		data['wp-refresh-post-lock'] = send;

In standard Core PHP (_admin_notice_post_locked()), Core only outputs #post-lock-dialog into the HTML if there is an active lock conflict from another user. The another user check is performed inside wp_check_post_lock :-

...
	if ( $time && $time > time() - $time_window && get_current_user_id() !== $user ) {
		return $user;
	}

	return false;
}

For a single user editing a post alone, Core never outputs that div, so post.js skips attaching wp-refresh-post-lock altogether.

Because of this, the test was passing the negative assertion simply because Core never attached the key in the first place, not because our visibility handler stripped it.

Proposed Fix:

We can implement a separate test to use a genuine multi-user flow using Playwright's browser.newContext() to simulate two actual users (User A & User B):

User B opens the post first in Context 1 to acquire the lock.
User A opens the post second in Context 2, which triggers Core's multi-user lock dialog naturally.

Then we perform visibility, detached from visibility tests on it.

Alternatively, we could simulate the lock conflict state by updating _edit_lock meta prior to page load if we want a single-browser run.

Which approach would you prefer for the test suite?

@josephfusco josephfusco Jul 23, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch, you're right that the negative assertion was always true. I'd maybe lean towards the browser.newContext() approach. Have User B navigate to the post edit page first to naturally acquire the lock, then User A opens the same post and the lock dialog is present. Run the visibility assertions from there. That's the most authentic test of the behavior.

await setVisibility( page, 'visible' );
const visible = await captureHeartbeatSend( page );
Expand Down
Loading