Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parent theme not recognized when child theme active #45811

Open
bgardner opened this issue Nov 16, 2022 · 21 comments
Open

Parent theme not recognized when child theme active #45811

bgardner opened this issue Nov 16, 2022 · 21 comments
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended

Comments

@bgardner
Copy link

bgardner commented Nov 16, 2022

Description

When a child theme is active, the parent theme is not being recognized—and therefore not loading the parent's theme.json file, etc. This is only happening when Gutenberg plugin is active.

Conversation around this happening in WP Slack #meta channel: https://wordpress.slack.com/archives/C02QB8GMM/p1668569269811889

I first noticed this when I had my Powder News (a child theme) approved on the WP.org repo. You can see the preview here: https://wp-themes.com/powder-news/

For testing purposes, here are the themes:

https://wordpress.org/themes/powder/
https://wordpress.org/themes/powder-news/

Step-by-step reproduction instructions

  1. Upload parent/child themes (Powder/Powder News)
  2. Activate Powder News.
  3. View front end to see it's working.
  4. Activate Gutenberg plugin.
  5. View front end to see it's not working.

Screenshots, screen recording, code snippet

Screen.Recording.2022-11-16.at.8.30.00.AM.mov

Environment info

WordPress 6.1.1
Gutenberg 14.5

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@ndiego ndiego added [Type] Bug An existing feature does not function as intended Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. labels Nov 16, 2022
@ndiego
Copy link
Member

ndiego commented Nov 16, 2022

I have confirmed this with a simple TT3 child theme as well. Some theme.json styles from the parent seem to be coming through, but not all. Notably fonts.

@polarstoat
Copy link

This sounds like it's related or similar to #45790 that I reported too

@annezazu
Copy link
Contributor

cc @jffng in case you might have any insight here.

@bgardner
Copy link
Author

Yeah, there's a lot of weird happening here. On my Powder News demo, it seems to be more of a font-only issue, yet the WP org theme preview demo, it's all of theme.json.

@bgardner
Copy link
Author

bgardner commented Nov 16, 2022

I don't know if this is helpful, but for some reason when Gutenberg plugin is activate, the font in global styles is reverted to "Default" and when Outfit is selected, it appears to work as expected. (Though Monospace isn't listed as a font choice when plugin is active.) Watch the video below to see what is happening:

Untitled.mp4

@jffng
Copy link
Contributor

jffng commented Nov 17, 2022

I looked into this for awhile today and can confirm that some fontFamilies — ones that do not have any fontFace declarations but are "system fonts — are being stripped the merged theme.json data. I have yet to figure out where or why this is happening for child themes only.

@bgardner
Copy link
Author

bgardner commented Nov 17, 2022

@jffng This issue (while fonts are definitely one aspect) is bigger and in some cases, loads nothing from theme.json—resulting in this, which I'm about to request temporarily removal: https://wp-themes.com/powder-news/

@bgardner
Copy link
Author

bgardner commented Nov 17, 2022

Ok, so I have (what I believe) resolved the font specific issue by doing this, which I suggested as a fix in #45790:

@polarstoat Can you try something for me in Frost theme.json? Try changing "slug": "primary", to "slug": "outfit", where the font is defined, and then down in typography, change "fontFamily": "var(--wp--preset--font-family--primary)", to "fontFamily": "var(--wp--preset--font-family--outfit)",.

I had a hunch that the slug being set to primary was somehow causing a conflict.

However, there is still a major issue being seen at my theme preview on WP.org, which is essentially not loading any theme.json from the parent theme: https://wp-themes.com/powder-news/

@dd32
Copy link
Member

dd32 commented Nov 18, 2022

I've opened https://core.trac.wordpress.org/ticket/57141 as I believe at least part of the issue is contained within WP_Theme, I've been unable to duplicate any change in behaviour with/without Gutenberg on wp-themes.com, but with the PR to core above, I've been able to reliably fix the issue.

I suspect differing behaviour is due to the above issue, but potentially that it depends on which order the WP_Theme instances are fetched - if the parent is fetched first, and the cache populated, the child-theme WP_Theme will work correctly, but if the child is requested first and then the parent, it won't.

@seothemes
Copy link

seothemes commented Nov 18, 2022

Ran into this as well, it appears the parent theme.json is not being loaded.

When testing locally, deleting all of the parent theme.json (except version number) produces the same result as the wp.org previews.

I was able to find a temporary fix by adding the following to the parent theme:

add_filter( 'wp_theme_json_data_theme', __NAMESPACE__ . '\\theme_json_fix', 11 );
/**
 * Temporary fix for parent theme.json not loading in wp.org previews.
 *
 * @since 1.0.0
 *
 * @param mixed $theme_json \WP_Theme_JSON_Data|\WP_Theme_JSON_Data_Gutenberg.
 *
 * @return mixed
 */
function theme_json_fix( $theme_json ) {
	if ( ! is_child_theme() ) {
		return $theme_json;
	}

	$theme_json->update_with(
		array_replace_recursive(
			wp_json_file_decode(
				get_template_directory() . '/theme.json',
				[ 'associative' => true ]
			),
			wp_json_file_decode(
				get_stylesheet_directory() . '/theme.json',
				[ 'associative' => true ]
			)
		)
	);

	return $theme_json;
}

Another option is to add all of the missing parent theme.json to the child theme, however that also caused there to be some duplicated options in editor settings which may be a separate issue.

@bgardner
Copy link
Author

Thanks for the work/update @dd32. Where does this leave us—are we waiting on review of this: https://core.trac.wordpress.org/ticket/57141?

I'd love to start working on more (child) themes to submit to the repo, but figure it makes sense to wait this out. (Rather than having multiple themes with broken demos.)

@StevenDufresne
Copy link
Contributor

I understand it's not ideal but worth noting, if your parent theme has an index.php file it will work right now.

@bgardner
Copy link
Author

Thanks @StevenDufresne, updated Powder on repo and the demo seems to be working as expected. cc: @seothemes

It would be nice to have this fixed on the theme preview site, so that at some point, the index.php file can be removed from the parent theme for good.

@StevenDufresne
Copy link
Contributor

Thanks @StevenDufresne, updated Powder on repo and the demo seems to be working as expected. cc: @seothemes

It would be nice to have this fixed on the theme preview site, so that at some point, the index.php file can be removed from the parent theme for good.

Once @dd32's patch you mention above gets merged, it will solve it for good. The fact that it works elsewhere without it being merged is a bit of luck and probably cache magic.

@Otto42
Copy link
Member

Otto42 commented Nov 21, 2022

> function theme_json_fix( $theme_json ) {
> 	if ( ! str_contains( home_url(), 'wp-themes.com' ) || ! is_child_theme() ) {
> 		return $theme_json;
> 	}

@seothemes

Delete this code from your theme immediately, or your theme will be removed from wordpress.org, and you will be banned.

Under absolutely no circumstances should you make any attempt to change how wordpress.org works through themes or code that you can upload. That is a massive violation. If I find your theme before you remove it, you are done here.

We allow a lot of latitude on themes. If you think about it, we're essentially running your code on our systems. Targeting the theme preview website directly is an epic abuse of that privilege. Such behavior will not be tolerated.

@seothemes
Copy link

@bgardner perfect, glad it turned out to be an easy fix.

@Otto42 removing it right now, I'll have it uploaded in a few minutes. Please note I'm not trying to change the behaviour in any way, it was just a temporary fix to get the preview working correctly as it only happened in theme previews.

@Otto42
Copy link
Member

Otto42 commented Nov 21, 2022

@seothemes

I understand the reasoning. It kinda doesn't matter.

Essentially, you're targeting the wp-themes site and making it behave differently there than it does for everybody else. That's horribly bad. That's malware tactics.

Don't do it again.

@seothemes
Copy link

seothemes commented Nov 21, 2022

@Otto42 Understood, it's been removed. I've updated the code example above as well. The URL check was only there to avoid loading it when not needed, it didn't display differently in any way. It was the opposite if anything, making it display the same as it does elsewhere since only the previews were broken.

@oandregal
Copy link
Member

I tested this WordPress nightly and WordPress 6.1.1, both with and without Gutenberg trunk active. I was unable to reproduce and don't see any difference.

@polarstoat
Copy link

I can't reproduce this using the power themes as is, with Wordpress v6.1.1 and Gutenberg v14.9.0.

However, I believe the issue remains as altering the theme.json of powder to change the defined font family's slug to testoutfit and then the updating elements.typography.fontFamily to match will reproduce this.

#45790 also mentions this. Custom font family's slugs are being auto-generated from settings.typography.fontFamilies.fontFace.fontFamily and not respecting the value defined in settings.typography.fontFamilies.slug.

@webmandesign
Copy link
Contributor

I think this may be related or it is actually is the same issue (except I don't need to activate Gutenberg plugin, the issue appears on WP6.2):

When I use parent theme only, and I hook a function onto wp_theme_json_data_theme filter, I get all the relevant data from parent theme.json to work with.

Then, when I create a child theme (no need to create a theme.json in the child theme) and hook a function onto wp_theme_json_data_theme (or use the obove one from parent theme), the data I can work with are basically an empty array (this is what WordPress returns for me: [ 'version' => 2 ]).

I expected parent theme data to be untouched (or modified with a child theme.json data if I provided such file). Am I missing something?

(Thanks @seothemes for the fix!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests