Skinr failing to load CSS files when added at the option level of a Skin

You need this in the .info file:

skinr[api] = 2
skinr[directory] = skins

You also need to make sure these skins are in a "skins" directory in the theme.

It would also make more sense to put this at the skin level, instead of the option level, but it should work either way:

'attached' => array('css' => array('wires.css')),

I actually looked at this, and it seems the bug only exists when the css is attached at the option level.

Well, Skinr UI tests are failing locally. :(

Looks like this is a fundamental problem with our code starting in skinr_skin_get_files(). A lot of this code was written to facilitate JS live preview functionality, but since we don't currently have that I'm going to strip out the offending code. We can revisit this, and figure out a better way when we implement JS live preview in D7.
EDIT: Looks like all of this needs to be refactored to use drupal_process_attached(). Can someone tell me how to use that function, though? It confuses me. :-$ Which $elements do you pass to the function?

OK, figured it out.
This patch removes the skinr_skin_extract(), skinr_skin_get_files() and _skinr_in_multiarray(), integrating them into skinr_preprocess(). It now uses drupal_process_attached() properly.

I can't help but feel this code can use some optimizing, though, so if you have some ideas, please chime in.

Uh... and the patch.

EDIT: Ran tests locally. All tests successful.

Committed! Thanks :)

Leaving the status as fixed for now, but there are a couple of remarks for this patch:

+++ skinr.module
@@ -83,6 +83,7 @@ function skinr_preprocess(&$variables, $hook) {
+  $skin_info = skinr_get_skin_info();

@@ -90,204 +91,58 @@ function skinr_preprocess(&$variables, $hook) {
-      // @todo Kill skinr_skin_extract(), and only store what has been
-      //   configured, enabled, and should be actively applied, in order to use
-      //   drupal_process_attached().

Although we're using drupal_process_attached() and killed skinr_skin_extract() now, this @todo is still the most critical one. I'm not sure whether there is already an issue for it, but it basically boils down to the task of removing that skinr_get_skin_info() and other function calls, which involve to retrieve and load lots of data into PHP memory for every. single. theme. function. that runs.

+++ skinr.module
@@ -90,204 +91,58 @@ function skinr_preprocess(&$variables, $hook) {
+      if (!is_array($sids)) {
+        $sids = array($sids);
       }

It's important to know that is_array() is a quite slow operation in PHP; which matters when code is executed hundreds or even thousands of times in a single request. Given that knowledge, I wonder why $sids can not be an array sometimes, and, whether we cannot enforce it to be an array in some other location, or, just make your Drupal site die in hell if it some plugin or skin doesn't return an array.

+++ skinr.module
@@ -90,204 +91,58 @@ function skinr_preprocess(&$variables, $hook) {
+              if (!is_array($skin_options)) {
+                $skin_options = array($skin_options);
               }

Same issue with is_array() here. In this case, I wonder why it's needed though - I don't think the API docs state that an option can be a plain string or do they? (that would be wrong)

Powered by Dreditor.

@sun:
1) I completely agree with you on removing skinr_get_skin_info() and minimizing the amount of code used in the preprocess function. I believe #1082842: Update storage of skin configurations to give more granular control moves us a step closer, but there will still need to be a decent bit more done to optimize performance.

2) Checking for $sids to be an array should be possible to remove. We'd need to update all the functionality plugins to require arrays be returned, but that's not a big deal. (marking as needs work for this issue; the above issue is a bigger one and should be addressed elsewhere)

3) Same with checking $skin_options to be an array... I'll have to trace that one back to all its origins, but we should be able to fix that too.

1. See #1147936: Decouple having to load skin info when we only need to apply skins.
2. Calls to is_array() have been removed with #1082842: Update storage of skin configurations to give more granular control.
3. See 2.