WordPress.org

Make WordPress Core

Opened 8 months ago

Last modified 5 days ago

#48491 reviewing enhancement

[Theme compatibility] WP/PHP compatibility tests for single site theme updates/activation

Reported by: afragen Owned by: SergeyBiryukov
Milestone: 5.5 Priority: normal
Severity: normal Version:
Component: Site Health Keywords: has-patch needs-testing dev-feedback servehappy needs-copy-review has-screenshots needs-dev-note
Focuses: administration Cc:

Description

As part of the WP/PHP version compatibility testing for themes, this ticket is directed towards theme updating and activation on a single site installation.

If the theme is not compatible with the currently running WP or PHP version, updating will be blocked. This also includes blocking theme activation for incompatible themes.

This addresses the themes.php and upgrade-core.php pages.

Attachments (17)

48491.diff (13.2 KB) - added by afragen 8 months ago.
screenshot_02.png (149.1 KB) - added by afragen 8 months ago.
screenshot_04.png (201.2 KB) - added by afragen 8 months ago.
screenshot_01.png (49.8 KB) - added by afragen 8 months ago.
48491.2.diff (13.2 KB) - added by afragen 8 months ago.
Precedence: API > readme.txt > style.css
48491.3.diff (14.2 KB) - added by afragen 8 months ago.
Disable 'Live Preview' button
48491.4.patch (11.1 KB) - added by afragen 8 months ago.
removed changes to class-wp-theme.php, now in #48515
48491.4.diff (10.8 KB) - added by jorbin 5 months ago.
customizer-popup.png (1.0 MB) - added by afragen 4 months ago.
customizer-activate.png (42.4 KB) - added by afragen 4 months ago.
customizer-browser.png (471.4 KB) - added by afragen 4 months ago.
48491.5.diff (19.2 KB) - added by afragen 4 months ago.
add Customizer part
48491.6.diff (19.6 KB) - added by afragen 4 months ago.
minor updates to patch
48491.7.diff (19.7 KB) - added by afragen 4 months ago.
update 'Activate & Publish' back to button type
admin-themes-incompatible.png (149.8 KB) - added by desrosj 5 days ago.
admin-themes-overlay-incompatible.png (667.7 KB) - added by desrosj 5 days ago.
customizer-incompatible-theme.png (681.0 KB) - added by desrosj 5 days ago.

Change History (61)

#1 @afragen
8 months ago

Still to do are tickets/patches for installing themes and multisite.

#2 @afragen
8 months ago

Related: #44592

#3 @SergeyBiryukov
8 months ago

  • Milestone changed from Awaiting Review to 5.4

@afragen
8 months ago

#4 @afragen
8 months ago

The screenshots screenshot_02.png, screenshot_04.png, and screenshot_01.png show an incompatible update.

I manually set the PHP requirement so it would fail.

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

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


8 months ago

@afragen
8 months ago

Precedence: API > readme.txt > style.css

#6 @afragen
8 months ago

Hmm, I think I need to deactivate the Live Preview button as it can be used to activate.

#7 @afragen
8 months ago

It is still possible, using the Customizer, to see updates and activate a non-compliant theme. I will need help with fixing this component.

#8 @afragen
8 months ago

  • Keywords has-screenshots added

@afragen
8 months ago

Disable 'Live Preview' button

#9 @afragen
8 months ago

It is my plan to clean up this a bit. My intent is to remove the changes to WP_Theme and put them in their own ticket/patch. This patch would be the first patch needed to be reviewed/committed as it will be required by all these subsequent tickets/patches.

That way this ticket doesn't need to be committed before any of the others. Hopefully this simplifies the testing.

@afragen
8 months ago

removed changes to class-wp-theme.php, now in #48515

#10 @SergeyBiryukov
7 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

#11 @sathyapulse
7 months ago

@afragen I am not able to apply the patch in my local environment. Could you please regenerate the patch?

#12 @afragen
7 months ago

@sathyapulse it's likely because the patch was created against core.git.wordpress.org, not develop.git.wordpress.org.

#13 @afragen
7 months ago

@sathyapulse I've tested the patch again and it is applies cleanly on trunk using npm run grunt patch:48491

It might depend upon how your local environment is set up.

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


6 months ago

This ticket was mentioned in Slack in #core-site-health by carike. View the logs.


6 months ago

This ticket was mentioned in Slack in #core-site-health by clorith. View the logs.


5 months ago

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


5 months ago

#18 @afragen
5 months ago

Pinging @jorbin per our discussion at WCPHX.

#19 @SergeyBiryukov
5 months ago

Just noting this is on my list to commit today (along with #48507), more eyes definitely welcome though :)

#20 @jorbin
5 months ago

I'm taking a look now, but that doesn't need to block merging by any means

@jorbin
5 months ago

#21 @jorbin
5 months ago

Uploaded update that applies cleanly, still testing

#22 @jorbin
5 months ago

I think this is going to need the customizer piece in order to be complete and land in trunk.

Based on timing, I would suggest punting and work continuing so it can be ready for 5.5

#23 @TimothyBlynJacobs
5 months ago

For the customizer stuff, it looks like this data can be added in the wp_ajax_query_themes() PHP function. And adding a modification like this in the tmpl-theme-preview template in wp-admin/theme-install.php.

<# if ( data.installed ) { #>
    <a class="button button-primary activate" href="{{ data.activate_url }}"><?php _e( 'Activate' ); ?></a>
<# } else if ( data.compatible_php ) { #>
    <a href="{{ data.install_url }}" class="button button-primary theme-install" data-name="{{ data.name }}" data-slug="{{ data.id }}"><?php _e( 'Install' ); ?></a>
<# } else { #>
    <a href="{{ data.install_url }}" class="button button-primary theme-install disabled" data-name="{{ data.name }}" data-slug="{{ data.id }}"><?php _e( 'Install' ); ?></a>
<# } #>

#24 @Clorith
5 months ago

  • Milestone changed from 5.4 to 5.5

Moving this to the 5.5 milestone, as it's marked early, and we are one day away from beta for 5.4

#25 @afragen
4 months ago

Latest patch 48491.5.diff adds compatibility testing when in the Customizer.

To test simply change the Requires PHP header of the style.css of an installed theme. You can test the update part by also decrementing the version number too.

@afragen
4 months ago

add Customizer part

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


4 months ago

@afragen
4 months ago

minor updates to patch

@afragen
4 months ago

update 'Activate & Publish' back to button type

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


3 months ago

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


3 months ago

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


2 months ago

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


2 months ago

#31 @afragen
7 weeks ago

  • Keywords needs-dev-note added

#32 follow-up: @SergeyBiryukov
7 weeks ago

This is great work, I'm sorry the review took so long. There really are a lot of different layers and parts of the themes UI, so testing that everything works as expected is non-trivial.

Some notes on the patch:

  • No need to use ReflectionObject in wp-admin/customize.php, the WP_Customize_Manager::theme() method can be used to retrieve the theme object instead.
  • It seems like the update parts need some more work. Unless I've missed something in my testing, at least in a few places the messages state that the theme update is incompatible, but the compatibility checks are actually performed for the currently installed version, not for the update.
  • The activation parts look good, commit incoming.

#33 @SergeyBiryukov
7 weeks ago

In 47816:

Themes: Prevent activation and live preview of themes that require a higher version of PHP or WordPress.

Props afragen, jorbin, TimothyBlynJacobs, SergeyBiryukov.
See #48491.

#34 @SergeyBiryukov
7 weeks ago

In 47817:

Themes: Remove debug call from wp-admin/theme-install.php.

Follow-up to [47816].

See #48491.

#35 @SergeyBiryukov
7 weeks ago

In 47818:

Themes: Remove extra whitespace from wp-admin/includes/ajax-actions.php and wp-admin/includes/theme.php.

Follow-up to [47816].

See #48491.

#36 in reply to: ↑ 32 @SergeyBiryukov
7 weeks ago

Replying to SergeyBiryukov:

There really are a lot of different layers and parts of the themes UI, so testing that everything works as expected is non-trivial.

For future reference:

  • wp-admin/customize.php
    • Includes displaying the "Activate & Publish" button in Customizer side panel.
  • wp-admin/includes/ajax-actions.php
    • wp_ajax_query_themes() provides data for the Add Themes screen.
  • wp-admin/includes/theme.php
    • customize_themes_print_templates() is used for displaying the Theme Details modal in the Customizer theme browser.
  • wp-admin/theme-install.php
    • #tmpl-theme is used for displaying the grid on the Add Themes screen.
    • #tmpl-theme-preview is used for displaying the Details & Preview modal on the Add Themes screen.
  • wp-admin/themes.php has three different templates:
    • The first one (in PHP) is used for displaying the grid on the Themes screen when JS is turned off.
    • #tmpl-theme is used for displaying the grid on the Themes screen.
    • #tmpl-theme-single is used for displaying the Theme Details modal on the Themes screen.
  • wp-includes/customize/class-wp-customize-theme-control.php
    • WP_Customize_Theme_Control::content_template() is used for displaying the grid in the Customizer theme browser.
Last edited 7 weeks ago by SergeyBiryukov (previous) (diff)

#37 @SergeyBiryukov
7 weeks ago

To summarize: with the activation and live preview parts out of the way in [47816], it should hopefully be easier to focus on the update parts now.

#38 @afragen
7 weeks ago

Thanks @SergeyBiryukov 🤩

Thanks for documenting all the above. 😊

Last edited 7 weeks ago by afragen (previous) (diff)

#39 @SergeyBiryukov
7 weeks ago

In 47819:

Themes: Prevent installation of themes that require a higher version of PHP or WordPress.

Props afragen.
Fixes #49653. See #48491.

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


7 weeks ago

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


4 weeks ago

#42 @davidbaumwald
9 days ago

  • Keywords early removed

Removing early tag per discussion with @SergeyBiryukov

#43 @desrosj
5 days ago

Just wanted to circle back on this and make sure that the error messages were not omitted on accident. @SergeyBiryukov I believe that is what you mean in 48491#comment:36, but wanted to make sure.

Attached screenshots to document the current experience. The missing error messages are very apparent.

Also, #48245 was created to add some extra details to the error message displayed when a user attempts to activate a plugin without proper support. I think some of these points could be used to improve the messaging that will be added here. From @johnbillion in #48245:

Some helpful information should be added to this screen:

  • The plugins' minimum required PHP version
  • The current PHP version
  • A link to the support documentation on how to update PHP

#44 @afragen
5 days ago

I believe the error messages on these particular screens are omitted on purpose as there’s no obvious location and error messages do exist in other locations. I haven’t looked at this recently however.

Note: See TracTickets for help on using tickets.