Automatically inherit theme .info settings from base themes to subthemes

I haven't checked but if your subtheme doesn't declare any regions at all, then It's definitely a bug. It should inherit what's set in the base theme.

For now, you can redeclare what's set in the base theme.

That looks good except that regions from the base theme should replace all the regions instead of going one-by-one having a mix of both the default and parent theme regions. I'm just scanning the code here and that's what it looks like it's doing.

It should also check if the current sub-theme has it's own regions set. When it does, it should skip interfering with the regions. Looks like it would add to what's already there.

Parsing the info file shouldn't be done. It seems awkward. It's usually cached and placed within the global $theme_info but that already populates the regions even when none is set IIRC. There must be another way.

Thanks for the patch, I'll try and help look into this myself. Let me also verify this bug since I never tested it.

Kevin, the patch wasn't working at all. I had some free time to work on this. Instead of focusing on the regions, this makes more sense to me.

Here are the changes:

Specific keys within the .info file will automatically be inherited from it's immediate base theme. It also cascades so it works with multiple levels of inheritance. For example, if a base theme is 2 or more levels apart and the intermediate themes do not define regions, it will inherit the regions from that distant parent. This is similar to how the rest of the system works with inheritance.

• sub-theme 5
  - regions undefined (inherited from sub-theme 3)
• sub-theme 4
  - regions undefined (inherited from sub-theme 3)
• sub-theme 3
  - regions defined
• sub-theme 2
  - regions undefined (inherited from base theme 0.)
• sub-theme 1
  - regions undefined (inherited from base theme 0.)
• base theme 0
  - regions defined

The keys with this behavior are 'regions', 'features', 'core' and 'php'. You can see all the options in the handbook. More can be added easily if needed. This would change the requirement that 'core' needs to be defined for sub-themes as it will carry over.

I'm pushing this to 7.x-dev since this "bug" was documented. I forgot that I already made those notes and changing it now could break some themes.

Typo.

This would be a nice enhancement. No takers?

This could be nice, but some properties like "core" should be defined for every sub-theme. Suppose that you have a base theme with version 'x' and a sub-theme with it's own set of template files. Now a new version 'y' of Drupal is released, and you update your base theme, but the sub-theme is not updated. How can Drupal know that the sub-theme is not compatible?

That could be an issue. Sub-themes will still be able to set its core version but I think your right. It shouldn't be automatic since updating could cause some confusion.

It's a single line change.

What do you think of 'php'? Less likely to be a problem but I'm not sure.

subscribing. Need sleep now.

Is this fix definitely making it into D7? Region inheritance would be great. Would be willing to help depending on what needs to be done.

Also, there are a number of other modules/themes who are using the .info file for settings that would benefit from being inherited:

Zen default theme settings (http://drupal.org/node/248103)
Panels 3 theme plugins (http://drupal.org/node/495654)
Skinr styles (http://drupal.org/project/skinr)

Would it make sense to have core handle .info setting inheritance in general and define a protocol for any module/theme that wants to take advantage of inheritance through the .info file?

Or perhaps define an accepted/standard method for module/theme developers to use in their own code to enable inheritance?

Or just let folks continue to roll their own solutions on a case-by-case basis?

Subscribing. +1. Anyone up for updating this issue with a comment explaining the current state of the problem (has anything changed since #11) and re-rolling a patch?

+1. It's very odd that subthemes don't inherit custom regions.

Not sure what needs to be done at this point, or if it's possible to get this into Drupal 7.

Isn't this a duplicate of #563708: Improve theme_get_setting() and make custom theme settings a true form_alter ?

Subscribe

Nope, that issue is dealing with the bug that default values for theme settings are supposed to be inherited, but aren't.

This one is dealing with the other data not being inherited.

Needs to be fixed in D8 first, then backported to D7.

How about using NestedArray::mergeDeep? We would use the base_themes .info data as default for the subtheme and then override anything wich is redefined in the subthemes .info file.

In my manual tests, this seems to work fine.

Ups, wrong status, sry -> updated

Tagging for tests.

Was asked to review this... Code looks good and is indeed a desirable feature. However, we really need tests for this.

Needs to be fixed in D8 first, then backported to D7.

I am not really sure that we can backport this at all. Potentially breaks existing sites that run with base themes.

Here are some tests. I thought it would make sense to test sub-sub-themes here as well. Leaving the "Needs tests" tag until we hear back from testbot and someone else has a chance to review.

The ThemeTest class is passing locally, but ThemeInfoStylesTest is not. I'll take a look tomorrow.

Actually the patch from #23 will make ThemeInfoStylesTest fail on its own, the test is exposing a legitimate bug. #575298: Provide non-PHP way to reliably override CSS got committed a day after #23 was posted, so that's why it showed up green here. I'll trigger a re-test.

It looks like we shouldn't be merging everything from the base theme's .info, in this case doing so breaks the behaviour of stylesheets-override and stylesheets-remove.

#23: info_inherit-301295-23.patch queued for re-testing.

The test+fix patch in #29 was only passing locally because I had a copy of the test grandchild theme in sites/default/themes for manual testing. I've updated theme_test_system_theme_info() to add the grandchild theme.

Attached are a revised test-only patch and a revised test+fix patch, as well as an interdiff from #23. I've put in a fix for the bug exposed in ThemeInfoStylesTest (see #32).

The 3 exceptions thrown in ThemeInfoStylesTest with the test-only patch are exposing this bug as well, because test_subtheme isn't inheriting the regions from test_basetheme.

I'm not sure whether this change proposal makes sense.

What if a subtheme does not want or need a region from its base themes? How would the subtheme remove inherited properties?

I think this is essential, since most base themes are developed in a very generic way — if they wouldn't, then they wouldn't be base themes.

Therefore, base themes have e.g. a full range of potentially needed regions. But as a themer with a concrete site + theme at hand, I most often don't want to inherit all of those regions, because instead of abstract/generic, I have a very concrete and specific use-case and know exactly what regions I want to offer.

In essence, with this change, you'd have to supply me a way to undo any .info property of any ancestor theme; e.g., for the case of the regions property, I'd need a regions-remove[] property.

Furthermore, certain .info properties MUST NOT be inherited to subthemes; e.g., starting with the 'base theme' property itself, continuing with others like 'hidden', etc.pp. You also cannot merge the 'stylesheets' and 'scripts' properties, since all of the specified files would be additionally loaded from the subtheme otherwise.

In turn, this proposed merge algorithm is too naive and needs to be far more sophisticated anyway:

+++ b/core/modules/system/system.module
@@ -3060,6 +3061,16 @@ function _system_rebuild_theme_data() {
+    // Inherit the data from the current base theme, and override it with the
+    // specific subtheme settings.
+    $basetheme_info = $themes[$base_theme]->info;
+    // Remove stylesheets-override and stylesheets-remove from the info array,
+    // these settings should not be inherited by subthemes.
+    // @see _drupal_theme_initialize().
+    unset($basetheme_info['stylesheets-override'], $basetheme_info['stylesheets-remove']);
+    $nestedArray = new NestedArray();
+    $subtheme_info = $nestedArray::mergeDeep($basetheme_info, $themes[$key]->info);
+    $themes[$key]->info = $subtheme_info;

Why don't we just do what the original issue title proposed? Merge the theme settings ...

Oh, ok. Sure, that's a different thing and probably more appropriate... (but not at all what the patch is doing)

That said, theme settings should actually be moved out of the .info files and be converted into config files in D8. I think we have an issue for that somewhere. Of course, that would mean a dramatically different merge process (and also different spot in the code base, in which merging can be considered).

Closed #1535904: Subtheme inheritance: logo, regions, settings as a duplicate of this issue.

This likely needs to be done in 8.1.x now.

This is an API improvement rather than a bug, moving to task. Also could use an issue summary update.