WordPress.org

Make WordPress Core

Opened 3 weeks ago

Last modified 30 hours ago

#47644 new enhancement

Improve wording for PHP update warning in Site Health

Reported by: danieltj Owned by:
Milestone: 5.3 Priority: normal
Severity: minor Version: 5.2
Component: Site Health Keywords: has-patch 2nd-opinion needs-copy-review
Focuses: Cc:

Description

In the Site Health section of WordPress there is a warning about PHP being out of date needing to be updated to the latest version. This wording needs improvement for a number of reasons.

  1. It uses the word 'we' when no other checks use such language, this one thing is specifically personal and not uniform across other checks made.
  1. It doesn't include the version of PHP you are currently using. This can only be viewed under the 'Info' tab, this should be included in the check rather than forcing you to find out what it is.
  1. Most importantly, it doesn't include the PHP version you should be using. The development environment I'm currently using which I saw this on is using PHP 7.2.x. I thought it was the latest (and I'm not getting a PHP warning on the dashboard homepage). So it's rather confusing to be told it's out of date but not what version I really need.

Changing the wording from...

We recommend that you update PHP

to...

Your PHP version is out of date

This is inline with other checks such as:

  • Your WordPress version is up to date (5.2.2)
  • Your site can perform loopback requests
  • Your site is not set to output debug information

The paragraph of text further explaining the issue should also include the current PHP version (like it does in the WordPress check I've listed above) and the PHP version you need to update to.

Attachments (14)

wp-sitehealth-php-warning.png (60.9 KB) - added by danieltj 3 weeks ago.
The PHP check made in Site Health
47644.diff (609 bytes) - added by chetan200891 3 weeks ago.
Created initial patch.
wp-sh-php-73x.png (28.0 KB) - added by danieltj 2 weeks ago.
Running php version 7.3.x
wp-sh-php-72x.png (28.4 KB) - added by danieltj 2 weeks ago.
Running php version 7.2.x
wp-sh-php-71x.png (28.4 KB) - added by danieltj 2 weeks ago.
Running php version 7.1.x
wp-sh-php-56x.png (28.5 KB) - added by danieltj 2 weeks ago.
Running php version 5.6.x
47644.2.diff (2.4 KB) - added by danieltj 2 weeks ago.
Proposed patch for PHP test.
wp-sh-dj-patch-73x.png (30.2 KB) - added by danieltj 2 weeks ago.
Patch applied using PHP 7.3.x
wp-sh-dj-patch-72x.png (30.8 KB) - added by danieltj 2 weeks ago.
Patch applied using PHP 7.2.x
wp-sh-dj-patch-56x.png (30.8 KB) - added by danieltj 2 weeks ago.
Patch applied using PHP 5.6.x
47644.3.diff (2.6 KB) - added by garrett-eclipse 2 weeks ago.
Updated patch to introduce translator comments and update description verbiage to indicate it's the 'minimum recommended'
47644.3.revert-is_secure.diff (2.7 KB) - added by garrett-eclipse 2 weeks ago.
Proposing re-introducing the seperation of is_supported and is_secure
Screen Shot 2019-07-07 at 8.12.19 AM.png (79.4 KB) - added by garrett-eclipse 2 weeks ago.
When PHP 7.2 the update is only recommended
Screen Shot 2019-07-07 at 8.14.14 AM.png (79.1 KB) - added by garrett-eclipse 2 weeks ago.
When on PHP 7.1 the admin should update PHP

Download all attachments as: .zip

Change History (31)

@danieltj
3 weeks ago

The PHP check made in Site Health

#1 follow-up: @swissspidy
3 weeks ago

  • Keywords needs-patch added
  • Milestone changed from Awaiting Review to Future Release

Great to see you here again, Daniel!

These sound like some good improvements to me.

@chetan200891
3 weeks ago

Created initial patch.

This ticket was mentioned in Slack in #core by justinahinon. View the logs.


2 weeks ago

#3 @afragen
2 weeks ago

As long as we’re considering changing the language, can we somehow reference that the PHP version is lower than the current recommended version of PHP.

The reason is that currently the warning shows for supported versions like 7.2

#4 @garrett-eclipse
2 weeks ago

It would be nice as well if this verbiage showed you what your current version is and what the recommended version is.

#5 in reply to: ↑ description @SergeyBiryukov
2 weeks ago

Replying to danieltj:

It uses the word 'we' when no other checks use such language, this one thing is specifically personal and not uniform across other checks made.

Related: #46057

#6 in reply to: ↑ 1 @danieltj
2 weeks ago

Replying to swissspidy:

Great to see you here again, Daniel!

These sound like some good improvements to me.

Thanks Pascal 🤓

@afragen & @garrett-eclipse

I definitely think that it should include the current PHP version and the version you should update to, yes.

#7 @garrett-eclipse
2 weeks ago

Thanks @danieltj another place I've come across that could use PHP version information is when a plugin update is blocked due to minimum PHP not met. I didn't think it directly related here though so opened #47651

#8 @afragen
2 weeks ago

It should be easy enough to add text with phpversion(). We would simply need go some sort of blessing and a consensu about what the string would be.

#9 @JeffPaul
2 weeks ago

  • Milestone changed from Future Release to 5.3

#10 @danieltj
2 weeks ago

I've looked at the wording for this specific test and it seems the title for it changes depending on the version, but the description of the test stays the same.

I'll attach some screenshots of the different states that I could find.

@danieltj
2 weeks ago

Running php version 7.3.x

@danieltj
2 weeks ago

Running php version 7.2.x

@danieltj
2 weeks ago

Running php version 7.1.x

@danieltj
2 weeks ago

Running php version 5.6.x

#11 @danieltj
2 weeks ago

As you can see, the only instance where it states the current version is when you're running the latest one, that's not very helpful really. I'll see if I can do a patch with some proposed wording.

It's more about having a helpful title for the test as the description is largely fine.

@danieltj
2 weeks ago

Proposed patch for PHP test.

@danieltj
2 weeks ago

Patch applied using PHP 7.3.x

@danieltj
2 weeks ago

Patch applied using PHP 7.2.x

@danieltj
2 weeks ago

Patch applied using PHP 5.6.x

#12 follow-up: @danieltj
2 weeks ago

I took it upon myself to merge the is_supported and is_secure recommended checks together because to me they didn't seem to serve much of a purpose as 7.2.x and 7.1.x are both out of date PHP version but also both supported anyway so it made little sense to have a slightly different wording for something that is basically the same.

Saves translators doing an additional string. Sorry for so many replies at once!

@garrett-eclipse
2 weeks ago

Updated patch to introduce translator comments and update description verbiage to indicate it's the 'minimum recommended'

@garrett-eclipse
2 weeks ago

Proposing re-introducing the seperation of is_supported and is_secure

#13 in reply to: ↑ 12 @garrett-eclipse
2 weeks ago

  • Keywords has-patch 2nd-opinion added; needs-patch removed

Thanks @danieltj this is great. I see them being much more useful with the version clear and available even when unexpanded.

In 47644.3.diff I've made the following additions;

  • Translator comments;

Line 653 - translators: %s: The minimum recommended PHP version
Line 676 -
translators: %s: The server PHP version.
Line 687 - translators: %s: The server PHP version.
*This also caused the sprintf functions there to become multi-line.

  • Verbiage in description was updated for clarity as the value is the recommended_version and not the latest version (as there's more recent minors).

I went with - The minimum recommended version of PHP is %s.
*Used minimum recommended verbiage over just recommended as I would assume the most recent minor (7.3.7) would really be the recommendation but we're using the most recent major (7.3).

Replying to danieltj:

I took it upon myself to merge the is_supported and is_secure recommended checks together because to me they didn't seem to serve much of a purpose as 7.2.x and 7.1.x are both out of date PHP version but also both supported anyway so it made little sense to have a slightly different wording for something that is basically the same.

Saves translators doing an additional string. Sorry for so many replies at once!

I've also supplied 47644.3.revert-is_secure.diff as I feel there's a large difference between is_supported and is_secure and although it may seem slight the difference between the should and recommend verbiage is important.
https://www.php.net/supported-versions.php
Referring to the support matrix for PHP;

Active support (is_supported) - A release that is being actively supported. Reported bugs and security issues are fixed and regular point releases are made.
Security fixes only (is_secure) - A release that is supported for critical security issues only. Releases are only made on an as-needed basis.

Many government bodies and large companies have a policy of stability in software and stay on the active support versions but don't upgrade to the latest. For these users and many others they should upgrade when their version drops from active support (is_supported) to security fixes (is_secure), but when they are on an actively supported version while it's recommended they upgrade in many cases they probably shouldn't due to their policies.
Note: The reversion patch also has the translator comments and verbiage change.

Happy to discuss that point further.

Again, thanks for the patch @danieltj marked for testing and a 2nd-opinion on the above.

@garrett-eclipse
2 weeks ago

When PHP 7.2 the update is only recommended

@garrett-eclipse
2 weeks ago

When on PHP 7.1 the admin should update PHP

#14 @garrett-eclipse
13 days ago

From #core-site-health discussion today some notes;
@clorith;

It's on my radar to look over the input there, ideally I'd like to avoid moving numbers in the "headers", and instead introduce a second paragraph with the relevant numbers in the description to keep the information overload to a minimum whenever possible (edited)

@afragen;

Also, when listing the PHP requirement we need to avoid confusion between 7.3 and 7.3.x versions

@clorith;

Would it be better if we just show the <major>.<minor> and strip out the patch version to keep things simpler for the user, at least when making recommendations ?

@garrett-eclipse;

That makes sense, would suggest using ‘minimum recommended’ verbiage instead of just ‘recommended’ so users don’t just do the 7.3 and will feel ok to go with a 7.3.x

#15 follow-up: @danieltj
3 days ago

For what it's worth I don't think the version must be in the title, although it would add some consistency compared to other checks that are made which have versions in them.

I do think usage on the word 'we' needs to go though and the versions need to be included somewhere to say what you have and what you need to update to.

#16 in reply to: ↑ 15 @garrett-eclipse
2 days ago

  • Keywords needs-copy-review added

Replying to danieltj:

For what it's worth I don't think the version must be in the title, although it would add some consistency compared to other checks that are made which have versions in them.

I do think usage on the word 'we' needs to go though and the versions need to be included somewhere to say what you have and what you need to update to.

Thanks for the input @danieltj looking at other checks I see what you mean about version in the title as that's in the WP version check, on my up to date install it states Your WordPress version is up to date (5.2.2). I'm also with you on dropping the 'we' and agree we need to include what version someone is on and what they need to update too most likely in the description.

Adding 'needs-copy-review' to get thoughts on the use of 'we' and suggest copy for the description portion of the check.

#17 @SergeyBiryukov
30 hours ago

What's the reason for changing "build and maintain WordPress" to "power WordPress"? Maybe it's just me, but the former seems a bit easier to read and translate.

Note: See TracTickets for help on using tickets.