Skip to content

Posts: Unstick a post given a password through Quick Edit. - #13246

Open
wppoland wants to merge 6 commits into
WordPress:trunkfrom
wppoland:trac/64810-sticky-password
Open

wppoland wants to merge 6 commits into
WordPress:trunkfrom
wppoland:trac/64810-sticky-password

Conversation

@wppoland

@wppoland wppoland commented Aug 24, 2026 •

Copy link
Copy Markdown

Trac ticket: https://core.trac.wordpress.org/ticket/64810

Opened at @audrasjb's request in comment:15: the change from #11180 plus the unit test that was only sitting in a comment there, so the ticket can lose needs-unit-tests and be committed as one changeset.

The change

@Hug0-Drelon's patch from #11180, committed under their authorship, plus one guard added in review (see below).

Quick Edit posts sticky but never posts visibility. edit_post() only unsets sticky inside case 'password' of the visibility switch, so that branch was never reached and a post could end up both sticky and password protected, a combination the block editor itself forbids.

Inferring visibility => 'password' from a non-empty post password makes the existing branch run for a caller that sends no visibility of its own:

// Infer the password visibility when the caller did not send one of its own.
if ( ! isset( $post_data['visibility'] ) && ! empty( $post_data['post_password'] ) ) {
	$post_data['visibility'] = 'password';
}

Why the ! isset() guard

Inferring unconditionally would have outranked the caller. Two inputs changed behaviour beyond the reported one:

  • visibility => 'public' sent with a non-empty password kept the password, where trunk drops it.
  • visibility => 'private' sent with a password left the post public and password protected instead of private. This one is reachable from the screen the ticket is about: wp_ajax_inline_save() sets visibility to private whenever "Private" is ticked (ajax-actions.php:2146).

With the guard, the only behaviour this PR changes is the reported one.

The tests

Five tests in tests/phpunit/tests/admin/includesPost.php, which had no coverage of the visibility / sticky / password interaction at all.

  • test_edit_post_unsticks_a_post_when_a_password_is_set_without_visibility() reproduces the Quick Edit payload: sticky is posted, visibility is not. This is the ticket's bug.
  • Three characterisation tests for the public, password and private branches reached by an explicit value. The private one sends a password alongside, which makes it the regression test for the Quick Edit Private case above.
  • test_edit_post_keeps_an_explicit_visibility_over_an_inferred_one() pins that the inference never overrides the caller.

Verified in three directions locally, PHP 8.3 against a single site:

src/wp-admin/includes/post.php Result
this branch OK (5 tests, 10 assertions)
trunk, tests kept 1 failure, only the ticket's bug
unguarded inference, tests kept 2 failures, the private case and the explicit-public case

Whole file: 56 tests, 125 assertions, 0 failures. The 6 warnings are the pre-existing E_DEPRECATED expectation notices PHPUnit 9.6 emits across that file.

phpcs on both touched files: 0 errors. The single warning in src/wp-admin/includes/post.php is at line 901 and predates this branch.

Testing

Independently confirmed in Playground by @ArkaPrabhaChowdhury in comment:20 against 7.2-alpha, PHP 8.3.

Credit

The fix is @Hug0-Drelon's work and the props on #11180 already list hugod, wildworks, abcd95, motylanogha. If this lands instead of #11180, that props line should carry over, plus arkaprabhachowdhury for the test report.

The open question this PR does not settle

2nd-opinion was removed from the ticket on 2026-08-24, but the disagreement behind it stands on its own: in comment:13 @wildworks argued that password protected and sticky should be allowed together in all cases, and that #11180 removes something Quick Edit could previously do.

That is a disagreement about intended behaviour, not about the code. This PR exists because @audrasjb asked for the change and the test in one place; if the ticket resolves the other way, the same test file is where the inverse expectation would go.

Use of AI Tools

AI assistance: Yes
Tool(s): Antigravity IDE
Model(s): Gemini 2.5 Pro
Used for: Code analysis, conflict resolution, test suite updates, and patch verification. All changes were manually reviewed and tested locally.


This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

@github-actions

github-actions Bot commented Aug 24, 2026 •

Copy link
Copy Markdown

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 hugod, motylanogha, gwendolinep.

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

@github-actions

Copy link
Copy Markdown

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@Hug0-Drelon Hug0-Drelon left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Should we take the opportunity to improve code coverage for https://github.com/WordPress/wordpress-develop/pull/13246/changes#diff-7c58dcea3675e604ab0caa9263522686924e3dd881d5a36b94865dacfbca129bR309-R323?

i.e. testing with $post_data['visibility'] set to 'public'|'password'|'private'?

Otherwise, thanks for the proposed tests!

@wppoland

Copy link
Copy Markdown
Author

Good call, done in 5540b57. Three characterisation tests for the switch, plus a fourth that turned out to matter more than I expected.

  • public clears an existing password.
  • password unsticks the post. Same branch as the first test, but reached by a caller sending the value outright rather than by inference.
  • private sets post_status to private, clears the password and unsticks.

The fourth one

While writing those I noticed the change alters behaviour for one input nobody has mentioned on the ticket.

Inferring the visibility from the password happens after the caller's own value is read, so the two can now contradict each other and the inferred one wins. On trunk, visibility => 'public' sent alongside post_password => 'secret' hits case 'public' and the password is discarded. With this change it becomes case 'password', so the password is kept and the post is unstuck instead.

test_edit_post_prefers_the_password_over_a_contradicting_public_visibility() pins that. Both editors clear the password field when Public is selected, so the pair should not arrive from the UI, but edit_post() is reachable from bulk edit and Quick Edit too, and silently keeping a password where trunk dropped it is the kind of thing better written down than discovered later.

If that precedence is wrong, the fix is to infer only when the caller sent no visibility at all. Happy to change it either way, it is a one-line difference. @wildworks this may be relevant to the 2nd-opinion question, since it is a second behaviour the change alters beyond the sticky one.

Verification

Four new tests, negative control run rather than assumed. Against trunk's post.php with the tests kept, exactly two fail: the original inferred-visibility test and the precedence one. The three characterisation tests pass either way, which is what they are for.

Whole file: 56 tests, 125 assertions, 0 failures. The 6 warnings are the pre-existing E_DEPRECATED expectation notices in the test_post_exists_* tests. phpcs clean.

@Hug0-Drelon

Copy link
Copy Markdown

If that precedence is wrong, the fix is to infer only when the caller sent no visibility at all. Happy to change it either way, it is a one-line difference.

I think this needs a core committer opinion.

@wppoland

Copy link
Copy Markdown
Author

@Hug0-Drelon Fair, and while writing up the question for a committer I found a second case that answers it, so I have taken the narrow route in 4ecdf88.

The precedence question does not need a ruling

wp_ajax_inline_save() sets visibility to private whenever "Private" is ticked (ajax-actions.php:2146). So Quick Edit does send a visibility, in one case, and a user who ticks Private and types a password sends visibility => 'private' with post_password => 'secret'.

Inferring unconditionally rewrote that to password. The post stayed public with its password intact instead of becoming private. That is not the contradictory pair I had written off as unreachable from the UI: it is two clicks away in the very screen this ticket is about.

The change

// Infer the password visibility when the caller did not send one of its own.
if ( ! isset( $post_data['visibility'] ) && ! empty( $post_data['post_password'] ) ) {
	$post_data['visibility'] = 'password';
}

The inference now only fills a gap. Every caller that sends a visibility behaves exactly as it does on trunk, so the only behaviour this PR changes is the reported one: a caller that sends no visibility at all, which is Quick Edit in every case except Private.

Tests

  • test_edit_post_privatises_and_unsticks_the_post_when_visibility_is_private() now sends a password alongside, which turns it into the regression test for the case above.
  • The precedence test is inverted and renamed to test_edit_post_keeps_an_explicit_visibility_over_an_inferred_one(): explicit public plus a password still clears the password, as on trunk.

Verified in three directions

src/wp-admin/includes/post.php Result
this branch OK (5 tests, 10 assertions)
trunk, tests kept 1 failure, only ..._unsticks_a_post_when_a_password_is_set_without_visibility
unguarded inference, tests kept 2 failures, the private case and the explicit-public case

The middle row is the ticket's bug and nothing else. The bottom row is why the guard is there.

Whole file: 56 tests, 125 assertions, 0 failures. The 6 warnings are the pre-existing E_DEPRECATED expectation notices in the test_post_exists_* tests. phpcs clean on both files, apart from the unrelated line 901 warning that predates this branch.

If a committer would rather have the password win outright, dropping the ! isset() is a one-line revert and the two tests flip back. I do not think it should, on the strength of the Private case.

@wppoland
wppoland force-pushed the trac/64810-sticky-password branch from 4ecdf88 to 42e4b1a Compare September 13, 2026 07:40
@GwendolinePicquet

GwendolinePicquet commented Sep 17, 2026 •

Copy link
Copy Markdown
post_password_failed

At this day, the bug is like that (I'm a begginer).

case 'password':
unset( $post_data['sticky'] );
break;

https://www.php.net/manual/en/function.unset.php
unset-PHP

@wppoland

Copy link
Copy Markdown
Author

Welcome @GwendolinePicquet! Exactly: lines 310 and 315 use unset( $post_data['sticky'] ) to remove the sticky status whenever visibility is 'password' or 'private'.

The bug before this PR was that Quick Edit updates the password but never sends a visibility parameter at all. Because of that, edit_post() never entered the case 'password' block, so unset( $post_data['sticky'] ) never executed.

By inferring visibility = 'password' when a password is provided without an explicit visibility, the existing case 'password' branch now runs and unsets sticky as intended.

Hug0-Drelon and others added 4 commits September 25, 2026 08:22
Quick Edit posts `sticky` but never posts `visibility`, so `edit_post()`
never reached the `case 'password'` branch that drops the sticky flag. A
post could end up both sticky and password protected, a combination the
block editor itself forbids.

Set `visibility` to `password` whenever a post password is present, so
the existing branch runs whatever the caller sent.

Original patch by Hug0-Drelon in PR WordPress#11180.

See #64810.
…value.

Reproduces the Quick Edit payload: `sticky` is posted, `visibility` is
not. Fails on trunk, where the post keeps both the sticky flag and the
new password, and passes with the preceding change.

See #64810.
Adds the `public`, `password` and `private` cases of the visibility
switch, which had no coverage before, and pins the precedence between an
inferred visibility and a contradicting one sent by the caller.

That last one is a behaviour change worth naming: on trunk, sending
`visibility` as `public` alongside a non-empty `post_password` cleared
the password. Inferring the visibility from the password applies last,
so the password now wins instead. The editors never send that pair,
since choosing Public clears the password field, but `edit_post()` is
also reachable from bulk edit and Quick Edit.

See #64810.
The inference is meant to fill a gap, not to outrank the caller. Reading it
after the caller's own value let the two contradict each other, with the
inferred one applied last:

- `visibility => 'public'` sent with a non-empty password kept the password,
  where trunk dropped it.
- `visibility => 'private'` sent with a password left the post public and
  password protected instead of private. Quick Edit reaches this pair through
  `wp_ajax_inline_save()`, which sets `visibility` to `private` whenever
  "Private" is ticked.

Guarding the inference with `! isset( $post_data['visibility'] )` confines the
behaviour change to the reported case, a caller that sends no visibility at all.

The `private` characterisation test now sends a password alongside, and the
precedence test is inverted: an explicit visibility survives.

See #64810.
@wppoland
wppoland force-pushed the trac/64810-sticky-password branch from e280c39 to ccd71dc Compare September 25, 2026 06:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants