WordPress.org

Make WordPress Core

Opened 17 months ago

Closed 8 months ago

Last modified 5 months ago

#44350 closed task (blessed) (fixed)

Block plugin updates if required PHP version is not supported - Updates screen

Reported by: flixos90 Owned by: afragen
Milestone: 5.2 Priority: normal
Severity: normal Version:
Component: Site Health Keywords: servehappy has-patch commit
Focuses: Cc:
PR Number:

Description

Note: This ticket is a subtask for the overarching #40934 ticket.

When a plugin states it requires a specific minimum PHP version through its "Requires PHP" header information and the server does not support this PHP version, WordPress should block any possibility to update the plugin. This way, plugins will stay at the latest release that still supports the server's PHP version.

This ticket's goal is to prevent plugin updates from the general "Updates" admin screen. With that, it complements #43987, which deals with preventing plugins from the "Plugins" admin screen.

Attachments (18)

44350.diff (1.7 KB) - added by afragen 17 months ago.
For update-core.php
screenshot_06.png (62.3 KB) - added by afragen 17 months ago.
Checkbox gone and notice listed.
44350.2.diff (2.6 KB) - added by afragen 17 months ago.
feedback applied
screenshot_04.png (56.3 KB) - added by afragen 17 months ago.
44350.3.diff (2.6 KB) - added by afragen 17 months ago.
changed messaging text from 'upgrading' to 'updating'
44350.4.diff (2.7 KB) - added by miyauchi 16 months ago.
Improve escaping
44350.5.diff (2.7 KB) - added by afragen 16 months ago.
more escaping improvements
44350.6.diff (2.7 KB) - added by afragen 16 months ago.
fixed some escaping for esc_html not esc_attr
44350.7.diff (2.6 KB) - added by afragen 10 months ago.
Updated patch for post 5.0 merge, fixed link to update-php
44350.8.diff (5.0 KB) - added by afragen 10 months ago.
fix version_compare so 5.1.10 is greater than 5.1.2
44350.9.diff (4.7 KB) - added by afragen 10 months ago.
update patch for new functions wp_get_update_php_url() and wp_update_php_annotation()
44350.10.diff (2.3 KB) - added by afragen 10 months ago.
updated patch. This will require #46044 to be committed first as it uses new function from that ticket
44350.11.diff (2.3 KB) - added by afragen 8 months ago.
44350.12.diff (2.3 KB) - added by afragen 8 months ago.
close then open p tag
44350.13.diff (1.8 KB) - added by desrosj 8 months ago.
44350.14.diff (2.3 KB) - added by afragen 8 months ago.
change to <br><em>....</em>
incompatible-update-filtered-links.png (319.9 KB) - added by desrosj 8 months ago.
incompatible-update-default.png (296.3 KB) - added by desrosj 8 months ago.

Download all attachments as: .zip

Change History (53)

This ticket was mentioned in Slack in #core-php by flixos90. View the logs.


17 months ago

#2 @flixos90
17 months ago

  • Owner set to afragen
  • Status changed from new to assigned

@afragen
17 months ago

For update-core.php

@afragen
17 months ago

Checkbox gone and notice listed.

#3 @afragen
17 months ago

44350.diff for removing ability to update from update-core.php page.

The following screenshot shows the checkbox missing and therefore not possible to update along with a notice regarding PHP updating.

Checkbox gone and notice listed.

#4 @afragen
17 months ago

  • Keywords dev-feedback has-patch added; needs-patch removed

Are we also looking to disable updates if the WP version is not supported?

#5 @afragen
17 months ago

The last discussion on Slack was to change the text to

This plugin doesn’t work with your version of PHP. [Learn more about updating PHP]

I’ll update the patch accordingly unless there are other suggestions.

@schlessera @flixos90 @SergeyBiryukov any other thoughts?

@afragen
17 months ago

feedback applied

#6 @afragen
17 months ago

Updated patch 44350.2.diff with feedback incorporated.


Not much difference except for updating -> upgrading

This ticket was mentioned in Slack in #core-php by afragen. View the logs.


17 months ago

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


17 months ago

@afragen
17 months ago

changed messaging text from 'upgrading' to 'updating'

#9 @afragen
17 months ago

These patches are dependent upon some changes in the patch for #43987 for the View Details window to display a Cannot Update button.

This ticket was mentioned in Slack in #core-php by sergey. View the logs.


16 months ago

@miyauchi
16 months ago

Improve escaping

#11 @afragen
16 months ago

Translatable strings in core don’t generally seem to be escaped. Just my observation as core translations are more thoroughly vetted.

#12 follow-up: @miyauchi
16 months ago

Thanks @afragen

I thought properties of the $plugin_data like $plugin_data->Name should be escaped. :)

#13 @afragen
16 months ago

Core seems to be a slightly different case and I don’t honestly know the answer. I’m certain at some point we’ll get some feedback from @flixos90 or @SergeyBiryukov

Version 0, edited 16 months ago by afragen (next)

#14 in reply to: ↑ 12 @afragen
16 months ago

Replying to miyauchi:

Thanks @afragen

I thought properties of the $plugin_data like $plugin_data->Name should be escaped. :)

If you want to escape the properties, you might use something like esc_attr( $plugin_data )

@afragen
16 months ago

more escaping improvements

#15 @miyauchi
16 months ago

Hi @afragen

esc_attr() should be used to escape attributes for HTML.
In this case, I think it should esc_html().

But I agree to escape the property only as your patch. :)

Thanks. :)

@afragen
16 months ago

fixed some escaping for esc_html not esc_attr

This ticket was mentioned in Slack in #core-php by afragen. View the logs.


16 months ago

This ticket was mentioned in Slack in #core-php by afragen. View the logs.


15 months ago

#18 @afragen
14 months ago

@schlessera @flixos90 should this be milestone 4.9.9?

#19 @afragen
13 months ago

Probably need to milestone this for 5.1

#20 @flixos90
13 months ago

  • Milestone changed from 5.0 to 5.1

@afragen
10 months ago

Updated patch for post 5.0 merge, fixed link to update-php

This ticket was mentioned in Slack in #core-php by afragen. View the logs.


10 months ago

This ticket was mentioned in Slack in #core-php by afragen. View the logs.


10 months ago

@afragen
10 months ago

fix version_compare so 5.1.10 is greater than 5.1.2

#23 @afragen
10 months ago

There is clearly overlap with 43987 and this patch. Also, may need to lint the patch. Sorry. Not sure everyone wants to see yet another iteration.

#24 @afragen
10 months ago

We probably need to change milestone to 5.2

Last edited 10 months ago by afragen (previous) (diff)

@afragen
10 months ago

update patch for new functions wp_get_update_php_url() and wp_update_php_annotation()

#25 @afragen
10 months ago

@flixos90 I need to revise the wp_update_php_annotation() to wp_get_update_php_annotation() and update this patch too. I found that I really do need to return the annotation and not simply echo it for this patch to work.

I'll make a new ticket for the function rename. #46044

Last edited 10 months ago by afragen (previous) (diff)

#26 @flixos90
10 months ago

  • Milestone changed from 5.1 to 5.2

@afragen
10 months ago

updated patch. This will require #46044 to be committed first as it uses new function from that ticket

#27 follow-up: @TimothyBlynJacobs
8 months ago

Should this use the compatibility functions from #43992?

I don't see why escaping the plugin name is required here. It isn't done elsewhere in core. The Version also isn't escaped, see https://github.com/WordPress/wordpress-develop/blob/6d2f78d9bacf931fb9d4ba031e135c4eb5b17713/src/wp-admin/includes/class-wp-plugins-list-table.php#L802. And the new_version isn't either, see in that same file when building the view details link.

I think a committer needs to look at that.

If we do want escaping, then I believe, those should be esc_html. esc_attr is used elsewhere in that file because the final destination is in href tags.

#28 in reply to: ↑ 27 @afragen
8 months ago

I was planning on a new ticket/patch for utilizing the new is_wp_compatible() and is_php_compatible() later. I didn't want to include them here as I was concerned someone testing the patch would encounter an error that they couldn't easily explain.

As for escaping, I was simply trying to escape before output. If these don't need escaping; I agree that's something for a core-committer to decide.

Replying to TimothyBlynJacobs:

Should this use the compatibility functions from #43992?

I don't see why escaping the plugin name is required here. It isn't done elsewhere in core. The Version also isn't escaped, see https://github.com/WordPress/wordpress-develop/blob/6d2f78d9bacf931fb9d4ba031e135c4eb5b17713/src/wp-admin/includes/class-wp-plugins-list-table.php#L802. And the new_version isn't either, see in that same file when building the view details link.

I think a committer needs to look at that.

If we do want escaping, then I believe, those should be esc_html. esc_attr is used elsewhere in that file because the final destination is in href tags.

@afragen
8 months ago

#29 @desrosj
8 months ago

#46269 was marked as a duplicate.

@afragen
8 months ago

close then open p tag

@desrosj
8 months ago

#30 @desrosj
8 months ago

  • Keywords needs-testing added; dev-feedback removed

In 44350.13.diff:

  • It seems like the closing <p> is not needed when displaying the annotation here. Also, the .description class was causing light gray text on white, which was not good for a11y. Changing to a <span class="description"> fixes the mismatched HTML and the contrast issues.
  • I removed the added output escaping. The $plugin_data variable is populated by _get_plugin_data_markup_translate(), which passes each value through wp_kses() with a list of allowed tags for that attribute.

#31 @afragen
8 months ago

@desrosj I thought we were changing the class=“description” to an em?

@afragen
8 months ago

change to <br><em>....</em>

#32 @desrosj
8 months ago

  • Keywords commit added

Thanks, @afragen. I was tinkering with the markup a bit on that and included the wrong change.

I attached some screenshots to show what the notices look like now with the final patch.

Also, it looks like one of the escaping calls were re-added in 44350.14.diff (I can remove this before committing). But, otherwise it looks good.

#33 @desrosj
8 months ago

  • Keywords needs-unit-tests needs-testing removed

#34 @desrosj
8 months ago

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

In 44939:

Plugins: Block plugin updates on the update screen if required PHP version is not supported.

This brings the changes in [44937] to the WordPress Updates page in the admin. Now, when a site does not meet the minimum PHP version requirements for a plugin update, the user will not be able to update. Instead, they will be presented with educational information to guide them through the process of updating PHP.

Props afragen, miyauchi, TimothyBlynJacobs, desrosj.
Fixes #44350.

#35 @spacedmonkey
5 months ago

  • Component changed from Plugins to Site Health
Note: See TracTickets for help on using tickets.