Skip to content

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

Open
Intenzi wants to merge 2 commits into
WordPress:mainfrom
Intenzi:fix/hidden-post-lock-updates
Open

Fix: Hidden post-edit tabs still refresh the post-lock presence entry#78
Intenzi wants to merge 2 commits into
WordPress:mainfrom
Intenzi:fix/hidden-post-lock-updates

Conversation

@Intenzi

@Intenzi Intenzi commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Resolves Issue: #21
Related PRs: #19 , #20

How?

  • Deletes the wp-refresh-post-lock key from the shared heartbeat data object when the tab/document is backgrounded (document.visibilityState === 'hidden').
    • Because this key is deleted, WordPress Core's own heartbeat ping doesn't get sent and is thus no longer causing the intercept to update the presence database.
  • Wrapped the heartbeat listener in a document ready hook ($(function() { ... })). Because our script runs earlier/faster than WordPress Core, registering our listener resulted in an empty data object when attempting to delete the key. Delaying to document ready ensures that the data['wp-refresh-post-lock'] key exists by the time our handler runs, allowing us to successfully delete it.
  • Updated the visibility E2E test to assert that wp-refresh-post-lock is omitted from the heartbeat payload when the editor tab is hidden.

Note

Since we defer the registration specifically to ensure we run after WordPress Core's handler. Keeping all of our code inside this single deferred handler is the simplest approach for now. If we ever need to run other tasks at the very beginning of the heartbeat tick in the future, we can easily split this into two separate listeners (one immediate, one deferred) without any breaking changes.

Use of AI Tools

AI assistance: Yes
Tool(s): Antigravity IDE
Model(s): Gemini 3.5 Flash (Low)
Used for: Identifying relevant code file, going through related PRs, it suggested utilising the deferred execution later when debugging.

@Intenzi
Intenzi requested a review from josephfusco as a code owner July 17, 2026 19:27
@github-actions

Copy link
Copy Markdown
Contributor

▶ Preview in WordPress Playground

Open in WordPress Playground

Boots a fresh WordPress with this PR's presence-api build, seeds 5 demo users, and drops you on the dashboard.

Stress-test variant: 40 demo users · Built from 1a67dd600b70d5057dfb9edfa895f1f2dada43b2. Auto-updates when you push.

@github-actions

Copy link
Copy Markdown
Contributor

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props intenzi.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@josephfusco josephfusco added [Type] Bug An existing feature is broken [Area] Post Lock Bridge Issues for the post-lock bridge labels Jul 21, 2026

@josephfusco josephfusco left a comment

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.

Good fix. One suggestion inline.

const hidden = await captureHeartbeatSend( page );
expect( hidden[ 'presence-editor-ping' ] ).toBeUndefined();
expect( hidden[ 'wp-refresh-post-lock' ] ).toBeUndefined();

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

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Area] Post Lock Bridge Issues for the post-lock bridge [Type] Bug An existing feature is broken

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants