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

[ramon fincken]

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

[ramon fincken]

Checked before submit

[ramon fincken]

Unchecked after submit

[audrasjb]

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!

[ramon fincken]

Prepping patch file now :)

[ramon fincken]

patch 55900.1

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

[whaze]

Hello from contributor's day,
i added a PR

Trac ticket: 55900

[rafiahmedd]

I just found a better way to do this.

[khokansardar]

Here is the proper PR of the task

[ramon fincken]

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)

[ramon fincken]

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

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

[khokansardar]

Hi @ramon-fincken please check with my patch ​https://patch-diff.githubusercontent.com/raw/WordPress/wordpress-develop/pull/2772.diff that I have already added. Its working and I guess its a proper PR for this ticket.

[audrasjb]

PR2777 looks good to me: ​https://github.com/WordPress/wordpress-develop/pull/2777
Are we good to ship this version of the patch?

[khokansardar]

@audrasjb have you checked this one ​https://github.com/WordPress/wordpress-develop/pull/2772

[audrasjb]

Hey there,
I refreshed PR2777 so it passes unit tests.
​https://github.com/WordPress/wordpress-develop/pull/2777

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

[khokansardar]

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 - ​https://github.com/WordPress/wordpress-develop/pull/2772.
This PR also works in the 3 cases listed by Ramon.

[costdev]

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

<?php
// 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.

<?php

/**
 * Plugin Name: Blog Privacy Selector
 * Description: Enables radio options for search engine visibility.
 * Author:      WordPress Core Contributors
 * Author URI:  https://make.wordpress.org/core
 * 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' );

[costdev]

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

[Ankit K Gupta]

Hello All, I tested this issue from a user perspective and used PR ​https://github.com/WordPress/wordpress-develop/pull/2772 . This PR fixed the issue and Installation screen is working as expected.

Test Report

Env

• WordPress - 6.1-beta1-54282
• Chrome Version - 105.0.5195.125 (Official Build) (x86_64)
• OS - macOS Monterey V12.3.1
• Theme: Twenty Twenty Two
• PHP - 8.0.0
• Web Server - Apache
• Database - MySQL 5.7.28
• Used PR file - ​https://github.com/WordPress/wordpress-develop/pull/2772

Test result

Working expected with PR ​https://github.com/WordPress/wordpress-develop/pull/2772. Search engine checkbox appears as selected (if user checked it) after the validation occurs.

Screencast example:

Before Fix -

After Fix -

[audrasjb]

Ok the logic in PR2772 looks good to me.

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

[audrasjb]

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

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

[khokansardar]

Hi @audrasjb I have updated my PR and added value cast for POST data, here you can check - ​https://github.com/WordPress/wordpress-develop/pull/2772/

[audrasjb]

Looks good to me, thanks!

[ramon fincken]

does it pass all 3 testcases?

see them here https://core.trac.wordpress.org/ticket/55900#comment:7

[audrasjb]

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.

[audrasjb]

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

Commited in https://core.trac.wordpress.org/changeset/54326