Make WordPress Core

Opened 21 months ago

Closed 17 months ago

Last modified 17 months ago

#55900 closed defect (bug) (fixed)

Installer screen, when submitting a faulty form the search engine checkbox is reset

Reported by: ramon-fincken's profile ramon fincken Owned by: audrasjb's profile audrasjb
Milestone: 6.1 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: has-screenshots has-patch
Focuses: ui Cc:


When submitting with an empty field, the search engine checkbox is reset to zero

Attachments (5)

screencapture-localhost-wordpress-make-src-wp-admin-install-php-2022-06-02-13_10_52.png (129.1 KB) - added by ramon fincken 21 months ago.
Checked before submit
screencapture-localhost-wordpress-make-src-wp-admin-install-php-2022-06-02-13_11_54.png (106.2 KB) - added by ramon fincken 21 months ago.
Unchecked after submit
55900.1.patch (776 bytes) - added by ramon fincken 21 months ago.
patch 55900.1
55900.2.patch (1011 bytes) - added by rafiahmedd 21 months ago.
I just found a better way to do this.
55900.diff (599 bytes) - added by khokansardar 21 months ago.
Here is the proper PR of the task

Download all attachments as: .zip

Change History (32)

#1 @audrasjb
21 months ago

  • Focuses ui added
  • Milestone changed from Awaiting Review to 6.1
  • Owner set to whaze
  • Status changed from new to assigned
  • Version trunk deleted

Hello @ramon-fincken and thanks for the ticket!

Let's fix this in the next major :)

Assigning the ticket to @whaze who volunteered to handle this during WCEU contributor day!

#2 @ramon fincken
21 months ago

Prepping patch file now :)

@ramon fincken
21 months ago

patch 55900.1

#3 @ramon fincken
21 months ago

  • Keywords has-patch added; needs-patch removed

#5 @whaze
21 months ago

Hello from contributor's day,
i added a PR

This ticket was mentioned in PR #2772 on WordPress/wordpress-develop by itzmekhokan.

21 months ago

Trac ticket: 55900

21 months ago

I just found a better way to do this.

21 months ago

Here is the proper PR of the task

#7 @ramon fincken
21 months ago

I verified that the checked function does NOT work for this one. I agree that the shorter PHP is better. I will make a new patch.

3 cases:
1) first open of the page (should NOT be checked)
2) submit when checked (should be checked)
3) submit when NOT checked (should be NOT checked)

#8 @ramon fincken
21 months ago

update: I cannot get to work the 2nd patch, so sticking with the first one

the checked function is really funky as we need to test something that has value === 0

#10 @khokansardar
21 months ago

Hi @ramon-fincken please check with my patch that I have already added. Its working and I guess its a proper PR for this ticket.

#11 @audrasjb
21 months ago

PR2777 looks good to me:
Are we good to ship this version of the patch?

#13 @audrasjb
21 months ago

  • Keywords needs-testing added

Hey there,
I refreshed PR2777 so it passes unit tests.

This PR includes the conditional statement proposed by @ramon-fincken.
Looks like it works in the 3 cases listed by Ramon.

#14 @khokansardar
21 months ago

  • Keywords changes-requested dev-feedback 2nd-opinion added

Hey I think instead of checking again $_POST['blog_public'] and storing it in $checked variable and print it, we should modify the variable $blog_public when it's defined and assign the value based on $_POST['blog_public'] data.
So as per logic it should be something like my above PR -
This PR also works in the 3 cases listed by Ramon.

#15 @costdev
19 months ago

  • Keywords 2nd-opinion removed

I concur with @khokansardar's suggestion that PR 2772 is a better approach.

It's also worth noting that when the blog_privacy_selector action exists, PR 2777 will currently fail to maintain the previously selected option.

This would require

// PR 2777: Line 198
$checked = ( isset( $_POST['blog_public'] ) && $blog_public ) ? ' checked="checked"' : '';

to be moved to ~Line 186 and $checked also used on Lines 189 and 191. Rather than making multiple changes here, PR 2772 makes a single change that works with and without the blog_privacy_selector action being used.

To test the blog_privacy_selector, you can drop this into a Must-Use plugin prior to visiting the install screen.


 * Plugin Name: Blog Privacy Selector
 * Description: Enables radio options for search engine visibility.
 * Author:      WordPress Core Contributors
 * Author URI:
 * License:     GPLv2 or later
 * Version:     1.0.0

// Comment this out to test without the blog privacy selector.
add_action( 'blog_privacy_selector', '__return_empty_string' );
Last edited 19 months ago by costdev (previous) (diff)

This ticket was mentioned in Slack in #core-auto-updates by costdev. View the logs.

18 months ago

#17 @costdev
18 months ago

@audrasjb If you have time, can you take a look at my comment above and let me know your thoughts? Thanks!

#18 @Ankit K Gupta
17 months ago

Hello All, I tested this issue from a user perspective and used PR . This PR fixed the issue and Installation screen is working as expected.

Test Report


Test result

Working expected with PR Search engine checkbox appears as selected (if user checked it) after the validation occurs.

Screencast example:

Before Fix -

After Fix -

#19 @audrasjb
17 months ago

  • Keywords needs-testing changes-requested dev-feedback removed

Ok the logic in PR2772 looks good to me.

Marking as ready for commit (also, self assigning).

#20 @audrasjb
17 months ago

  • Keywords commit added
  • Owner changed from whaze to audrasjb
  • Status changed from assigned to accepted

#21 @audrasjb
17 months ago

  • Keywords commit removed

Hmm. Looking closely, we should probably escape/cast the POST value here?

$blog_public = isset( $_POST['blog_public'] ) ? $_POST['blog_public'] : $blog_public;

#22 @khokansardar
17 months ago

Hi @audrasjb I have updated my PR and added value cast for POST data, here you can check -

#23 @audrasjb
17 months ago

Looks good to me, thanks!

#25 @audrasjb
17 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 54326:

Upgrade/Install: Keep search engine checkbox value when reloading the Install screen.

This changeset ensures the value of the search engine checkbox is not reset to its default value when a faulty form is sent in the Install screen.

Props ramon-fincken, audrasjb, whaze, rafiahmedd, khokansardar, costdev, ankit-k-gupta.
Fixes #55900.

#26 @audrasjb
17 months ago

Yes @ramon-fincken as far as I know, I tested it and it passes the 3 test cases.

Note: See TracTickets for help on using tickets.