[Theme compatibility] Changes to WP_Theme to add correct header data

[afragen]

There are a couple of changes required for testing theme WP/PHP compatibility. This is a prerequisite to all other patches testing theme WP/PHP compatibility.

This started with #44592. This ticket incorporates #44592.
Fundamentally, this ticket adds the Requires and RequiresPHP headers to themes.

It then correctly sets these data with a new function get_theme_headers(). The get_theme_headers() function obtains the headers from the style.css and readme.txt files and combines them giving precedence to data in the readme.txt file. Then it obtains the current Requires and RequiresPHP data from the Theme API and inserts these data with highest precedence.

Related: #48491, #48507, and others to follow.

Please apply the patch on this ticket in order to test related patches.

[Otto42]

You got the precedence order incorrect.

Precedence order: API > readme.txt > style.css

The style.css should take precedence. The readme.txt is second. And the API should never be consulted if the theme is installed and local already, as the API is only used when the theme isn't installed yet.

[Otto42]

The API request prevents this change from going into core, as it should not be there and would cause fundamental breakage on wordpress.org by the excessive amount of server requests.

Furthermore, it is using a wp_remote_get call on a built URL instead of the correct use of the themes_api() function.

And furthermore, as stated above, it shouldn't be consulted here in the first place.

[afragen]

@Otto42 I will update and remove this. As long as I can get the API data on the install theme page it should be fine.

I do have to say that every default theme except Twenty Twenty, incorrectly lists Requires at least: WordPress x.x and not just Requires at least: x.x. I will need to add some code to strip that, it was in a previous version.

Thanks for you insight, invaluable as always.

[afragen]

Regarding precedence order, we had a brief discussion during the last #core-site-health meeting about precedence order and it was thought to future proof we should give precedence to readme.txt > style.css.

​https://wordpress.slack.com/archives/CKSU841L7/p1572883699040600

[afragen]

remove call to API

[joyously]

it was thought to future proof we should give precedence to readme.txt > style.css

The trick is that the readme.txt is for the Theme Repository. So not all themes will have readme.txt. Core reads style.css, which must exist, so it should have priority. Anything missing from style.css can be filled in from readme.txt if it exists.

[Otto42]

Yes, what joyously said. The style.css is a required file for themes, the readme.txt isn't. Additionally, most themes that have made readme files have not followed the same standard as plugin files have, historically, so it is an unreliable indicator.

Headers should be in the style.css file. The readme.txt file should not even be read by core at all, for plugins or themes. The fact that that was added for plugins was something of a mistake, and continuing to compound that error isn't a good approach forward.

Core should not be parsing readme.txt files, at all.

[afragen]

Perhaps it's my misunderstanding. I thought the intent was to require themes to have a readme.txt file.

There is clearly some disagreement as to precedence. In the Slack discussion, ​https://wordpress.slack.com/archives/CKSU841L7/p1572883699040600

@clorith and @williampatton seem to be asking for the precedence of readme.txt > style.css. Obviously this will need some further input from the #core-themes folks.

@Otto42 I really don't understand why reading the readme.txt file headers is a mistake. The data exists and is currently required for plugins/themes in dot org. Can you elaborate?

[afragen]

Replying to joyously:

it was thought to future proof we should give precedence to readme.txt > style.css

The trick is that the readme.txt is for the Theme Repository. So not all themes will have readme.txt. Core reads style.css, which must exist, so it should have priority. Anything missing from style.css can be filled in from readme.txt if it exists.

This was how I originally did this, but was convinced in Slack to change the precedence order. It's a simple change.

[Otto42]

When they added the requires header for this, they put it in the wrong file.

Historically, WordPress code code only parsed the plugin's main PHP file for header information. The readme.txt was, and still is, completely optional for plugins. It's been used only by the plugin directory itself, for information that is shown on WordPress.org or passed to WordPress as part of the API request when installing new plugins.

They put it in the readme.txt because they thought that this was required for it to be in the API response. However, this was not the case, the plugin directory gets information from both sources and always has. Since the intent was to eventually have core need this information from locally installed plugins as well, then the information should have been in the plugin's main PHP file from the start.

This not being the case, core was altered at one point to read and parse the readme.txt, which again, is supposed to be a completely optional file.

This is the fundamental error. Core should *not* parse the readme.txt file. The addition of this header to the readme.txt was a mistake, and further mistakes have been made compounding that error.

Now you're making the same mistakes and compounding it further with the themes. This needs to be corrected before it spreads any further.

• The header should be in style.css or in the plugin's main PHP file only. It should not be in readme.txt at all.
• The core should not parse the readme.txt. Code doing so should be removed in the long term. No new code should be added.
• The readme.txt should remain optional for both plugins and themes, and should only contain information to be used by the WordPress.org system itself, and not information that core should read locally.

[Otto42]

As for the slack discussion, yes, there is much confusion on this topic. I've been trying to clear it up for ages now, over many releases. This is getting far out of hand.

Please do not add any code to core that parses the readme.txt file, for any reason. This separation of these two files is fairly critical to how we process things, and adding in additional confusion is going to create additional technical burden for development of the directories as well as the API in the future. Every time we screw this up we're creating more and more backwards compatibility issues for ourselves.

Do not have core code parse the readme.txt file *for any reason*. Code in there that is doing it now *should be removed* in the first available release where it is possible to do so.

[afragen]

Thanks @Otto42. I remember when this was added for validate_plugin_requirements(). For plugins, precedence is for the plugin file headers. We could probably remove the check on readme.txt if we can some definitive statement of all the data that should exist in the main plugin/theme file.

New patch coming to remove parsing of readme.txt

[Otto42]

We could probably remove the check on readme.txt if we can some definitive statement of all the data that should exist in the main plugin/theme file.

ALL headers that core reads from a theme or a plugin should be in the theme's style.css file or in the plugin's main PHP file.

No exceptions.

[afragen]

@Otto42 the problem arises when there is no Requires at least or Requires PHP data in the main theme file but does exist in the readme.txt. This is the case for all the default themes.

What's the solution for that?

[Otto42]

The themes should be modified to put the header in the correct place, which is the style.css file.

As I stated, original confusion led to this being incorrectly put into the readme.txt. It never should have gone there. Themes and plugins with it in there should be modified to have it in the correct location.

[afragen]

How then do I get the Requires at least and Requires PHP headers added to all the default themes?

And we would then need a definitive statement of these requirements in both plugin and theme main files or no compatibility testing is possible.

[joyously]

Themes are not supposed to fatal, ever. So if they don't have the version fields, a low number should be supplied.

[Otto42]

How then do I get the Requires at least and Requires PHP headers added to all the default themes?

With a patch. The default themes are in the purview of core. They can be patched like anything else in core.

​http://develop.svn.wordpress.org/trunk/src/wp-content/themes/

Usually when themes get a change, we do a sort of mass update of them in the theme directory at the same time as the core release. Usually only happens for major releases.

[joyously]

Patching the default themes doesn't fix the thousands of existing themes that do no have the version headers.

[williampatton]

Replying to Otto42:

As for the slack discussion, yes, there is much confusion on this topic. I've been trying to clear it up for ages now, over many releases. This is getting far out of hand.

I'll admit that I was indeed confused about this. Plans were always to follow the same path that was taken with plugins for themes. If you are telling us that the way plugins handle it right now is not correct then certainly we should not follow and instead do it differently. We can port that back over to plugins and solve there too.

We do still have the issue of what to do with themes that do not add these headers (no matter where they are, we will always have old themes that do not have them included). I am not certain if we should assume those to be compatible or incompatible but I am leaning towards assuming compatible when no headers are stated. That seems less than ideal however I think that would be preferable.

[afragen]

Replying to joyously:

Themes are not supposed to fatal, ever. So if they don't have the version fields, a low number should be supplied.

As with plugins, if no number is supplied it passes through. Though I'm not sure how we can assume a theme would never fatal as a new version of PHP is released.

[afragen]

With all the reverts to this patch, this ticket can be closed in favor of #44592.

[afragen]

Related #48517

[Otto42]

We can port that back over to plugins and solve there too.

That is the great hope, yes. :)

We do still have the issue of what to do with themes that do not add these headers (no matter where they are, we will always have old themes that do not have them included). I am not certain if we should assume those to be compatible or incompatible but I am leaning towards assuming compatible when no headers are stated. That seems less than ideal however I think that would be preferable.

Realistically, assuming compatible is what we do now since there is no compatibility testing or information available. No reason to change that default behavior. More to the point, no need to create a blocking issue for ourselves by changing it unnecessarily.

Closing as per @afragen's comment above.

[SergeyBiryukov]

Replying to afragen:

And we would then need a definitive statement of these requirements in both plugin and theme main files or no compatibility testing is possible.

Right. To summarize the discussion, looks like the follow-up steps here would be to:

• Resolve ​#meta4621, so that all three headers (Requires at least, Tested up to, Requires PHP) could be defined in the same place.
• Publish posts on make/plugins and make/themes with an announcement for plugin and theme authors to move these headers to the main plugin file or the style.css file, and remove them from readme.txt.
• Make validate_plugin_requirements() only read headers from the main plugin file, not from readme.txt.

[williampatton]

• Publish posts on make/plugins and make/themes with an announcement for plugin and theme authors to move these headers to the main plugin file or the style.css file, and remove them from readme.txt.

@dingo_d you think you could look to write that post or can you think of anyone else that would be able to write it for themes?

[afragen]

@SergeyBiryukov I have your last one handled. #48520

[afragen]

We don't really use the Tested up to header in core and it is required in the readme to show the information in the repository.

[Otto42]

Replying to afragen:

We don't really use the Tested up to header in core and it is required in the readme to show the information in the repository.

Nothing in the readme is required, as such. If it's there, we use it for the w.org page.

In theory, this was originally put there so we could determine whether to provide an update or not based on the version of WordPress checking for updates. That didn't work well, so mainly we now use it for helping rank plugin search results. If it's up to date, then the author at least says they tested it. If not, then the author hasn't updated in a while and we can reduce their rank.

If core needs info, then it can be in the plugin file. If only the directory or API needs it, then it can be in the readme.

[SergeyBiryukov]

Replying to Otto42:

If core needs info, then it can be in the plugin file. If only the directory or API needs it, then it can be in the readme.

Right. It's just confusing to define Requires at least and Requires PHP in the main plugin file, and then Tested up to in readme.txt. It would make more sense to define all three headers in the main plugin file, for consistency.

Even though core only reads Tested up to from the API response and not from headers at the moment, that would open such a possibility in the future.

[Otto42]

Hmm. I'm not really feeling that confusion. I don't think that it makes sense to move the header to the style.css or main PHP file unless core actually needs it. This separation makes sense to me, so I feel that tested up to should remain in the readme unless there is a reason core needs this information locally.

[SergeyBiryukov]

In 51092:

Upgrade/Install: Remove parsing of readme.txt files for plugin or theme requirements.

This affects:

• validate_plugin_requirements()
• validate_theme_requirements()

Historically, the Requires PHP header was introduced in ​#meta2952 for the Plugin Directory first, so at the time it made sense to have it defined in the same place as Requires at least, which only existed in readme.txt.

Since parsing of PHP and WordPress requirements was later added to WordPress core, the core should retrieve all the necessary data from the main plugin or theme file and not from readme.txt, which only contains the data meant for the Plugin or Theme Directory.

The recommended place for Requires PHP and Requires at least headers is as follows:

• The plugin's main PHP file
• The theme's style.css file

The place for the Tested up to header remains in readme.txt for the time being, as it's not used by WordPress core.

Follow-up to [44978], [45546], [47573], [47574], ​[meta5841], ​[meta9050].

Props afragen, Otto42, joyously, williampatton, audrasjb.
Fixes #48520. See #48515, ​#meta2952, ​#meta4514, ​#meta4621.