Opened 7 months ago
Last modified 2 weeks ago
#64810 new defect (bug)
A password protected post can be sticky but sometimes this behaviour is forbidden.
| Reported by: | clementpolito | Owned by: | |
|---|---|---|---|
| Priority: | normal | Milestone: | 7.2 |
| Component: | Posts, Post Types | Version: | |
| Severity: | normal | Keywords: | has-screenshots has-patch has-unit-tests needs-testing has-test-info |
| Cc: | Focuses: |
Description
A password protected post can be sticky, in several ways, but sometimes it is forbidden.
With Block Editor
When working on a post, I can check the ‘Password Protected’ and ‘Sticky’ boxes at the same time.
But if I set a password and save, I get the error message :
Creating a new post
Publishing failed. A post can not be sticky and have a password.
Updating a post
Updating failed. A sticky post cannot be password protected.
Quick Edit
When using quick edit, I can set both at the same time (Password Protected and Sticky)
With Classic Editor 1.6.7
With the classic editor, in the UI, I cannot set a password-protected post as Sticky because "Password protected" is only available as an sub-option of Public page.
In REST API
I get an Error 400 :
curl --request POST \ --url 'https://zip-website.ddev.site/wp-json/wp/v2/posts?title=sticky%20bis&password=123456&sticky=true' \ --header 'authorization: Basic cGxsLWFkbWluOnY0WkUgZUxwYSBGVktaIGJUSGMgcUJjMiBadmJs'
Answer :
{
"code": "rest_invalid_field",
"message": "A post can not be sticky and have a password.",
"data": {
"status": 400
}
}
Can we improve the UI in the editor so that we don't have the two possible options?
Can we standardise the behaviour so that it is blocked everywhere or accepted everywhere?
Attachments (5)
Change History (29)
@
7 months ago
When I publish the post, I get an error message : "A post can not be sticky and have a password."
#1
@
7 months ago
In REST API, the code show that it's not possible to have both sticky and password proctected posts.
Code can be reach here : https://github.com/WordPress/wordpress-develop/blob/6.9.1/src/wp-includes/rest-api/endpoints/class-wp-rest-posts-controller.php#L1407-L1428
This ticket was mentioned in PR #11180 on WordPress/wordpress-develop by @hugod.
7 months ago
#2
- Keywords has-patch added; needs-patch removed
Visibility is never set for password when using quick edit, thus edit_post() cannot unset sticky status in https://github.com/WordPress/wordpress-develop/blob/6.9.1/src/wp-admin/includes/post.php#L308-L322.
Looking at it, I thought that wp_ajax_inline_save() would handle that like it does for private status. But it might be dead code around https://github.com/WordPress/wordpress-develop/blob/6.9.1/src/wp-admin/includes/ajax-actions.php#L2117-L2122.
Removing it does effect the feature (remove sticky status when a post is private).
Trac ticket: https://core.trac.wordpress.org/ticket/64810
#3
@
7 months ago
- Component General → Posts, Post Types
- Milestone Awaiting Review → Future Release
- Version 6.9.1
#5
@
7 months ago
I tested this issue and was able to reproduce the behavior.
I also tested the provided patch, and it is working as expected. The issue with sticky posts and password-protected posts appears to be resolved after applying the patch.
Editor without applying patch:-
https://prnt.sc/vt86y5NOEo0d
Quick edit:-
https://prnt.sc/56vvH2IXQutt
Editor after applying the patch:-
https://prnt.sc/hJ2AfH5yCg_N
#6
@
3 months ago
- Keywords needs-unit-tests added
To move this ticket forward, a PR review is needed, but it appears to be missing unit tests at a minimum.
This ticket was mentioned in Slack in #core by sajjad67. View the logs.
3 months ago
@abcd95 commented on PR #11180:
2 months ago
#8
Hi @Hug0-Drelon, do you have the bandwidth to address the reviews and add some test coverage for this PR?
@motylanogha commented on PR #11180:
2 months ago
#9
The ticket carries needs-unit-tests (added by @westonruter in comment:8) and this PR is fix-only, so here's a regression test for it.
It reproduces Quick Edit's actual payload: sticky is posted, visibility is not. On trunk the case 'password' branch in edit_post() is never reached, so the unset( $post_data['sticky'] ) never happens and the post stays sticky with a password set. With this PR the inferred visibility reaches that branch and the post is unstuck.
/** * Ensures that giving a sticky post a password through edit_post() unsticks it, * even when the caller never sends an explicit `visibility` value. * * The block editor sends `visibility`, so `edit_post()` reaches the * `case 'password'` branch and drops the `sticky` flag. Quick Edit never sends * it, so a post could end up both sticky and password protected, a combination * the editor itself forbids. * * @ticket 64810 * * @covers ::edit_post */ public function test_edit_post_unsticks_a_post_when_a_password_is_set_without_visibility() { wp_set_current_user( self::$admin_id ); $post_id = self::factory()->post->create( array( 'post_status' => 'publish', 'post_author' => self::$admin_id, ) ); stick_post( $post_id ); $this->assertTrue( is_sticky( $post_id ), 'The post was not sticky to begin with: check test setup.' ); // Mirrors Quick Edit, which posts `sticky` but no `visibility`. edit_post( array( 'post_ID' => $post_id, 'post_title' => 'Protected and sticky', 'post_status' => 'publish', 'post_password' => 'secret', 'sticky' => 'sticky', ) ); $this->assertSame( 'secret', get_post( $post_id )->post_password, 'The password was not saved: check test setup.' ); $this->assertFalse( is_sticky( $post_id ), 'A password protected post was left sticky when no explicit visibility was sent.' ); }
It goes in tests/phpunit/tests/admin/includesPost.php, which had no coverage of the visibility/sticky/password interaction at all.
Verified against a local build:
trunk: Failures: 1 ("A password protected post was left sticky when no explicit visibility was sent.")
trunk + this PR: OK (1 test, 3 assertions)
Whole file still green with the patch applied: 52 tests, 118 assertions, 0 failures (6 warnings, all pre-existing PHPUnit 10 deprecation notices in test_post_exists_*).
Also attached to the ticket as 64810-tests.diff. Take it into this branch if it helps, no attribution needed.
#10
@
2 months ago
Following @wildworks' note in comment:6 that this is missing unit tests: PR #11180 is fix-only, so here is a regression test for it. Posted on the PR as well (PR11180#issuecomment-5122723830).
It reproduces what Quick Edit actually sends, sticky but no visibility. On trunk the case 'password' branch in edit_post() is never reached, so unset( $post_data['sticky'] ) never runs and the post stays sticky with a password set. With the PR the inferred visibility reaches that branch and the post is unstuck.
/** * Ensures that giving a sticky post a password through edit_post() unsticks it, * even when the caller never sends an explicit `visibility` value. * * @ticket 64810 * * @covers ::edit_post */ public function test_edit_post_unsticks_a_post_when_a_password_is_set_without_visibility() { wp_set_current_user( self::$admin_id ); $post_id = self::factory()->post->create( array( 'post_status' => 'publish', 'post_author' => self::$admin_id, ) ); stick_post( $post_id ); $this->assertTrue( is_sticky( $post_id ), 'The post was not sticky to begin with: check test setup.' ); // Mirrors Quick Edit, which posts `sticky` but no `visibility`. edit_post( array( 'post_ID' => $post_id, 'post_title' => 'Protected and sticky', 'post_status' => 'publish', 'post_password' => 'secret', 'sticky' => 'sticky', ) ); $this->assertSame( 'secret', get_post( $post_id )->post_password, 'The password was not saved: check test setup.' ); $this->assertFalse( is_sticky( $post_id ), 'A password protected post was left sticky when no explicit visibility was sent.' ); }
Goes in tests/phpunit/tests/admin/includesPost.php, which had no coverage of the visibility / sticky / password interaction at all.
Verified locally:
- trunk: fails, "A password protected post was left sticky when no explicit visibility was sent."
- trunk + PR #11180: OK (1 test, 3 assertions)
- whole file with the patch applied: 52 tests, 118 assertions, 0 failures (6 warnings, all pre-existing PHPUnit 10 deprecation notices in the
test_post_exists_*tests)
Not adding has-unit-tests, since the tests are proposed here rather than committed to the PR; that keyword belongs to whoever folds them in.
@motylanogha commented on PR #11180:
2 months ago
#11
Rather than leave the test as a paste, I've opened it as a commit against this branch: Hug0-Drelon/wordpress-develop#1.
Merging it there brings the test into this PR so the fix and its coverage land as one changeset, and nothing else on your side changes. The fix stays yours; I only wrote the test.
This ticket was mentioned in Slack in #core by adrianduffell. View the logs.
2 months ago
#13
@
2 months ago
- Milestone 7.1 → 7.2
Proceeding with PR 11180 would mean that we can no longer apply both "Password Protected" and "Sticky" to a single post, a functionality previously achievable through quick edit.
I've reconsidered this matter from scratch, but is this truly the ideal behavior? For instance, a site administrator might want to make a password-protected post sticky to make an announcement for a limited audience?
I might be missing something, but I believe that "Password Protected" and "Sticky" should be allowed to be applied simultaneously in all cases. If there was any specific reason or background behind the decision to prohibit this, I would like to know about it.
In any case, I believe we should refrain from shipping PR 11180 in 7.1, as it restricts functionality that was previously available.
@motylanogha commented on PR #11180:
7 weeks ago
#14
@Hug0-Drelon this is the unit test @wildworks asked for in comment:6 when they marked #64810 needs-unit-tests. Bringing it here rather than leaving it on a fork branch, so it is visible to whoever picks this up.
It reproduces what Quick Edit actually posts, sticky with no visibility. On trunk edit_post() never reaches case 'password', so unset( $post_data['sticky'] ) never runs and the post stays sticky with a password set. With your fix the inferred visibility reaches that branch and the post is unstuck. tests/phpunit/tests/admin/includesPost.php had no coverage of the visibility and sticky interaction at all before this.
Ready to cherry-pick onto this branch as-is:
-
tests/phpunit/tests/admin/includesPost.php
diff --git a/tests/phpunit/tests/admin/includesPost.php b/tests/phpunit/tests/admin/includesPost.php index d9d39d8da727d..dac5c8bc86dfa 100644
a b public function test_user_get_refreshed_metabox_nonce() { 1371 1371 $this->assertNotEmpty( $response['wp-refresh-metabox-loader-nonces']['replace']['_wpnonce'] ); 1372 1372 $this->assertNotEmpty( $response['wp-refresh-metabox-loader-nonces']['replace']['metabox_loader_nonce'] ); 1373 1373 } 1374 1375 /** 1376 * Ensures that giving a sticky post a password through edit_post() unsticks it, 1377 * even when the caller never sends an explicit `visibility` value. 1378 * 1379 * The block editor sends `visibility`, so `edit_post()` reaches the 1380 * `case 'password'` branch and drops the `sticky` flag. Quick Edit never sends 1381 * it, so a post could end up both sticky and password protected, a combination 1382 * the editor itself forbids. 1383 * 1384 * @ticket 64810 1385 * 1386 * @covers ::edit_post 1387 */ 1388 public function test_edit_post_unsticks_a_post_when_a_password_is_set_without_visibility() { 1389 wp_set_current_user( self::$admin_id ); 1390 1391 $post_id = self::factory()->post->create( 1392 array( 1393 'post_status' => 'publish', 1394 'post_author' => self::$admin_id, 1395 ) 1396 ); 1397 1398 stick_post( $post_id ); 1399 $this->assertTrue( is_sticky( $post_id ), 'The post was not sticky to begin with: check test setup.' ); 1400 1401 // Mirrors Quick Edit, which posts `sticky` but no `visibility`. 1402 edit_post( 1403 array( 1404 'post_ID' => $post_id, 1405 'post_title' => 'Protected and sticky', 1406 'post_status' => 'publish', 1407 'post_password' => 'secret', 1408 'sticky' => 'sticky', 1409 ) 1410 ); 1411 1412 $this->assertSame( 1413 'secret', 1414 get_post( $post_id )->post_password, 1415 'The password was not saved: check test setup.' 1416 ); 1417 $this->assertFalse( 1418 is_sticky( $post_id ), 1419 'A password protected post was left sticky when no explicit visibility was sent.' 1420 ); 1421 } 1374 1422 }
It is yours to take, and it belongs in this changeset rather than a separate one, since it only passes with the fix in place. Ticket credit sorts itself out through the props bot either way.
#15
@
7 weeks ago
Hi @hugod do you have the bandwidth to include the proposed unit tests?
Anyway, everybody is welcome to open a new PR including the proposed change + the unit tests. Pinging @motylanogha, as you proposed the unit test.
This ticket was mentioned in PR #13246 on WordPress/wordpress-develop by @motylanogha.
7 weeks ago
#16
- Keywords has-unit-tests added; needs-unit-tests removed
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, unmodified, and committed under their authorship.
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.
Setting visibility to password whenever a post password is present makes the existing branch run whatever the caller sent.
## The test
test_edit_post_unsticks_a_post_when_a_password_is_set_without_visibility() reproduces the Quick Edit payload: sticky is posted, visibility is not.
Verified both directions locally, PHP 8.3 against a single site:
- with the change:
OK (1 test, 3 assertions) - with the change reverted, test kept:
Failed asserting that true is false. A password protected post was left sticky when no explicit visibility was sent.
The rest of the file is unaffected: tests/phpunit/tests/admin/includesPost.php runs 52 tests, 118 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.
## 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.
@motylanogha commented on PR #13246:
7 weeks ago
#18
Good call, done in 5540b57. Three characterisation tests for the switch, plus a fourth that turned out to matter more than I expected.
publicclears an existing password.passwordunsticks the post. Same branch as the first test, but reached by a caller sending the value outright rather than by inference.privatesetspost_statustoprivate, 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.
@hugod commented on PR #13246:
7 weeks ago
#19
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.
@
6 weeks ago
WordPress Playground PR #13246: Quick Edit saved a password-protected post without retaining sticky status.
#20
@
6 weeks ago
Tested PR #13246 in WordPress Playground (WordPress 7.2-alpha-20260824.165403).
Environment: https://playground.wordpress.net/?core-pr=13246 with PHP 8.3.
Steps:
- Opened Posts > Quick Edit for the published “Hello world!” post.
- Entered a password.
- Checked “Make this post sticky”.
- Clicked Update.
Result: PASS. Playground reported “Changes saved.” The post was shown as “Password protected” in the Posts list and was not retained as sticky. This reproduces the Quick Edit payload described in the ticket (sticky submitted without an explicit visibility value) and confirms the patch prevents a password-protected post from remaining sticky.
The screenshot of the passing result is attached as wp-64810-playground-quick-edit.jpg.
This verifies the reported Quick Edit regression. The broader product/design question of whether password-protected sticky posts should be allowed remains a separate decision.
@motylanogha commented on PR #13246:
6 weeks ago
#21
@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(): explicitpublicplus 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.
GwendolinePicquet commented on PR #13246:
3 weeks ago
#22
@motylanogha commented on PR #13246:
3 weeks ago
#23
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.
#24
@
2 weeks ago
### Test Report
Ticket: https://core.trac.wordpress.org/ticket/64810
PR: https://github.com/WordPress/wordpress-develop/pull/13246
#### Environment
- WordPress: 7.2-alpha-63958 (trunk)
- PHP: 8.3.33
- Web Server: Nginx / PHP-FPM (macOS Darwin 24.6.0)
- Database: MariaDB 10.11
#### Steps to Test
- On
trunk:- Create a new public post, check "Stick this post to the front page", and publish.
- Navigate to Posts → All Posts (
wp-admin/edit.php). - Click Quick Edit on the sticky post.
- Enter a password into the Password field (leaving Sticky untouched or checked) and click Update.
- Check the post in the list table and database: the post retains the "Sticky" badge alongside the password protection (an invalid state in WordPress).
- Check out PR #13246:
- Repeat the Quick Edit steps: edit the sticky post and assign a password.
- Click Update.
- Check the post in the list table: the "Sticky" badge is correctly removed, and the post is password-protected.
- Run PHPUnit regression tests:
`bash vendor/bin/phpunit tests/phpunit/tests/admin/includesPost.php`
#### Test Results
- Before Patch (trunk):
- In Quick Edit, setting a password on a sticky post kept the post sticky.
- Running
test_edit_post_unsticks_a_post_when_a_password_is_set_without_visibilityontrunkfailed:` Failed asserting that true is false. A password protected post was left sticky when no explicit visibility was sent.`
- After Patch (PR #13246):
- Setting a password via Quick Edit properly clears the sticky flag and saves the password, matching the behavior of the Block Editor and REST API.
- All 56 tests (and 125 assertions) in
tests/phpunit/tests/admin/includesPost.phppass cleanly:test_edit_post_unsticks_a_post_when_a_password_is_set_without_visibilitypassed.test_edit_post_clears_the_password_when_visibility_is_publicpassed.test_edit_post_unsticks_the_post_when_visibility_is_passwordpassed.test_edit_post_privatises_and_unsticks_the_post_when_visibility_is_privatepassed.test_edit_post_keeps_an_explicit_visibility_over_an_inferred_onepassed.
- All 81 tests in
tests/phpunit/tests/post.phpcontinue to pass. - PHPCS with
WordPress-Coreruleset passes with 0 errors on modified lines.
The PR is ready for commit.
---
AI Disclosure: In accordance with the WordPress AI policy, I disclose that generative AI (Google Antigravity) was used in running the test suites and drafting this test report. All steps were executed and validated manually.
![(please configure the [header_logo] section in trac.ini)](/chrome/site/your_project_logo.png)
I can check password protected and sticky in the same time