Skins in disabled basetheme cannot be enabled for basetheme (to appear in subtheme)

We original thought this whole thing through, but just having gone through code again today I realize that there's no way currently to easily enable skins from a disabled theme (usually the parent theme is disabled).

The loading code in #956994: Write load and parse code for Skinr include files in PHP format includes the function skinr_get_directories() which determines the skin plugins that could get loaded. It will never include skin plugins for disabled themes, however.

We discussed using hook_skinr_skin_info_alter() to pull in the skins we want from a parent theme. We would need a helper function that does the following:

1. Determine whether the theme is disabled or not; if it's not disabled we can skip to step 4.
2. The theme is disabled, so we need to manually load the parent theme's skin plugins (they weren't returned by skinr_get_directories()).
3. Invoke hook_skinr_skin_info() for these newly added plugins, and perhaps filter out unwanted skins (we could pass the function an optional array of names for the skins we want included)
4. Set the status for the desired skins to enabled.

Some notes:
I'm not too happy about having to manually go back and load new plugin files. Perhaps there's a way to load plugins, but not invoke the hooks for them in the first pass of loading plugins.

Comments on this approach are appreciated (and needed).

From this I assume therefor if the base theme in enabled and I do this...

    'status' => array(
      'adaptivetheme' => 1,
      'at_opal' => 1,
    ),

...then the skin should be available to the the subtheme?

This plugin/skin is in the base theme (adaptivetheme), and the subtheme is "at_opal". So far I have only been able to enable a skin that is actually in the theme - I assume some UI is to be built that allows me to enable any skin for any theme regardless of where it resides (a rule or configuration for the skin?).

Oh my, I didn't know you can have an enabled subtheme and the parent theme is disabled. Is it not considered a bug that that is so? Could Skinr require that parent themes be enabled to use parent skins?

Most of this issue should actually be resolved through the patch we committed in #1015614: Subtheme inheritance

Unfortunately, I don't see any actual bug report, feature request, problem statement, or summary in this issue. The OP references to the issue title or something, so it's entirely not clear what's actually the point of this issue. Can I haz some, plz? Thx! :)

This may or may not be related to this issue but I have run into a number of big problems in Core and other contrib modules when setting my base themes as hidden = TRUE, this is an option we're supposed to be able to use but unfortunately is so borked for themes its just too much hassle (updates not working, Drush borking out badly etc etc). IMO Skinr should work even when this flag is set by a base theme. We should be able to completely hide base themes from end users, they should know nothing about them, unless they need to update them (not the issue here at all, just a rant...).

Documentation?

Expectation?

Manual testing reveals that this works. Didn't run tests yet.

+++ skinr.api.php	21 Jan 2011 01:15:51 -0000
@@ -199,6 +199,8 @@ function hook_skinr_api_VERSION() {
+ *   whose corresponding values denote the desired default status for the particular theme.

d'oh, exceeds 80 chars.

Passes tests for me.

+++ tests/skinr_ui.test	21 Jan 2011 01:35:13 -0000
@@ -75,7 +75,7 @@ class SkinrUIBasicTestCase extends Drupa
-  function testSkinEdit() {
+  function xtestSkinEdit() {

Crap. Sorry.

Powered by Dreditor.

When the base theme does not know what the name of the subtheme is

Slight adjustment in expectations.

Posting what I currently have, need some sleep. Previous patches contained a bogus change. But anyway, doesn't work yet.

Hopefully, this revised title cuts it.

I still need to test with that patch that makes theme for tests load, but here's something to start with:.
After some testing, I found that skin plugins for disabled themes still get loaded. Disabling that is easy (test for $theme->status in skinr_implements()). Going to play with some code to load skin plugins on the hook_skinr_skin_info_alter().

Attached patch builds on the previous one and adds api functions to enable and disable status. It adds the skinr_statuses table and removes the unused skinr_skinsets and skinr_skins tables. In addition it clears the stored skin_info and group_info cache whenever a theme is enabled or disabled to allow it to get refreshed.

The tests that sun wrote to test default status don't seem quite right; they require updates to the admin ui before they can work. I've added a function to test statuses, but it needs to be filled in (I'm still not sure on how to best test that, and it will also require an updated admin section). Temporarily the status of the current theme is displayed on admin/appearance/skinr/skins. See #984402: Add "details" link that shows preview of rendered skin widget and additional info about the admin ui changes.

This patch isn't quite there yet, but we're moving closer.

+++ skinr.module
@@ -937,8 +1006,17 @@ function skinr_skin_info_process(&$skin_infos, $source) {
+    // Insert overridden statuses.
+    $result = db_query("SELECT skin, theme, status FROM {skinr_statuses} WHERE skin = :skin", array(
+      ':skin' => $skin_name,
+    ));
+    foreach ($result as $status) {
+      $skin_infos[$skin_name]['status'][$status->theme] = $status->status;
+    }

@moonray: Sorry, but that's the exact opposite of where we need to go; this logic does not belong into the skin processing code. Skin info handling has to be entirely decoupled from actual skin configuration. In other words: We have two APIs - one for registering and handling skin information (and only that), and second and separate one for loading/saving/applying skins ("skin configuration objects" if you will).

This particular issue is about getting the basic functionality working again, so those major database schema changes are too much. Rather belong into #1029058: [META ISSUE] Change skin configuration

I'm going to study your skinr_ui.admin.inc changes, as those seem to be the only that might make a difference to the second to latest patch.

Attached patch fixes this issue. Took me some hours to figure out the inheritance logic that allows a skin of a base theme to enable itself for the base theme, and therefore, also for all sub themes of that base theme.

If this was a painting, I'd call it 'artwork'.

The ability to set the status of a skin (from plugin info, not an applied skin) has nothing to do with the ability to apply skins. I don't see how that needs to be separated.

Otherwise I tested the patch, and it seems to be working nicely. Can you explain, though, under which circumstances the base theme or sub theme status for a skin info gets enabled or disabled inherited?

+++ tests/themes/skinr_test_basetheme/skinr_test_basetheme.skinr.inc	21 Jan 2011 02:59:08 -0000
@@ -7,7 +7,10 @@
 function skinr_test_basetheme_skinr_skin_info() {
   $skins['skinr_test_basetheme'] = array(
     'title' => 'Base theme skin',
-    'default status' => 1,
+    'default status' => 0,
+    'status' => array(
+      'skinr_test_basetheme' => 1,
+    ),
   );
   return $skins;
 }

That's it, basically. There are two different and possible scenarios:

1. A base theme simply sets the default status of its skin to 1. This, however, makes the skin available to all themes currently. If that is not desired, then:
2. A base theme sets the default status of its skin to 0. Additionally, it enables the skin for itself. Doing so will trigger the basetheme/subthemes inheritance logic in this patch, which makes the skin available for all subthemes of the basetheme. — That is, because the basetheme does not know which other subthemes are installed that use it.

set the status of a skin (from plugin info, not an applied skin) has nothing to do with the ability to apply skins

I understand. However, manual administration/management of skin statuses is a much larger topic, which we need to discuss in detail in a separate issue. Based on the current code, it seems like that information was previously stored in the database. Technically, this information rather belongs into a system variable — unless we are going to revamp the entire handling of skin information to be stored in the database, too (i.e., a {skinr_info} table that'd be similar to the {system} table, tracking what exists, information and API versions, status, etc); but as of now, I don't really see a need for this yet, and TBH, if you ever dealt with bug reports and issues related to that {system} table behavior, you'd be similarly hesitant of forking/using that pattern. In the end, there's a lot to discuss with regard to manual configuration of skin statuses, and most likely also plenty of expectations (which might have changed?).

Therefore, I'd like to focus on fixing the default status of skins for basethemes/subthemes via hook_skinr_skin_info() in this issue, which currently seems to block various base theme authors/maintainers from using Skinr.

EDIT: So it seems like all that is missing for this patch is a documentation tweak?

Fixed one test failure and added some docs explaining this inheritance to .api.php.