Allow block themes to be uploaded without index.php

[FlorianBrinkmann]

In [52940] it was made possible to activate a block theme without an index.php, but that file is still a requirement when uploading a theme via the theme install screen (tested with WP 6 RC1). I think that should not be the case, when activating is possible without it.

To reproduce: Try to upload a ZIP containing a block theme without an index.php file-

[SergeyBiryukov]

Hi there, welcome back to WordPress Trac! Thanks for the ticket.

Moving to the milestone for visibility, just in case this should be addressed for 6.0. Otherwise, I think this can be moved to 6.0.1 or 6.1.

[FlorianBrinkmann]

Thanks!

https://core.trac.wordpress.org/ticket/55682

[peterwilsoncc]

I forgot to post on the ticket at the time, I've set up a ​pull request linked to this ticket:

• themes are required to have index.php or templates/index.html
• error message updated to use indicate either file is required
• error uses an existing string in WP so it doesn't break the string freeze

[Boniu91]

Test Report

Env

• WordPress 6.0-beta3-53290-src and patch version
• Theme: Twenty Twenty Two (edited), Twenty Twenty (edited)

Steps to test

1. Upload block theme that:

• doesn't have index.php nor templates/index.html
• doesn't have index.php but has templates/index.html
• has index.php but doesn't have templates/index.html
• has index.php and templates/index.html

2. Upload non block theme that:

• doesn't have index.php nor templates/index.html
• doesn't have index.php but has templates/index.html
• has index.php but doesn't have templates/index.html
• has index.php and templates/index.html

Is it possible to upload a theme (before|after the patch)?

Block theme:

• ❌|❌
• ❌|✅
• ✅|✅
• ✅|✅

Non block theme:

• ❌|❌
• ❌|✅
• ✅|✅
• ✅|✅

Question

Should it be allowed to upload non block theme that doesn't have index.php but has templates/index.html?

[costdev]

Test Report

Environment

• Server: Apache (Linux)
• WordPress: 6.1-alpha-53344-src
• Browser: Chrome 101.0.4951.54
• OS: Windows 10
• Theme: Twenty Twenty-Two
• Plugins: None activated.

Steps

1. Create four theme folders:
   • Just style.css.
   • style.css, index.php and templates/index.html.
   • style.css and index.php.
   • style.css and templates/index.html.
2. Zip each folder into its own .zip file.
3. Navigate to Appearance > Themes > Add New > Upload Theme.
4. Attempt to upload each theme.
5. Delete the uploaded themes.
6. Apply ​PR 2692.
7. Repeat steps 3-4.

Results

1. Before applying ​PR 2692:
   • Just style.css. ❌
   • style.css, index.php and templates/index.html. ✅
   • style.css and index.php. ✅
   • style.css and templates/index.html. ❌
2. After applying ​PR 2692:
   • Just style.css. ❌
   • style.css, index.php and templates/index.html. ✅
   • style.css and index.php. ✅
   • style.css and templates/index.html. ✅

Notes/Queries

1. The PR appears to work as expected.
2. As a block theme may instead use block-templates/index.html for BC reasons ​ref, should the PR also account for this file?

[hellofromTonya]

​as noted in the PR, PR 2692 changed the behavior for classic themes.

• Before this PR, a classic theme without a index.php file could not be uploaded.
• After this PR, it can be if it has a templates/index.html file.

Is this problematic?

[aristath]

Replying to hellofromTonya:

​as noted in the PR, PR 2692 changed the behavior for classic themes.

• Before this PR, a classic theme without a index.php file could not be uploaded.
• After this PR, it can be if it has a templates/index.html file.

Is this problematic?

No, that's OK and not a problem. If the theme has a templates/index.html file, it would override the index.php file even if it existed.
It's possible to have both PHP and HTML templates in the same theme, but HTML templates get a higher priority than PHP templates, so an index.php file doesn't make any sense when the HTML file exists 👍

[costdev]

PR 2692 changed the behavior for classic themes.
Before this PR, a classic theme without a index.php file could not be uploaded.
After this PR, it can be if it has a templates/index.html file.

I agree with Ari that this isn't problematic, as it doesn't change the behaviour of a classic theme.

Classic themes and block themes can be defined in several ways. However, here are definitions that reflect the current behaviour of WP_Theme::is_block_theme() ​source:

Block theme:
A theme that has an index.html file in the templates or block-templates directory.

Classic theme:
A theme that does not have an index.html file in the templates or block-templates directory.

Since the theme in question has a templates/index.html file, it is not a classic theme, so classic theme behaviour has not been changed. As Ari points out, in the event that it has both index.php and templates/index.html files, the templates/index.html file takes precedence.

[peterwilsoncc]

As a block theme may instead use block-templates/index.html for BC reasons ​ref, should the PR also account for this file?

Yes, it should.

I think this is a wider bug, neither of the following check for the legacy folder configuration:

• validate_current_theme()
• WP_Theme::__construct

@aristath or @poena, could I get a logic check on this, would this be problematic in any way?

[poena]

It depends on if the intention is to keep the backwards compatibility for the folder names forever or if we expect theme authors to update the themes since they were experimental?

[poena]

19 of the 70+ themes in the theme directory still use the block-templates folder, so that's not great.
But they are also mostly Automattic themes. Pinging @jffng @jeffikus

[peterwilsoncc]

Replying to poena:

It depends on if the intention is to keep the backwards compatibility for the folder names forever or if we expect theme authors to update the themes since they were experimental?

The goal is to support backward compatibility when possible once a feature makes it in to core. Given there is no technical limitation I will update my pull request.

In ​https://github.com/WordPress/wordpress-develop/pull/2692/commits/c408ff64861900cc92a31c79ec9f34e557248905 I've added support back support for the deprecated paths in each of the locations that check for block themes.

I'll add in some tests to ensure deprecated block themes are not broken.

I left some comments about this patch on WordPress Slack (link requires registration at ​https://make.wordpress.org/chat):

​https://wordpress.slack.com/archives/C037DD0CLHY/p1652748680139119

and re-sharing here for visibility. I can confirm that in other places we use both folder names for detecting whether we have a block theme:

​https://github.com/WordPress/wordpress-develop/blob/3dcdd4b799de1442fa76f3b71b84485362fce83a/src/wp-includes/class-wp-theme.php#L1484-L1496

so it feels like we should be doing the same checks in all other places. @mcsf was so kind to also look at this problem and we both arrived at the same conclusions.

Thanks for the help, folks, I think this has caught all the areas that were missing the check for the legacy folder structure.

Riad mentioned in Slack the deprecated paths are handled by the function above too. Based on some quick testing that seems to be happening in the FSE interface already.

I'm sure I could add more tests but if I can get some approvals, I think this is good to go in as is.

[gziolo]

I'm approving this patch based on my previous comment and all the code checks I performed. I also went through all test reports which confirm that everything works as intended. I also saw that @SergeyBiryukov approved PR on GitHub. Thank you Peter for bringing full backward compatibility!

[audrasjb]

After a code logic review and a quick test using a custom theme, I think we're good to go.
Adding dev-reviewed workflow keyword.

[peterwilsoncc]

This was committed to trunk in [53416] but I used the incorrect ticket reference.

In [53416] :

Themes: Accept valid block themes.

Updates theme validation to accept block themes. This replaces the requirement for an index.php with a requirement for either an index.php, /templates/index.html or the deprecated /block-templates/index.html.

Validation is updated for theme uploads, within WP_Theme::__construct and validate_current_theme().

A block theme using the deprecated file structure is now included in the unit tests.

Props peterwilsoncc, sergeybiryukov, hellofromtonya, costdev, azaozz, gziolo, FlorianBrinkmann, Boniu91, aristath, poena, audrasjb.

[peterwilsoncc]

In 53417:

Themes: Accept valid block themes.

Updates theme validation to accept block themes. This replaces the requirement for an index.php with a requirement for either an index.php, /templates/index.html or the deprecated /block-templates/index.html.

Validation is updated for theme uploads, within WP_Theme::__construct and validate_current_theme().

A block theme using the deprecated file structure is now included in the unit tests.

Props peterwilsoncc, sergeybiryukov, hellofromtonya, costdev, azaozz, gziolo, FlorianBrinkmann, Boniu91, aristath, poena, audrasjb.
Merges [53416] to the 6.0 branch.
Fixes #55682.

Merged in https://core.trac.wordpress.org/changeset/53416

https://core.trac.wordpress.org/ticket/55682 targeting the 6.0 branch.