Opened 6 years ago
Closed 6 years ago
#3718 closed task (blessed) (fixed)
Theme Directory: Allow minimum WP and PHP version requirement
Reported by: | flixos90 | Owned by: | dd32 |
---|---|---|---|
Milestone: | Priority: | normal | |
Component: | Theme Directory | Keywords: | has-patch servehappy |
Cc: |
Description
As a follow up to https://core.trac.wordpress.org/ticket/40934 and #2952, this ticket aims to add headers for version requirements to WordPress themes.
Themes do not always support all WordPress versions (for example Twenty Seventeen requires WP 4.7), and, while that is not a common practice seen so far, there are also themes that require a higher PHP version than 5.2. Core should highlight such incompatibilities, but as a prerequisite we need to allow theme authors to indicate required versions.
Attachments (1)
Change History (35)
This ticket was mentioned in Slack in #core-php by flixos90. View the logs.
6 years ago
#4
@
6 years ago
FWIW I think this is part of the bigger discussion around Theme Meta. Unlike the Plugin Directory, Themes have no way to define extra meta-data like this, or even to have a theme description that isn't one-long-line-that's-unreadable-because-it-just-keeps-on-going-on-and-on.
The patch as-is adds it to the Theme style.css headers, which seems like the wrong place for it - but the only place we currently have.
All new themes in the repo need to have a readme.txt
- but the theme directory doesn't parse anything from it - #215
To me, it feels like the implementation should match what plugins have, where it's included in the readme (which is where all the Metadata ultimately comes from).
I have a feeling that borrowing the plugin directory readme parser and only supporting headers / description wouldn't require too much effort here and would provide for a much better platform for future changes (such as supporting a minimum WP version requirement too, etc)
This ticket was mentioned in Slack in #core-php by schlessera. View the logs.
6 years ago
This ticket was mentioned in Slack in #meta by tellyworth. View the logs.
6 years ago
#7
follow-up:
↓ 8
@
6 years ago
I agree that having new themes require a readme.txt file is the path forward. Parity with the plugin directory makes the most sense as does using the same parser.
For existing themes that don’t have a readme.txt file couldn’t we create a parsed data array from the info we do have and use that if no actual reader.txt file exists?
#8
in reply to:
↑ 7
;
follow-up:
↓ 9
@
6 years ago
Replying to afragen:
For existing themes that don’t have a readme.txt file couldn’t we create a parsed data array from the info we do have and use that if no actual reader.txt file exists?
That's what would happen, yes.
I'm simply suggesting that skipping this header entirely and pulling at least that info from the readme would be a better long-term solution
#9
in reply to:
↑ 8
@
6 years ago
Replying to dd32:
I'm simply suggesting that skipping this header entirely and pulling at least that info from the readme would be a better long-term solution
Agreed. I don’t think the style.css file needs to be parsed at all. I would suggest the following process for getting the information.
- Parse an existing readme.
- Use the API to get existing data, eg.
https://api.wordpress.org/themes/info/1.2/?action=theme_information&request[slug]=twentyten
We might be referring to the same process by different names. I’m not sure.
This ticket was mentioned in Slack in #meta by sergey. View the logs.
6 years ago
This ticket was mentioned in Slack in #themereview by joyously. View the logs.
6 years ago
This ticket was mentioned in Slack in #themereview by joyously. View the logs.
6 years ago
This ticket was mentioned in Slack in #core-php by sergey. View the logs.
6 years ago
This ticket was mentioned in Slack in #design by boemedia. View the logs.
6 years ago
This ticket was mentioned in Slack in #core-php by afragen. View the logs.
6 years ago
This ticket was mentioned in Slack in #themereview by andreiglingeanu. View the logs.
6 years ago
#18
@
6 years ago
If we are hoping to achieve a similar effect for blocking theme installation that doesn't meet the developer specified compatibility requirements as listed in the themes readme.txt
we will need the Themes API, as described above, to return these data Requires at least
and Requires PHP
.
This ticket was mentioned in Slack in #core by afragen. View the logs.
6 years ago
This ticket was mentioned in Slack in #meta by afragen. View the logs.
6 years ago
This ticket was mentioned in Slack in #themereview by poena. View the logs.
6 years ago
#24
@
6 years ago
- Owner set to dd32
- Status changed from new to accepted
[8789] starts extracting some minimal header information, based off the themes readme.txt
standard.
This data is intentionally not exposed in the API yet, this is the minimal steps needed to start extracting the data for later use - and also to act as a flag to easier identify which themes do have extractable readme.txt data (hopefully all of them).
We can start exposing the data in various places once we've got a few themes with data in the database - 3718.diff looks like it'll be mostly ready-to-go.
#26
@
6 years ago
Just noting that so far we've imported 49 themes (new submissions & updates) who have the Requires PHP
header.. So far 8 require PHP 7.2.14, the rest are mostly split PHP 5.2 and 5.6.
Looking at the themes that specify 7.2.14, it doesn't look like any of them actually require PHP 7.2 (Tide confirms it too) so I'm not sure if it's Authors mis-using the field, preferring recent versions, or just copying from an example somewhere?
#32
@
6 years ago
This should now be rolled out.
As it turned out, I missed committing some of the code last week, but it's fixed now.
The API's (info, and update-checks) should now also be returning the data.
eg: https://api.wordpress.org/themes/info/1.2/?action=theme_information&request[slug]=doody
I've not backfilled the data for older themes, but I'm going to call this fixed for now, given the speed at which most themes get updated.
If going back and importing data for all themes is needed, let me know and I'll make it happen.
3718.diff adds support for a
Requires
header (for required WP version) and aRequires PHP
header (for required PHP version). While we could go withRequires WP
, I thought it would make sense to align the names with the names of the plugin headers, so people can remember them easily for both plugins and themes.As far as I can tell, my patch covers all areas necessary to have full support for those headers, but since I haven't contributed to the theme directory meta part before, I might have missed something else.
Also, be aware that this patch would require https://core.trac.wordpress.org/ticket/44592 to be merged into core first, in order to have support for the two headers in the
WP_Theme
class, which is also used by the theme directory plugin.