Disable themes when theme engine or base theme aren't available

Themes without an engine can be created. See Chameleon.

Never mind.. Misread, sorry.

Here are all the .info options.

There's the "base theme". If that's defined but doesn't exist, shouldn't we have the same behavior?

Subscribing. This is a common problem with Zen sub-themes. “Oh. I need to install the Zen theme, too?” :-p

This should be backported to D6 too, IMO.

Subscribing. +1 for back-porting to D6.

Another component which may be listed elsewhere but I haven't seen is that the plugin manager happily downloads and installs a sub-theme leading to all sorts of errors if the base theme isn't already installed. Ideally this would be handled in a way similar to module dependencies.

Marking as major because the effect on site administrators will be significant.

New version of patch againsy current head that checks the engine and base theme presence.

New version of patch that no longer makes the theme active by default and instead prompts the user to visit the admin page to enable it. This then treats themes the same as modules. The validation on the admin page then ensures that we can't enable an unsupported theme.

adding tag

Moving to d8 as we are not going to get this in due to string changes

Per #1050616: Figure out backport workflow from Drupal 8 to Drupal 7, major and critical bugs are now preventing development in other areas of Drupal core. Unless this issue is actually a bug (data loss, functionality not working), we can mark this as a "major feature request" or a "normal bug", but not a "major bug".

Just taking a look at this as it happened to me today. I've rewritten some of the other patches in this queue and made a patch for D8.
First core patch so I hope this works!

Just altered the order a little bit to make it more consistent with existing code.

#17: disable-themes-when-no-base-or-engine-304540-17.patch queued for re-testing.

Re-rolled this patch to make it work correctly under Drupal 8 dev, and tested the functionality with both a sub-theme with a missing base theme, as well as with a theme that was missing its engine.

Not quite sure a reroll was required to rename elements in an array...

@typhonius: Yes, the reroll was necessary, since the array key for base theme name is base theme, not base.

This patch works. I'm working on adding a test case for it now.

Two files are attached here:

• 304540-23_test_only.patch contains only the test case, so it will fail tests.
• 304540-23_fixed.patch contains the test case and the fix from #20, so it will pass tests.

I uploaded patches in the wrong order, so the test-only patch triggered a status change when it (correctly) failed.

Last one, I promise: the last patch mis-used t() in the test case.

#26: 304540-26.patch queued for re-testing.

I tested this patch, it applied and now instead of 'Enable' 'Enable and Set default' I see 'This theme requires the theme engine ets to operate correctly.'

Looks good to me. Committed/pushed to 8.x.

Followup here: #1726810: Theme page test module not marked as "hidden"

Also, was this supposed to be backported? I see string additions, but no string changes, and it is a bugfix. (#14 moved it to D8 without tagging for backport on account of strings.)

In principle it seems backportable to me.

There's an issue with this when the you have a subtheme of a subtheme.

Let us use 3 themes: A, B & C

C is a subtheme of B, which is a subtheme of A.

Assume themes B and C are present on the site yet A is not. The code in system.admin.inc only checks the parent base theme, not any further up the chain.

$theme->incompatible_base = (isset($theme->info['base theme']) && !isset($themes[$theme->info['base theme']]));

A real world example of this would be if someone created a rubik subtheme and then downloaded rubik but not tao. The theme page would allow the user to enable the rubik subtheme (but not enable rubik).

Although my patch doesn't cover tests, it does have some logic in it that detects whether all base themes for a theme are present.

As an explanation:

I noticed that $theme->base_themes is an array of base themes which, if the base theme is not present the value is NULL for the key of that base theme. By using array_filter with no arguments it removes any empty elements hence making the array different to that of a non filtered array of base themes.

Added a patch that includes a test for the above. I've not written a test for core before so I've piggy-backed on the test that was committed to HEAD previously for detecting invalid base theme with my basic test knowledge.

#36

+++ b/core/modules/system/tests/modules/theme_page_test/theme_page_test.module
@@ -16,6 +16,7 @@ function theme_page_test_system_info_alter(&$info, $file, $type) {
-}
\ No newline at end of file
+}

-}

No newline at end of file

+++ b/core/modules/system/tests/themes/test_invalid_basetheme/test_invalid_basetheme_sub.info
@@ -0,0 +1,5 @@
+base theme = test_invalid_basetheme ¶

trailing whitespace

Removed the whitespace and had a go at adding a newline. Not sure if that worked though...

#38: 304540-38.patch queued for re-testing.

Reroll and retest.

Since I'm adding in another test I've been advised by larowlan to test a patch that includes just the new test and a patch that includes the new test and the fix.

This is to ensure that the test is actually testing what is being fixed.

Re-rolled.

Rerolled

Patch applies.

The reroll above only includes the test, not the fix.

Rerolled

rerolled patch

Rerolled

Re-rolled patch from comment #42

code totally outdated so manual reroll

+++ b/core/modules/system/tests/themes/test_invalid_basetheme_sub/test_invalid_basetheme_sub.info.yml
@@ -0,0 +1,6 @@
\ No newline at end of file

please add the line break

added the line break

Looks ready

Committed/pushed to 8.1.x and cherry-picked to 8.0.x, thanks!

Moving to 7.x for backport.

Patch rerolled for 7.x

Missed a few files there.
Here is the full reroll for 7.x

Test fix.
Tested Reroll for 7.x