Opened 3 years ago
Last modified 3 years ago
#54910 new defect (bug)
5.9 Bug - Blank Home Page
Reported by: | cantuaria | Owned by: | |
---|---|---|---|
Milestone: | Awaiting Review | Priority: | normal |
Severity: | major | Version: | 5.9 |
Component: | Themes | Keywords: | has-patch has-unit-tests |
Focuses: | Cc: |
Description
After upgrading to 5.9, a few of my sites started showing a blank screen on Home Page when the show_on_front option is set to Latest Posts. Trying the new Visual Editor would output "This block has encountered an error and cannot be previewed"
I discovered that this occurs if you have a index.html
inside a templates
folder inside your theme folder. I believe that the new UI editor is failling back to this file when the proper file is not found. When the file is removed, everything works as before and the Site Editor is disabled.
In my case, I always have an empty index.html
file inside my themes and plugins file, to avoid directory browsing in webservers. It's not really empty, it have a HTML comment line, that's how I discovered that WordPress was loading the file.
Attachments (5)
Change History (63)
#2
follow-up:
↓ 3
@
3 years ago
I think we need a more robust way to identify if a theme *really* is a full site editing theme.
This problem was less prominent before the renaming of the block-templates folder to templates which is a more common folder name.
#3
in reply to:
↑ 2
@
3 years ago
Replying to poena:
a more robust way to identify if a theme *really* is a full site editing theme.
Would suggest to use get_theme_support() to identify theme features.
#4
@
3 years ago
I agree. My first approach to fix the issue was looking for a way to explicity tell that my theme does not support Full Site Editing. I couldn't find any, only then I started tracking to see which file from my theme was causing the conflict.
Using the default file (index.html) for this, doesn't seems really good in a backward compatibility sense, since "templates" is also a common folder name used across themes and plugins.
Maybe keeping only the "block-templates" folder or use the standard way to declare support add_theme_support()
would be more concise.
#5
follow-up:
↓ 6
@
3 years ago
One option is just to require the full-site-editing
tag, or similar, in style.css
as an explicit way to denote a block theme.
Another way is a filter for WP_Theme->is_block_theme()
. I submitted PR 2175 for #54829 which implements an is_block_theme
filter.
If a legacy theme had templates/index.html
along with templates/some_php_file.php
, it could add the following:
<?php add_filter( 'is_block_theme', '__return_false' );
Similarly, if a theme developer wanted to organise their folders differently, say fse/page.html
, they could add the following filter to make sure WP_Theme->is_block_theme()
still returned true
:
<?php add_filter( 'is_block_theme', '__return_true' );
Related train of thought
I don't know if there's currently a way to specify an alternative templates folder. Is this available?
For example, what if someone wanted to organise their folders differently say, site_templates/page.html
?
If this is already possible, does the theme still need to have templates/index.html
for WP_Theme->is_block_theme()
to return true
?
If this isn't available, I don't believe there's any existing strict theme folder naming restrictions in Core and it would be beneficial to allow theme developers to organise their directories based on what suits them. That's a separate ticket in itself, but some food for thought when implementing a solution that allows theme developers to change the outcome of WP_Theme->is_block_theme()
as needed.
#6
in reply to:
↑ 5
@
3 years ago
Replying to costdev:
If a legacy theme ..., it could add the following ...
Legacy themes should not have to add anything as they most likely never will be updated.
If new themes support new features like FSE or block-templates (as in #54917), they should use add_theme_support() - that's what this API was made for.
... it would be beneficial to allow theme developers to organise their directories based on what suits them ...
The existing API for theme features can handle things like that already, e.g. like this:
add_theme_support( 'full-site-editing', array( 'templates_folder' => 'site_templates', ) );
#8
follow-up:
↓ 9
@
3 years ago
I disagree with adding the theme support in the full site editing theme because the purpose of these type of themes is to be easier to create.
Long term, adding more requirements to the block theme would be against the end goal.
-Most theme support is already enabled for block themes.
-A functions.php file is not required for block themes, and adding this would make it required to include one.
Requiring a theme.json may be one way, but the same exception could still be triggered since classic themes can use theme.json.
The classic themes that have chosen to include html files are the exception, as these files are of a non standard format for classic themes.
The burden of this choice should not be placed on other WordPress theme authors.
Ideally WordPress should recognize the format automatically.
The second best is an opt-out in the classic theme not the block theme.
#9
in reply to:
↑ 8
@
3 years ago
Replying to poena:
because the purpose of these type of themes is to be easier to create
It is not easier for existing users with existing sites, they were there first.
would be against the end goal.
The end goal is to provide continuously working websites when updates are installed, see also Our mission:
... The basic WordPress software is simple and predictable so you can easily get started. ...
Requirement of changes in legacy themes and plugins are not predictable.
A functions.php file is not required for block themes,
A functions.php file is also not required for legacy themes.
as these files are of a non standard format
There is no standard format for own files, there are no required folders in legacy themes.
The WordPress community has decided to use the existing WordPress installation base with millions and millions of legacy themes and plugins to roll out all the new theme features into.
There would have been the possibility to fork/create a completely new project BlockPress or similar and cutting all technical debt and backwards compatibility, some also have suggested this when Gutenberg was started, but the community has decided otherwise.
There also would have been the possibility to introduce a new wp-content/block-themes/
location for themes with breaking new features, but the community has decided otherwise.
Now everybody in the complete WordPress userbase has to live with this technical debt, both existing users and new creators.
If a new theme for whatever reasons can not use functions.php to call add_theme_support(), it would be simple as a core fallback to implement a new field in style.css
header or theme.json
which the core then interprets as add_theme_support( 'full-site-editing' )
. Then the exiting API with get_theme_support() can be used everywhere (similar to #54917) to identify a full site editing theme.
An example how to add new fields in header can be seen in a recent change in WordPress 5.8, so the relevant code is already there and well tested.
Requirement of adding an opt-out in legacy themes is not an option.
#10
@
3 years ago
What's the purpose of add_theme_support()
if not used to check if a theme support a feature?
Even if WordPress never used .html files from the theme folder before, it's really irresponsible to assume that no theme will have it. It's a common file used in any web environment, so the correct assumption would be that it may contain, and it's far simpler using an existing API to verify the support than condemn all themes that use .html files.
This way of thinking may cause a lot of insecurity for developers - it's the same as say that any file, besides style.css & functions.php, in your theme, may break your site in a future WordPress update.
#11
@
3 years ago
My standpoint remains that we as theme authors are responsible for the files that we place in the theme.
WordPress expects the file to be used correctly and deliberatly.
Perhaps it could try to fallback more gracefully to the index.php in the themes root folder if there is no block markup in the index.html file.
#12
@
3 years ago
Submitted a patch for consideration based on @poena's suggestion to check for block markup in index.html
.
Edit
Updated the patch to add a check for wp_is_block_theme()
before running locate_block_template()
to prevent "Empty template: Index" from being output on the frontend when index.html
is empty.
Note:
if ( $content && ! empty( parse_blocks( $content ) ) {}
is overkill, as(bool) array() === false
.if ( $content && parse_blocks( $content ) ) {}
should be enough.
I used the former to clearly indicate the intention, but this can be changed to the latter if preferred.
This ticket was mentioned in Slack in #core by costdev. View the logs.
3 years ago
This ticket was mentioned in Slack in #core-editor by costdev. View the logs.
3 years ago
#15
@
3 years ago
- Keywords has-patch added
The patch looks good to me. It doesn't add too much unnecessary checks since the condition is checked only if such file exists and is readable 👍
#16
@
3 years ago
- Keywords needs-unit-tests added
I like the approach here, are you able to add some unit tests for a theme with an empty index.html
vs a block theme? It seems like something that could easily break in the future.
There's a number of themes included in the test suite in the tests/phpunit/data
directory.
#17
@
3 years ago
Yeah I can add unit tests 👍
I just noticed though that the patch doesn't exactly cover a populated index.html
file that has non-block HTML in it, as parse_blocks()
will parse non-block HTML as a block with blockName => NULL
. Adding a modified patch with a way to achieve this - would appreciate another review of it.
However, I believe that shortcodes also have blockName => NULL
, so this would fail if a template consisted of only shortcodes. There may be other instances where blockName => NULL
for valid block templates, but I'm not aware of any.
#18
@
3 years ago
If we're going to be adding more work to the method than just checking if a file exists, I think it would probably be worthwhile to cache this data so the check only happens once.
#19
@
3 years ago
Thanks @costdev! I think this should probably be using the WP_Theme::cache_add
methods.
@
3 years ago
Use WP_Theme::cache_get()/add()
and add the is_block_theme
to the keys to delete in WP_Theme::cache_delete()
.
#20
@
3 years ago
Patch updated - thanks @TimothyBlynJacobs!
I'll pull this to a PR tomorrow and get started on unit tests.
#22
@
3 years ago
I've created an issue upstream in the Gutenberg repo to get more insights from the editor team on block theme architectural concepts, performance, and to help prevent breaks now and in the future.
#23
@
3 years ago
@costdev
Maybe we should false !== strpos( $content, '<!-- wp:' )
instead of parsing the blocks. This method should be a little bit faster.
It's similar to what has_blocks
uses under the hood - https://developer.wordpress.org/reference/functions/has_blocks/.
P.S. Thanks for adding conditional to the get_query_template
that item was on my list.
#25
@
3 years ago
Hi there! thanks for catching that bug. I’d have loved to find this problem sooner now we can’t ask folks to change their themes because it would impact either block themes or classic themes.
On theme supports, the goal is make themes easy to created and also make them serializable (you can create without writing code) so ideally we want to remove the need for functions.php and php code entirely from block themes. Theme supports are slowly being replaced by theme.json equivalents in that sense.
Anyway, @mamaduka’s solution here seems like the simplest and most efficient one for me, and consistent with what we did before. Any downside to that?
#26
@
3 years ago
One downside of checking templates/index.html
for blocks via strpos
or has_blocks
is that a block theme that has an index
template with no blocks in it (netiher single blocks, nor template parts) will not register as a block theme. Am I mistaken?
#27
follow-up:
↓ 29
@
3 years ago
str_contains()
is PHP8+ and will cause fatal error with lower PHP.
#29
in reply to:
↑ 27
@
3 years ago
Replying to Ov3rfly:
str_contains()
is PHP8+ and will cause fatal error with lower PHP.
It will not cause fatal error. There is a polyfill version of that function.
#30
follow-up:
↓ 38
@
3 years ago
@andraganescu That's correct. A theme with a templates/index.html
file that is either empty or contains non-block HTML will not be identified as a block theme. This is a possible regression - how likely is it that a block theme's templates/index.html
would be empty or have no single blocks/template parts? - Genuine question.
In addition, a concern raised by @hellofromTonya:
Misidentified hybrid theme as a block theme, ie a theme that has classic HTML and templating but also could have blocks defined in them.
The hybrid theme issue above would not be a regression, as the mere existence of a templates/index.html
with any amount of content would misidentify a hybrid theme as a block theme.
Given that, we can evaluate the concern, decide whether it needs to be handled and attempt to handle it in this ticket, but we can also adapt is_block_theme()
to cover this later if we need more time.
#32
@
3 years ago
Replying to sabernhardt:
The block-based themes require a readable index.html file in both the 'block-templates' and 'templates' directories, so you apparently had the two directories.
index.html
file has to be present in either block-templates
or templates
directory for a theme to be recognized as a block theme. It doesn't have to be present in both directories. Please correct me if I'm wrong.
#33
@
3 years ago
Random thought: I wonder if we should not assume that the block theme is the default kind of theme and check if the theme is a classic theme instead :) as in if not classic theme then it is a block theme. Is there something to check that a theme is definitely NOT a block theme?
how likely is it that a block theme's templates/index.html would be empty or have no single blocks/template parts?
@costdev no idea but likely or not, for developers it's going to cause some stackoverflow searches because it is unexpected to require some random contents just to be able to start with the theme, and switch to it :D
#34
@
3 years ago
This is a possible regression - how likely is it that a block theme's templates/index.html would be empty or have no single blocks/template parts? - Genuine question.
I am actually building a one-page theme and using front-page.html
for the main/only template, which I thought made the most sense in this case. Currently, I have an index.html
with the Post Content block in it for testing, but I had considered making this an empty template altogether. The reason I even have an index.html
template there is because it is necessary to "register" it as a block theme with WP.
So, it's at least something I've thought about and a good question. It's not released yet, so I'll wait for this ticket to run its course before making any final decisions.
Ideally, we'd also have a "isBlockTheme":true
flag in theme.json
to specifically enable it instead of running file checks.
#35
@
3 years ago
it is unexpected to require some random contents just to be able to start with the theme, and switch to it :D
Very important point. The theme author won't be able to start with a blank file and build it out in the Editor. As @youknowriad said:
the goal is make themes easy to create and also make them serializable (you can create without writing code)
So we would have to implement a way to tell the difference between an empty templates/index.html
file in a classic/hybrid theme, and a block theme that is only starting development. Without being able to identify a theme as a skeleton theme, we would need a different approach here.
Regarding flipping the approach to non-block theme detection, I can't think of something that definitely won't be in a block theme, but definitely will be in classic/hybrid themes.
So, some thoughts:
Tag: full-site-editing
exists forstyle.css
. If a theme provides this, thenWP_Theme::is_block_theme()
should returntrue
. While we're steering away fromstyle.css
as a requirement in the future,theme.json
is currently optional.- If a theme uses
theme.json
, I can't think of an existing way to definitely identify a block theme. Maybe something likesettings.isBlockTheme: true
would be beneficial here.
I see @greenshady also mentioned an isBlockTheme
flag in theme.json
. I know we're trying to avoid asking theme authors to make a change to their themes, but at the same time, I wonder if we're at such an early stage of block themes, that changing this now will have the most minimal impact vs maybe needing it later.
#36
@
3 years ago
#37
@
3 years ago
- Keywords has-unit-tests added; needs-unit-tests removed
I have submitted a PR based on the paths outlined by @antonvlasenko. Not sure why it isn't appearing in Trac (yet).
Includes unit tests and ready for review.
#38
in reply to:
↑ 30
@
3 years ago
Here is another idea. Use scandir to check whether there are any non-html files in the templates folder, in addition to the existing check. In this case, the only false positives would occur if the theme has a templates folder with index.html and only other html files, which is not impossible but seems highly unlikely. It would be interesting to know if that would fix the false positive for @cantuaria your theme.
#39
follow-up:
↓ 40
@
3 years ago
I think @costdev's PR appropriately addresses the underlying issue, key to which is fixing compatibility with classic and hybrid themes that validly use (block-)templates/index.html
files. (Thank you to everyone who has supplied input on the complexity here.)
As noted, `Tag: full-site-editing` already exists, so any future supports requirements will have this as an option, if needed.
But by not requiring explicit FSE registration, this update is consistent with the "no code" future envisioned for theme building, and accommodates old and new themes.
Kudos for the variations in the unit tests, as well a raising the issue on Gutenberg!
#40
in reply to:
↑ 39
@
3 years ago
Replying to ironprogrammer:
I think @costdev's PR appropriately addresses the underlying issue, key to which is fixing compatibility with classic and hybrid themes that validly use
(block-)templates/index.html
files. (Thank you to everyone who has supplied input on the complexity here.)
As noted, `Tag: full-site-editing` already exists, so any future supports requirements will have this as an option, if needed.
But by not requiring explicit FSE registration, this update is consistent with the "no code" future envisioned for theme building, and accommodates old and new themes.
Kudos for the variations in the unit tests, as well a raising the issue on Gutenberg!
I don't agree. As a plugin developer, I often look at the source code of core methods to ascertain what they are doing. This patch turns a fairly readable method into something that looks quite mystifying and difficult to make sense of. Admittedly, the flowchart helps somewhat, and could be linked in the documentation, but I'm not convinced that the complexity is unavoidable here.
I also think checking tags is a mistake. Tags are for compatibility with the theme directory, which not all themes need/want to be compatible with. It doesn't appear that a precedent exists in core, as case-sensitive searches through core files for ['Tags']
and ["Tags"]
yielded no results. Plus, it duplicates exactly the same issue at hand, by assuming that any theme that happens to have a full-site-editing
tag is a block theme.
It would not be correct to imply that this confusing flowchart-based approach is the only alternative to explicit registration. I would appreciate your consideration on my scandir
suggestion above.
This ticket was mentioned in PR #2240 on WordPress/wordpress-develop by costdev.
3 years ago
#41
Based on the paths outlined by @anton-vlasenko. Includes unit tests. Ready for review.
Trac ticket: https://core.trac.wordpress.org/ticket/54910
#42
follow-up:
↓ 44
@
3 years ago
@ironprogrammer and @manfcarlo Thanks for your comments!
This patch turns a fairly readable method into something that looks quite mystifying and difficult to make sense of.
Can you identify the areas that are difficult to make sense of so that I can make it clearer?
It duplicates exactly the same issue at hand, by assuming that any theme that happens to have a full-site-editing tag is a block theme.
The Handbook entry for theme tags states:
Full Site Editing – support for block content areas (experimental)
and it links to the Handbook entry for Block themes.
The PR doesn't aim to require theme authors to add Tags: full-site-editing
to their themes. It aims to take advantage of a theme that explicitly identifies itself as a block theme.
However, the patch is still a work in progress and the progress we've made means that the full-site-editing
tag was, until now, the only way to identify a block theme when there is an empty (block-)templates/index.html
file.
Use scandir to check whether there are any non-html files in the templates folder, in addition to the existing check.
I'm interested in possibly including this approach. Before I go and implement this and add unit test data, can we get some more opinions on this approach from both architectural and performance perspectives?
Note: Performance is a key concern here. If we do implement this and, in future, the list of valid index templates files includes other files in the (block-)templates
trees, then at that point a $directories_to_skip
mechanism should be included for this check. I can implement this now, but I'm hesitant to add code that doesn't apply to the files we're currently checking for.
In this case, the only false positives would occur if the theme has a templates folder with index.html and only other html files, which is not impossible but seems highly unlikely.
Absolutely, it is not impossible - but I agree that it's highly unlikely. I couldn't find any themes in the theme directory that have this, for example.
#43
@
3 years ago
@Ov3rfly:
Would recommend to use native available PHP functions as suggested in #23.
Sorry, I just saw this comment.
While I typically agree to use native functions:
- Allowing contributors to write code using modern PHP functions makes the contributor experience more consistent with modern development, and lowers the learning curve for newer developers.
- As long as you evaluate the modern PHP function before polyfilling it, and you follow its spec when writing the polyfill, it should offer benefits in performance and/or stability and/or readability.
- As we move to higher minimum PHP versions, the codebase remains intact except for updating
wp-includes/compat.php
. - We tested the polyfill extensively. That said, those interested in doing so can test it for discrepancies with the native
str_contains()
in PHP8+.
#44
in reply to:
↑ 42
@
3 years ago
Replying to costdev:
Can you identify the areas that are difficult to make sense of so that I can make it clearer?
Just the way that 5 different checks plus a caching mechanism are required, simply to determine whether a theme is a block theme or not. Intuitively, I would think it can't possibly be that hard.
My suggestion, which may not work, but assuming it does, would make for simpler and more readable code. It would only need to perform scandir
, check whether all the file names end in .html
, and check whether one of them is called index.html
. It wouldn't need to parse blocks, check substrings, check the file content, check the theme tags or check for theme support.
I think you also misread my point about the full-site-editing
tag, or maybe quoted the wrong part. In the part that you quoted, I was saying that if some hypothetical theme from 2012 happens to have the full-site-editing
tag for some reason, maybe one that is outside of the theme directory, it would be interpreted as a block theme, despite being made 10 years before block themes were supported. That is not only unwanted, but the exact same problem as the current approach.
#45
@
3 years ago
@manfcarlo Thanks for clarifying!
So:
- we check the cache. If it exists, we return the value, saving resources.
- we check that a
block-templates/index.html
ortemplates/index.html
exists and is readable.- If none exists and is readable, we consider it a classic/hybrid theme.
- For the
index.html
files that exist, we scan the directory for files that don't end in.html
and are not.htaccess
orweb.config
.- As soon as a 'disallowed' file is found, we skip to the next iteration/end the loop. If this happens for all possible
index
files, we consider it a classic/hybrid theme. - Otherwise, if no 'disallowed' files are found, we consider it a block theme.
- As soon as a 'disallowed' file is found, we skip to the next iteration/end the loop. If this happens for all possible
Let's look at the case that opened this ticket: A templates/index.html
file exists in a classic theme.
With the checks outlined above, this will be identified as a block theme only if index.html
exists/is readable and there are no other files in the directory. The only time this is likely to occur is when the author has just created the directory.
I think it's safe to have a false positive in this instance until the theme author adds files to this directory, as it's likely a matter of seconds until a .php
file is added.
For classic themes that use templating engines, these tend to be within inc
, assets
or other folders. I did a cursory scan for a common templating engine substring in the wp.org themes directory and reviewed the 40 themes that appeared in the results. None of them use the templates
directory. For other scans, a templates
directory was used, but didn't use .html
files.
The one scenario I can think of is a newly initialized boilerplate where the additional files may not be added until later in the theme's development. However, I'd like to think that any boilerplate worth using would have more than just an index.html
file in its templates
folder. Given that, I think this implementation is safe enough in accuracy.
I've prepared a PR for this implementation, including unit tests, so that we can review and compare the implementations for accuracy and performance.
I'd appreciate everyone's opinions on this to see where we stand, as I'd like to see our agreed changes be considered for 5.9.1 if possible.
This ticket was mentioned in PR #2247 on WordPress/wordpress-develop by costdev.
3 years ago
#46
This PR proposes that:
- we check the cache. If it exists, we return the value, saving resources.
- we check that a
block-templates/index.html
ortemplates/index.html
exists and is readable.- If none exists and is readable, we consider it a classic/hybrid theme.
- For the
index.html
files that exist, we scan the directory for files that don't end in.html
and are not.htaccess
orweb.config
.- As soon as a 'disallowed' file is found, we skip to the next iteration/end the loop.
- Otherwise, if no 'disallowed' files are found, we consider it a block theme.
Trac ticket: https://core.trac.wordpress.org/ticket/54910
3 years ago
#47
Removed the fix for get_query_template()
as this turned out to be the fix for another ticket and is more appropriate to include there instead.
3 years ago
#48
Removed the fix for get_query_template()
as this turned out to be the fix for another ticket and is more appropriate to include there instead.
#49
@
3 years ago
Thank you for continuing to explore this, @costdev and @manfcarlo.
While PR #2247 avoids parsing index.html
, in favor of checking for other files while identifying block themes, it raises a couple of conerns:
- In the context of the reported issue for this ticket -- having an existing
templates/index.html
file that doesn't contain blocks -- the theme is still incorrectly classified as FSE. - Going in the opposite direction, any implicit disallowed files would break FSE support. For instance, if an FSE theme included a
.md
or.txt
reference file, or had a "shelved" HTML file, like.html.bak
during development, as well as numerous other dotfiles that might sneak in. This could unintentionally hamper the FSE theme building workflow.
The implication with this approach is that block theme templates must ONLY have .html
files in their directories, and that classic/hybrid themes must never do this. But as unconventional as it may appear, there are no rules against structuring a classic theme to use HTML in a templates
folder, nor against plugins leveraging that folder for templated overrides built in HTML. These are only problematic now that FSE has come along with new assumptions.
Given the flexibility with theming afforded by WordPress, I'm not sure how we can know the intent of such files without parsing templates/index.html
, when it exists. Since FSE themes do not need to "register" themselves, and there are no rules restricting subdirectories, it is up to WordPress to best understand their intent. The logic is slightly more complex, but PR #2240 is in a better position to make the determination as to whether the theme is intentionally targetting FSE.
#50
@
3 years ago
@ironprogrammer I agree that PR 2247 relies too much on .html
or other "allowed" files. I couldn't find any examples of a classic/hybrid theme using templates
only for .html
files, but I agree that it's possible and would misidentify these as block themes.
Regarding PR 2240, there's one thing I'd like to resolve, if possible.
An empty index.html
file will only activate FSE when a full-site-editing
tag is specified in the Tags
header in style.css
.
While an empty index.html
may be seen as a rare event, as it would soon be populated with block markup, this will often not be the case.
For many (most?) authors, the index.html
file will be empty until they go into the Editor, add some blocks, then copy and paste the markup into index.html
. However, with PR 2240, they can't access the Editor until they add the full-site-editing tag
.
This means a basic block theme that will primarily be developed within the Editor would be initialized as such:
style.css: /** * Theme Name: My Block Theme * Tags: full-site-editing */ --- index.php: (blank) --- templates/index.html: (blank)
This solution isn't ideal, as it effectively requires a block theme to register itself as one - something that's been avoided up until now. Some mix of parsing index.html
and scandir
would be too much of a performance hit IMO, even with caching.
Since the only two files required for all themes are index.php
and style.css
, I can't see a reliable way to identify an initialized block theme that has an empty index.html
file without requiring some kind of 'registration' mechanism.
Can anyone see a way that a block theme might be identified based on something other than filenames/index.html contents, but without requiring a 'registration' mechanism?
#51
@
3 years ago
Are block themes able to go without theme.json
? If not, they could be identified by the existence of both templates/index.html
and theme.json
.
This ticket was mentioned in Slack in #core-editor by andraganescu. View the logs.
3 years ago
#53
@
3 years ago
This really got complicated, so complicated that it needs a drawing to figure it out. There are currently under 50 block themes and we appear to try and fix a problem that block themes have by fixing how the system works instead of the problem at hand. The problem appears to be that that the original idea to consider a block theme any theme that has a file named "index.html" in a folder named "templates" was not bullet proof. Can't we fix that, I wonder, by:
- asking the relatively, by comparison, few available block themes to explicitly declare themselves as block theme,
- in a way which is compatible with our desire to make block themes "easy to be created and also make them serializable",
- providing an escape hatch for users of block themes that don't update themselves?
I think that if we come up a good spec of how a block theme says "I am a block theme" and put up a dev note people will have time to update, as this whole new feature in WP is quite young. It is a better path than to surround one bad assumption with more relatively benign but potentially bad assumptions just to shield the original bad assumption.
#54
@
3 years ago
I agree with @andraganescu above but I also have doubts about the lifetime of the block theme test. In the long run, I imagine it will be more sensible to hide/show different features based on the more specific checks, i.e. WP_Theme_JSON_Resolver::theme_has_support()
for global styles and current_theme_supports( 'block-templates' )
for templates/parts/navigations, and phase out the use of wp_is_block_theme()
altogether. Introducing a new declaration for themes should only be done if it's certain to be needed for the long term, and I think this was the reasoning for not introducing it thus far.
#55
@
3 years ago
I think best to keep any solution here simple and then respond to further feedback as it arises.
The str_contains
approach seems reasonable to me. Using parse_blocks
will add an undesired performance hit and using scandir
will not play nice with files such as .editorconfig
, .DS_Store
, etc.
If the issue is not particularly prevalent, then another option is to simply do nothing.
#56
@
3 years ago
I agree with @andraganescu, to quote 'The Zen of Python':
Explicit is better than implicit. Simple is better than complex.
Inferring the information using a flowchart is implicit and complex, declaring it via a constant or theme.json entry is explicit and simple.
#57
follow-up:
↓ 58
@
3 years ago
There seems to be no consensus on how to move forward best. Recognizing @costdev 's effort to patch this ticket, and everyone else's input, should we just give this some more time to see if it is an actual problem? Maybe the number of times some classic theme is having this problem is extremely low and simply documenting the issue solves everything?
#58
in reply to:
↑ 57
@
3 years ago
Replying to andraganescu:
should we just give this some more time to see if it is an actual problem? Maybe the number of times some classic theme is having this problem is extremely low and simply documenting the issue solves everything?
Thanks for raising this question. Yes, my vote goes to giving this more time, especially given the impact of any of the proposed measures, and the uncertainty around affected sites and user expectations.
Hi and thanks for the report!
The block-based themes require a readable index.html file in
bothone of the 'block-templates' and 'templates' directories,so you apparently had the two directories.To avoid directory browsing, you could switch one or both of those index files to an index.php file.