WordPress.org

Make WordPress Core

Opened 3 weeks ago

Last modified 13 hours ago

#43986 new task (blessed)

Disable "Install Plugin" button for PHP required version mismatch

Reported by: schlessera Owned by:
Milestone: 5.0 Priority: normal
Severity: major Version:
Component: Plugins Keywords: needs-unit-tests servehappy dev-feedback has-patch
Focuses: Cc:

Description

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

As a first step in enforcing the "Requires PHP" plugin header information, we should disable the "Install" (plugin) button while displaying a notice with the reason why.

In contrast to blocking updates (which has infrastructure & security" implications), disabling the "Install" button is relatively easy to do, provides most of the benefit of the "Requires PHP" header from all new and future installs, and most of all, provides a very real and convincing incentive for actually working on upgrading their servers.

While any installation path should be properly blocked based on this header, just disabling the "Install" button already makes sure we stop creating more problematic installations sites in the future.

Attachments (29)

WHIP core - add plugin page.png (268.1 KB) - added by schlessera 3 weeks ago.
Mockup by @hedgefiled: Incompatible plugins cannot be installed and show up red in the overview. Used the WP colors #FBEAEA for the background and #DC3232 for the button.
WHIP core - plugin detail backend.png (383.6 KB) - added by schlessera 3 weeks ago.
Mockup by @hedgefiled: The plugin directory page in the backend can show a warning if the detected PHP version is too low, and the install button is red here too. I didn't edit the statistics in this one because it's still using the old plugin directory theme (see #40971)
WHIP core - plugin detail backend2 (1).png (430.1 KB) - added by schlessera 3 weeks ago.
Mockup by @hedgefiled: I updated the More Details modal with the new plugin directory theme, made the PHP warning more fierce (because it's not a warning) and added more messaging next to the button, which I changed into a label, because it shouldn't be clickable.
WHIP core - add plugin page2.png (260.7 KB) - added by schlessera 3 weeks ago.
Mockup by @hedgefield: And an alternative take on the Add Plugins page without the red tint and the button turned into a label. I used the compatibility check in the card's footer to put the PHP warning, since it doesn't matter if the plugin is compatible with Wordpress if it's not compatible with your PHP.
43986.diff (1.8 KB) - added by afragen 12 days ago.
screenshot_01.png (62.3 KB) - added by afragen 12 days ago.
This is what patch will display.
43986-1.diff (1.8 KB) - added by afragen 12 days ago.
This should cleanly apply.
43986-2.diff (1.7 KB) - added by afragen 5 days ago.
Updated patch to account for results of add plugin search
43986-3.diff (1.7 KB) - added by afragen 5 days ago.
Just a fix for proper spacing. No other differences from 43986-2.diff
screenshot_01.2.png (64.9 KB) - added by afragen 5 days ago.
Current PHP version information added
screenshot_02.png (65.8 KB) - added by afragen 5 days ago.
More information here
screenshot_01.3.png (66.2 KB) - added by afragen 5 days ago.
This uses a new action hook to add text to the plugin card bottom. Also removes info below disabled install link.
43986v2-1.diff (2.8 KB) - added by afragen 5 days ago.
This patch removes the extra stuff from the plugin action links and adds it to the bottom of the plugin card via a new action hook.
screenshot_01.4.png (45.4 KB) - added by johnalarcon 39 hours ago.
screenshot_02.2.png (62.6 KB) - added by afragen 35 hours ago.
43986v2-2.diff (5.5 KB) - added by afragen 35 hours ago.
Simplifies message and adds color, but CSS needs to be built.
screenshot_03.png (61.5 KB) - added by afragen 34 hours ago.
minimal words
43986v2-3.diff (5.4 KB) - added by afragen 34 hours ago.
This patch has fewest words.
43986v2-4.diff (5.5 KB) - added by afragen 33 hours ago.
Oops, now includes something if PHP is compatible. PHP shown as compatible if no requirement listed.
43986.2.plugin-list.PNG (72.6 KB) - added by SergeyBiryukov 30 hours ago.
43986.2.more-details.PNG (67.4 KB) - added by SergeyBiryukov 30 hours ago.
43986.2.diff (7.8 KB) - added by SergeyBiryukov 30 hours ago.
43986.3.diff (6.8 KB) - added by SergeyBiryukov 29 hours ago.
43986.3.plugin-list.PNG (70.9 KB) - added by SergeyBiryukov 29 hours ago.
43986.3.more-details.PNG (67.0 KB) - added by SergeyBiryukov 29 hours ago.
screenshot_02.3.png (414.3 KB) - added by afragen 14 hours ago.
more details of 43986v2-5.diff
43986v2-5.diff (9.5 KB) - added by afragen 14 hours ago.
add aria-labels everywhere, fixes the 'more details' iframe
43986v2-6.diff (9.5 KB) - added by afragen 12 hours ago.
fixed aria-label text for WordPress compatibility to make more sense
43986v2-7.diff (9.5 KB) - added by afragen 9 hours ago.
improve wording of aria-labels

Change History (61)

@schlessera
3 weeks ago

Mockup by @hedgefiled: Incompatible plugins cannot be installed and show up red in the overview. Used the WP colors #FBEAEA for the background and #DC3232 for the button.

@schlessera
3 weeks ago

Mockup by @hedgefiled: The plugin directory page in the backend can show a warning if the detected PHP version is too low, and the install button is red here too. I didn't edit the statistics in this one because it's still using the old plugin directory theme (see #40971)

@schlessera
3 weeks ago

Mockup by @hedgefiled: I updated the More Details modal with the new plugin directory theme, made the PHP warning more fierce (because it's not a warning) and added more messaging next to the button, which I changed into a label, because it shouldn't be clickable.

@schlessera
3 weeks ago

Mockup by @hedgefield: And an alternative take on the Add Plugins page without the red tint and the button turned into a label. I used the compatibility check in the card's footer to put the PHP warning, since it doesn't matter if the plugin is compatible with Wordpress if it's not compatible with your PHP.

#1 @afragen
3 weeks ago

Am I understanding the flow correctly?

The user’s Install page would need to survey every plugin listed on the page and query the API to get the requires data from readme.txt. Then it would have to check the user’s PHP version, and add the custom labels, buttons, etc.

Wouldn’t some sort of data return from the API that builds the page be needed?

#2 @flixos90
3 weeks ago

  • Keywords needs-patch needs-unit-tests servehappy added
  • Milestone changed from Awaiting Review to 5.0

#3 @afragen
3 weeks ago

Nevermind, I found the 'plugin_install_action_links' in the plugin card.

#4 @Luciano Croce
12 days ago

  • Keywords dev-feedback added
  • Severity changed from normal to major

@afragen
12 days ago

#5 @afragen
12 days ago

  • Keywords has-patch added; needs-patch removed

Here's an initial pass at displaying an inactive button and message when the PHP requirement is not met. It needs styling.

https://core.trac.wordpress.org/attachment/ticket/43986/43986.diff

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


12 days ago

@afragen
12 days ago

This is what patch will display.

@afragen
12 days ago

This should cleanly apply.

#7 @afragen
5 days ago

Somehow need to hook into AJAX search results.

OK, I've gotten this solved. New patch coming.

Last edited 5 days ago by afragen (previous) (diff)

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


5 days ago

@afragen
5 days ago

Updated patch to account for results of add plugin search

@afragen
5 days ago

Just a fix for proper spacing. No other differences from 43986-2.diff

#9 @joyously
5 days ago

I think it's a shame that the version number needed is known, but not shown to the user. Wouldn't it be better to tell them which version they need to upgrade to in order to use that plugin?

#10 @afragen
5 days ago

Where would you like to see the current PHP version number?

All the messages are very much open for discussion. I simply put in something similar to the proposed design.

@afragen
5 days ago

Current PHP version information added

#11 @afragen
5 days ago

@joyously we could do something like the above.

@afragen
5 days ago

More information here

#12 @afragen
5 days ago

Sorry @joyously I misread your comment. See the last screenshot as an example.

#13 @afragen
5 days ago

Perhaps only displaying the required PHP version would be simplest.

#14 @joyously
5 days ago

@afragen I think that is too much to put below the button. Can you put it where the WP compatibility line is instead, like in the screenshot schlessera showed?

#15 @afragen
5 days ago

I don’t think there’s no current hook in that location, but I’ll look into adding one if it doesn’t exist.

@afragen
5 days ago

This uses a new action hook to add text to the plugin card bottom. Also removes info below disabled install link.

#16 @afragen
5 days ago

@joyously how about this one 👆

@afragen
5 days ago

This patch removes the extra stuff from the plugin action links and adds it to the bottom of the plugin card via a new action hook.

#17 @Luciano Croce
5 days ago

@afargen Please: consider to add the number of Downloads. Thx!

#18 follow-up: @afragen
5 days ago

@Luciano-Croce I’m pretty sure that’s not in scope of this ticket.

Last edited 5 days ago by afragen (previous) (diff)

#19 in reply to: ↑ 18 @Luciano Croce
4 days ago

Replying to afragen:

@Luciano-Croce I’m pretty sure that’s not in scope of this ticket.

Yes, of course, but I’m pretty sure that’s since this modification is open, it could be added without having to open another ticket, something more to keep in mind in this "refactoring".

Have a nice day.

Thanks.

Last edited 4 days ago by Luciano Croce (previous) (diff)

#20 @johnalarcon
39 hours ago

If there's too much text, it won't get read. Or, if it seems jammed together, the aesthetic suffers (and it also probably wouldn't get read). It also doesn't seem necessary to tell users what version of PHP or WordPress they're running - if they don't meet the requirement, they don't meet them. I think stating the requirements and using checks-n-exes would be a cleaner approach. See ss above. That little block of requirements could be moved around wherever, and still probably fit nicely.

Last edited 39 hours ago by johnalarcon (previous) (diff)

#21 @afragen
35 hours ago

@johnalarcon I can do something pretty close to that if that's the consensus.

I think we need a lot more input before a final decision.

@joyously what do you think?

@afragen
35 hours ago

Simplifies message and adds color, but CSS needs to be built.

#22 @johnalarcon
34 hours ago

The word "required" for PHP version is redundant under a "Requirements" header. Also, WordPress version doesn't have a "v", but PHP version does...I'd nix it for both - people know what these numbers are. I think it looks quite nice and (the reqs info) could even fit in nicely under the More Details link, which would bring the footer back to a reasonable size.

#23 @afragen
34 hours ago

I’ll adjust the patch accordingly.

I actually like the placement and it’s technically the “compatibility” section of the card.

@afragen
34 hours ago

minimal words

@afragen
34 hours ago

This patch has fewest words.

#24 @afragen
34 hours ago

Now if you're running trunk, you still get the standard Untested message.

Yes, the CSS still needs to be built.

I'm really liking this version.

@afragen
33 hours ago

Oops, now includes something if PHP is compatible. PHP shown as compatible if no requirement listed.

#25 @afragen
33 hours ago

Might consider using a question mark for untested so it would look like ? WordPress. Some other character might be more appropriate but I'm not sure how to figure out the best one.

#26 @johnalarcon
31 hours ago

That looks really nice! Yeah, re: untested... I'd lean toward a question mark, as well.

#27 @SergeyBiryukov
30 hours ago

43986.2.diff is my take on the patch, mostly following @hedgefield's mockups.

Screenshots: 43986.2.plugin-list.PNG, 43986.2.more-details.PNG.

I don't think we should change the UI too much. "Incompatible with your version of PHP. Learn more" seems enough for our purposes, and we can explain the details in "More Details" modal.

I didn't implement red buttons (went with regular disabled buttons instead), as we don't have them anywhere else in core, and I'd like to avoid introducing new design conventions here. Maybe even the pink background on plugin cards is too much? I'm open to suggestions though if someone has strong feelings either way :)

#28 @johnalarcon
30 hours ago

@SergeyBiryukov

Re: SS 43986.2-plugin-list.PNG, I agree about the pink background; it's a bit much.

Re: SS 43986.2-more-details.PNG, a couple of things.

The SS depicts a warning. Giving them a "notice" seems more appropriate. I feel that a warning would only be warranted if the installation was actually allowed to proceed - ie, there was some sort of risk or danger.

The text is a bit verbose. Does it need to say here that the plugin can't be installed? My feeling is the big disabled button communicates that chunk of the sentence in a place where users will more easily find it. That said, I couldn't pare the text down by much, but here's my thought:

Notice: This plugin requires PHP X.x or later. Please contact your web host or site admin for assistance in upgrading.

#29 @schlessera
29 hours ago

Some notes regarding the above patches (should apply equally to @afragen's & @SergeyBiryukov's patches):

  • You should account for the possibility of $plugins not containing the 'requires_php' index, to make the code more robust. If that index is not included, just assume it is compatible.
  • Information should not be coded in color or symbols only, because of a11y reasons. This applies to things like a pink background just as well as the colored symbols. As an example, the color-coded ✔️and ❌should contain invisible screen-reader-text so that screen readers can read out loud what is happening.

@SergeyBiryukov I think your logic is off. If the plugin does not contain the required headers, the code will default to blocking installations.

In terms of code, I think we have two different approaches here: action-based vs hard-coded. One is more flexible and looks cleaner, the other is a better enforcement. I'm not yet sure on which method I'd prefer...

#30 @SergeyBiryukov
29 hours ago

Thanks for the feeback! In 43986.3.diff:

  • Correct the logic to account for requires_php header not being present.
  • Change Warning label in More Details to Error, since that's what the message is.
  • Remove pink background from plugin cards.

#31 @afragen
24 hours ago

Currently, if requires_php is not present the value is present and defaults to false.

Do we want to display any information regarding WordPress compatibility? Currently the only notice is untested.

Thanks for the great feedback!

Last edited 24 hours ago by afragen (previous) (diff)

@afragen
14 hours ago

more details of 43986v2-5.diff

@afragen
14 hours ago

add aria-labels everywhere, fixes the 'more details' iframe

#32 @afragen
13 hours ago

@schlessera

minimal words

Now has aria-labels over the requirements text and other option is simply WordPress untested, also with aria-label.

more details of 43986v2-5.diff

Also has aria-labels.

I added a test for isset( $plugin['requires_php']) in the relevant functions for completeness.

I also added an additional filter hook to be able to replace the 'More Details' button.

As @SergeyBiryukov and I are going at this from 2 slightly different methods, perhaps we can discuss it more at the #core-php meeting?

The minimalist plugin card is growing on me. Especially since more verbiage is present in the More Details iframe.

@afragen
12 hours ago

fixed aria-label text for WordPress compatibility to make more sense

@afragen
9 hours ago

improve wording of aria-labels

Note: See TracTickets for help on using tickets.