Theme-Requirements icon indicating copy to clipboard operation
Theme-Requirements copied to clipboard

Place information about all existing checks with errors and requirements in one document

Open carolinan opened this issue 3 years ago • 20 comments

The purpose of this issue is to place information about all existing requirements in one document.

  1. Collect the information
  2. Requirements that should be reduced to recommendations will be removed from the list and placed in https://github.com/WPTT/Theme-Requirements/issues/11
  3. Publish a proposal with the new, remaining requirements on the make blog.

Theme Review action

https://github.com/WordPress/theme-review-action

Check Level Is it on the requirements page?
Structure check -check for required files depending on theme type. Required/error yes
Skip links warning (temporarily) yes
Keyboard navigation -Partial warning (temporarily) yes
Links within content and comments must be underlined warning (temporarily) yes
No PHP or JavaScript errors, warnings or notices Required/Error yes
Themes can include one single front facing credit link, which is restricted to the Theme URI or Author URI defined in style.css required/error yes
Block themes: Include required files: Index.php, style.css, readme.txt, theme.json, and index.html inside a folder called block-templates. Required/Error yes
Block templates should be complete Required/Error yes

Theme Check

https://github.com/WordPress/theme-check

Check Run on block theme Level Is it on the requirements page?
Bad things yes required not the details, the requirements page only says that the theme must be secure.
Basic (doctype, body class, theme support) no required yes, but in separate places.
cdn -can be replaced with an allowed list? yes required yes
constants Without statistics, it is impossible to know if theme authors still use these and we need to check for them 🤷‍♀️ no required yes, under 4, but not listed by name.
customizer (checks for sanitize callbacks) no required yes
Directories (checks for .git etc) yes required yes
Escaping yes warning, required yes
Filenames checks for development config files, php.ini etc. yes required yes, add as example under Files
FSE required files yes required yes
Included zip files ⚠️ This does not need to be its own check, move to filenames? yes required yes, under plugins
Line endings this is required because of svn yes required yes
Non GPL sites yes required yes, in the license requirements
Plugin territory yes required yes
Screenshot yes recommended, required yes
Style CSS Header yes required yes
Title tag theme support no required yes
Theme and author URI -theme uri can not be WordPress.org yes required yes
Worms yes required not the details, the requirements page only says that the theme must be secure.
Underscores _s no required yes

carolinan avatar Jun 10 '21 08:06 carolinan

Of the theme check checks, I suggest that the following are moved to warnings or recommended:

  • Generated themes
  • Screen reader text CSS class- The theme may look ugly, but its the authors responsibility.
  • Favicon (this will allow themes to include a favicon)
  • Deprecated, both checks
  • Constants
  • Post formats (Because I personally don't care if you style the aside like an aside...)
  • Customizer -Only because the current checks for sanitize callbacks are not good enough and block theme uploads if variables are used.
  • The file names list is controversial. I suggest removing anything from it that isn't a security risk.

carolinan avatar Jun 15 '21 11:06 carolinan

For post formats tag, it should be requirement and need to block upload. Because theme users use filters while searching themes on theme page based on tags. But if the theme having post-formats tags is not properly styled and support it, that makes no sense.

Here is a ticket for more reference.

kafleg avatar Jun 16 '21 06:06 kafleg

In my opinion, that does not align with https://make.wordpress.org/themes/2021/03/17/next-steps-on-themes-and-reviews/

"One of the action items from our call with Matt, was to come up with a list of non-negotiable guard rails for what could prevent a theme from being added to the repo. After some good discussions with him, I have a short list of guard rails but with the clarification that they shouldn’t prevent authors from submitting themes."

carolinan avatar Jun 16 '21 09:06 carolinan

Changelog: reduced the search form check to warning and improved the explanation.

carolinan avatar Jun 16 '21 12:06 carolinan

Style tags checks for allowed and deprecated tags in style.css -I propose that these should all be info's.

carolinan avatar Jun 16 '21 12:06 carolinan

Theme and author URI -theme uri can not be WordPress.org -This one is kept because it falls under the new Trademark requirement?

carolinan avatar Jun 16 '21 12:06 carolinan

In my opinion, that does not align with https://make.wordpress.org/themes/2021/03/17/next-steps-on-themes-and-reviews/

"One of the action items from our call with Matt, was to come up with a list of non-negotiable guard rails for what could prevent a theme from being added to the repo. After some good discussions with him, I have a short list of guard rails but with the clarification that they shouldn’t prevent authors from submitting themes."

Okay, I got the point.

kafleg avatar Jun 16 '21 12:06 kafleg

The plugin-territory functions currently includes 4 functions: https://github.com/WordPress/theme-check/blob/17dd4eb9393a6e6eca45401f01f629fe752e81e5/checks/class-plugin-territory-check.php#L33-L38

  • register_post_type: This should definitely be kept because if we remove it then users will lose content when switching themes.
  • register_taxonomy: Same as register_post_type. Taxonomies can be used to add content too, and categorize/organize content. Removing a taxonomy when the theme changes would be detrimental to users and their content.
  • wp_add_dashboard_widget: I believe we can remove this. It is not content-related and does not create lock-in.
  • register_block_type: This needs to be kept. Blocks are used to build content, and having a block registered in a theme would lock users to that theme and prevent them from changing 'cause otherwise they'll bet broken blocks in their content.

aristath avatar Jun 16 '21 12:06 aristath

I think we can also remove the appearance-menu rule...

aristath avatar Jun 16 '21 12:06 aristath

Admin menu rules reduced and removed from the requirements page draft 🎉

carolinan avatar Jun 16 '21 12:06 carolinan

I think we can remove favicon requirement too. It's been so long and I haven't seen any theme that is summitted having favicon functionality from theme.

kafleg avatar Jun 16 '21 13:06 kafleg

Suggest to remove - Menu locations and sidebar IDs are exceptions from prefixing section. If that is exception, then it will only create confusion.

kafleg avatar Jun 17 '21 08:06 kafleg

I suggest to change the text here, Place WordPress core features behind a paywall.

kafleg avatar Jun 17 '21 08:06 kafleg

I suggest to change the text here, Place WordPress core features behind a paywall.

Why change this? They can place their own features behind a paywall if they want, but not things that are from core or plugins. Changing the text to include "core" makes it too specific and they can say "oh, so I can add a paywall in my theme and disable variable-products in WooCommerce 'cause styling for them is only included in the premium version of my theme". Leaving it without "core" makes more sense IMO

aristath avatar Jun 17 '21 08:06 aristath

I suggest to change the text here, Place WordPress core features behind a paywall.

Why change this? They can place their own features behind a paywall if they want, but not things that are from core or plugins. Changing the text to include "core" makes it too specific and they can say "oh, so I can add a paywall in my theme and disable variable-products in WooCommerce 'cause styling for them is only included in the premium version of my theme". Leaving it without "core" makes more sense IMO

Thanks. I just thought in a straight way.

kafleg avatar Jun 17 '21 09:06 kafleg

For the file list,

		$blocklist = array(
			'thumbs\.db'          => __( 'Windows thumbnail store', 'theme-check' ),
			'desktop\.ini'        => __( 'windows system file', 'theme-check' ),
			'project\.properties' => __( 'NetBeans Project File', 'theme-check' ),
			'project\.xml'        => __( 'NetBeans Project File', 'theme-check' ),
			'\.kpf'               => __( 'Komodo Project File', 'theme-check' ),
			'^\.+[a-zA-Z0-9]'     => __( 'Hidden Files or Folders', 'theme-check' ),
			'php\.ini'            => __( 'PHP server settings file', 'theme-check' ),
			'dwsync\.xml'         => __( 'Dreamweaver project file', 'theme-check' ),
			'error_log'           => __( 'PHP error log', 'theme-check' ),
			'web\.config'         => __( 'Server settings file', 'theme-check' ),
			'\.sql'               => __( 'SQL dump file', 'theme-check' ),
			'__MACOSX'            => __( 'OSX system file', 'theme-check' ),
			'\.lubith'            => __( 'Lubith theme generator file', 'theme-check' ),
			'\.wie'               => __( 'Widget import file', 'theme-check' ),
			'\.dat'               => __( 'Customizer import file', 'theme-check' ),
			'phpcs\.xml\.dist'    => __( 'PHPCS file', 'theme-check' ),
			'phpcs\.xml'          => __( 'PHPCS file', 'theme-check' ),
			'\.xml'               => __( 'XML file', 'theme-check' ),
			'\.sh'                => __( 'Shell script file', 'theme-check' ),
			'postcss\.config\.js' => __( 'PostCSS config file', 'theme-check' ),
			'\.editorconfig.'     => __( 'Editor config file', 'theme-check' ),
			'\.stylelintrc\.json' => __( 'Stylelint config file', 'theme-check' ),
			'\.eslintrc'          => __( 'ES lint config file', 'theme-check' ),
			'favicon\.ico'        => __( 'Favicon', 'theme-check' ),
		);

Are we only removing these for now?

'phpcs\.xml\.dist'    => __( 'PHPCS file', 'theme-check' ),
'phpcs\.xml'          => __( 'PHPCS file', 'theme-check' ),
'postcss\.config\.js' => __( 'PostCSS config file', 'theme-check' ),
'\.editorconfig.'     => __( 'Editor config file', 'theme-check' ),
'\.stylelintrc\.json' => __( 'Stylelint config file', 'theme-check' ),
'\.eslintrc'          => __( 'ES lint config file', 'theme-check' ),

I don't know anything about

			'project\.properties' => __( 'NetBeans Project File', 'theme-check' ),
			'project\.xml'        => __( 'NetBeans Project File', 'theme-check' ),
			'\.kpf'               => __( 'Komodo Project File', 'theme-check' ),

carolinan avatar Jun 17 '21 12:06 carolinan

I closed https://github.com/WordPress/theme-check/issues/126 and admin pointers are not listed on the requirements page.

carolinan avatar Jun 21 '21 04:06 carolinan

PR's submitted today for:

File check (still required) title tag and wp_title -changed to recommended wp_filesystem -one check changed to info, others remain as warnings.

Once merged, the list above will be updated.

carolinan avatar Jul 01 '21 12:07 carolinan

There is also a PR for updating the favicon check to prevent it from stopping uploads.

carolinan avatar Jul 01 '21 13:07 carolinan

I opened #13 not sure if it made sense to add it here. Thoughts?

StevenDufresne avatar Jul 21 '21 04:07 StevenDufresne