Make WordPress Core

Opened 5 years ago

Closed 4 years ago

#50864 closed feature request (wontfix)

Handling readme.txt file in validate_theme_requirements function

Reported by: tmatsuur's profile tmatsuur Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.5
Component: Themes Keywords:
Focuses: Cc:

Description

The validate_theme_requirements function added in version 5.5 will read the readme.txt file if it exists.

This readme.txt file name is README.txt for some themes.

For example, Twenty Seventeen from WordPress 5.2 (Currently changed to readme.txt).

Is the file name of readme.txt already unified as "readme.txt"?

If it is not unified yet, I think it is better to read "README.txt".

Change History (3)

#1 in reply to: ↑ description @SergeyBiryukov
5 years ago

Thanks for the ticket!

This readme.txt file name is README.txt for some themes.

For example, Twenty Seventeen from WordPress 5.2 (Currently changed to readme.txt).

Yes, it was changed to readme.txt for Twenty Seventeen in [46719] for consistency.

Is the file name of readme.txt already unified as "readme.txt"?

If it is not unified yet, I think it is better to read "README.txt".

As far as I know, readme.txt is the official name, but it appears to be treated by the plugin and theme directories as case-insensitive, see Import::find_readme_file() for plugins and WPORG_Themes_Upload::get_readme_data() for themes.

So I guess checking both names would make sense. It's worth noting though that readme.txt is only used as a fallback here, the primary location for the Requires at least and Requires PHP headers is the theme's style.css file, or the plugin's main PHP file. So maybe complicating the fallback check is not worth it.

#2 follow-up: @tmatsuur
5 years ago

Thanks @SergeyBiryukov

| So maybe complicating the fallback check is not worth it.

I agree with you on this.

However, I think that the current source code will behave differently depending on the OS.

If README.txt exists in the theme directory, README.txt will be read on Windows OS, but README.txt should not be read on most other OSes.

What are your thoughts on this?


Also, I am concerned about the part where the arrays are merged.

For example, create a small theme and write the following at the beginning of style.css.

/*
 Theme Name:   Twenty Twenty Child
 Description:  Twenty Twenty Child Theme
 Template:     twentytwenty
 Requires at least: WordPress 5.5.0
 Version:      0.0.1
*/

The beginning of readme.txt is as follows:

=== Twenty Twenty Child ===
Contributors: the WordPress team
Requires PHP: 5.6.0
Tested up to: 5.5
Stable tag: 1.0

Now when you call the validate_theme_requirements function, the contents of $requirements before the merge are as follows:

array(2) {
  ["requires"]=>
  string(15) "WordPress 5.5.0"
  ["requires_php"]=>
  string(0) ""
}

The contents of $readme_headers are as follows:

array(2) {
  ["requires"]=>
  string(0) ""
  ["requires_php"]=>
  string(5) "5.6.0"
}

The contents of merging these two arrays are as follows.

array(2) {
  ["requires"]=>
  string(15) "WordPress 5.5.0"
  ["requires_php"]=>
  string(0) ""
}

The contents of $readme_headers are overwritten by the contents of $requirements because the array keys are the same.

Is this as expected?

#3 in reply to: ↑ 2 @SergeyBiryukov
4 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Thanks for the follow-up!

Going to close this in favor of #48520, which aims to remove the parsing of readme.txt files for both plugin and themes, and only read the requirements from the plugin's main PHP file or the theme's style.css file.

Note: See TracTickets for help on using tickets.