Make WordPress Core

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)

01-post-password-protected-empty-and-sticky-possible.jpg​ (318.6 KB ) - added by clementpolito 7 months ago.
I can check password protected and sticky in the same time
02-post-password-protected-and-sticky-error-message.jpg​ (318.6 KB ) - added by clementpolito 7 months ago.
When I publish the post, I get an error message : "A post can not be sticky and have a password."
03-quick-edit-post-password-protected-and-sticky.jpg​ (604.5 KB ) - added by clementpolito 7 months ago.
I can define a password and stick the post in the quick edit 01/02
04-quick-edit-post-password-protected-and-sticky-validated.jpg​ (654.5 KB ) - added by clementpolito 7 months ago.
I can define a password and stick the post in the quick edit 02/02
wp-64810-playground-quick-edit.jpg​ (16.5 KB ) - added by arkaprabhachowdhury 6 weeks ago.
WordPress Playground PR #13246: Quick Edit saved a password-protected post without retaining sticky status.

Download all attachments as: .zip

Change History (29)

@clementpolito
7 months ago

I can check password protected and sticky in the same time

@clementpolito
7 months ago

When I publish the post, I get an error message : "A post can not be sticky and have a password."

@clementpolito
7 months ago

I can define a password and stick the post in the quick edit 01/02

@clementpolito
7 months ago

I can define a password and stick the post in the quick edit 02/02

#1 @jsmansart
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 @westonruter
7 months ago

  • Component General → Posts, Post Types
  • Milestone Awaiting Review → Future Release
  • Version 6.9.1

#4 @audrasjb
7 months ago

  • Milestone Future Release → 7.1

Looks like it moved enough to be milestoned to 7.1

#5 @khushi1501
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 @wildworks
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 @motylanogha
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.

Last edited 2 months ago by motylanogha (previous) (diff)

​@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 @wildworks
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() {  
    13711371                $this->assertNotEmpty( $response['wp-refresh-metabox-loader-nonces']['replace']['_wpnonce'] );
    13721372                $this->assertNotEmpty( $response['wp-refresh-metabox-loader-nonces']['replace']['metabox_loader_nonce'] );
    13731373        }
     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        }
    13741422}

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 @audrasjb
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.

#17 @audrasjb
7 weeks ago

  • Keywords needs-testing has-test-info added; 2nd-opinion removed

​@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.

  • 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.

​@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.

@arkaprabhachowdhury
6 weeks ago

WordPress Playground PR #13246: Quick Edit saved a password-protected post without retaining sticky status.

#20 @arkaprabhachowdhury
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:

  1. Opened Posts > Quick Edit for the published “Hello world!” post.
  2. Entered a password.
  3. Checked “Make this post sticky”.
  4. 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(): 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.

​GwendolinePicquet commented on ​PR #13246:


3 weeks ago
#22

https://github.com/user-attachments/assets/469f0e07-1400-45a8-bcdc-be651dbbf6b9
At this day, the bug is like that (I'm a begginer).

​@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 @therssoftware
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

  1. 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).
  2. 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.
  3. 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_visibility on trunk failed: ` 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.php pass cleanly:
      • test_edit_post_unsticks_a_post_when_a_password_is_set_without_visibility passed.
      • test_edit_post_clears_the_password_when_visibility_is_public passed.
      • test_edit_post_unsticks_the_post_when_visibility_is_password passed.
      • test_edit_post_privatises_and_unsticks_the_post_when_visibility_is_private passed.
      • test_edit_post_keeps_an_explicit_visibility_over_an_inferred_one passed.
    • All 81 tests in tests/phpunit/tests/post.php continue to pass.
    • PHPCS with WordPress-Core ruleset 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.

Note: See TracTickets for help on using tickets.