http://drupal.org/node/225125 says that color.module data in themes isn't inherited. Comments on that page suggest copying color/, images/ and styles.css to the subtheme directory, which is a waste of space. That is not true: with the exception of color.inc, which must be copied, it *is* possible to inherit color.module information. For example, in Minnelli color/color.inc:

<?php
 
'css' => array(
   
'../style.css',
  ),
[...]
 
'slices' => array(
   
'../images/body.png'                      => array(0, 37, 1, 280),
   
'../images/bg-bar.png'                    => array(202, 530, 76, 14),
[...]
?>

This is made possible by line 443 of color.module, which replaces "some-directory/../" with "".
<?php
    $before
= preg_replace('`(^|/)(?!../)([^/]+)/../`', '$1', $before);
?>

Unfortunately, it only replaces one instance of '../'. This is a problem with themes inside sites/all/themes/.

To demonstrate: Copy minelli/ to sites/all/themes/myexample/, create an approporiate myexample.info, and in color/color.inc, replace '../' with '../../../../themes/garland/'. (So ../style.css becomes ../../../../themes/garland/style.css.) Recoloring it won't work: the recolored CSS will have correct colors (e.g. text color, solid background color), but all the image slices it references (gradients etc) will be wrong. Line 443 does exactly that - replace addresses of recolored image slices.

A simple, though inelegant solution is to wrap the line in a while loop. It's probably possible to do it entirely with regular expressions, but this code isn't exactly performance critical.

<?php
   
while (preg_match('`(^|/)\.\./`', $before)) {
     
$before = preg_replace('`(^|/)(?!\.\./)([^/]+)/\.\./`', '$1', $before);
    }
?>

With this change, all instances of '../' are replaced correctly and the theme can be recolored.

I heard somewhere that color.module may go to contrib in 7.x, but this is still applicable for 6.x. The fix doesn't break any API and nobody relies on the old behavior (because the old behavior was half of the page colored incorrectly).

Files: 
CommentFileSizeAuthor
#11 466268.subtheme-inheritance.11.patch792 bytesdlu
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Invalid PHP syntax in core/modules/color/color.module.
[ View ]
#1 color-regex-D6.patch562 bytesTomi
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch color-regex-D6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Comments

StatusFileSize
new562 bytes
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch color-regex-D6.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

An attempt at a patch against 6.12.

Oh, and it seems I've got the line number wrong. It's not 443, but somewhere nearby.

Version:6.12» 7.x-dev

I gather that bugs are first fixed upstream, then backported. Setting to 7.x to get more attention for this issue.

Status:Active» Reviewed & tested by the community

Patch does work as intended for Drupal 6.12. Should be rerolled for newer versions.

Status:Reviewed & tested by the community» Needs work

+++ modules/color/color.module 2009-05-18 22:44:27.000000000 +0200
@@ -429,7 +429,9 @@
-    $before = preg_replace('`(^|/)(?!../)([^/]+)/../`', '$1', $before);
+    while(preg_match('`(^|/)\.\./`', $before)) {
+      $before = preg_replace('`(^|/)(?!../)([^/]+)/../`', '$1', $before);
+    }

There should be a space after while for conformance to coding standards.

However, this fix looks rather ugly. :\ Is there really no other way to do this check?

Finally, anywhere we're dealing with regexes, we should have a brief summary comment as to what they're doing so someone can get the jist without having to read punctuation.

This review is powered by Dreditor.

This patch worked great for me, thanks

Not sure this is the right place to say this but for newbies trying to get their color module working...

after copying minnelli folder per instructions above, in color/color.inc I had to replace "../" with 5 sets of dots not four, that is

"../../../../../themes/garland/style.css"

I can only guess why, dots confuse me

Version:7.x-dev» 8.x-dev
Issue tags:+needs backport to D7

Needs a better regex.

Category:bug» feature

http://drupal.org/node/225125 says that color.module data in themes isn't inherited.

Based on that alone I'm marking this as a "feature" since it works as expected. I also agree with @tim.plunkett, this needs better regex and/or logic to actually make this work as expected.

Issue tags:+Novice

sigh, forgot to tag

Assigned:Unassigned» dlu
Issue tags:-Novice
StatusFileSize
new792 bytes
FAILED: [[SimpleTest]]: [PHP 5.4 MySQL] Invalid PHP syntax in core/modules/color/color.module.
[ View ]

Rerolled patch to D8. Discussed ideas for a cleaner fix with markcarver and cottser, so far don't have any…

The issue with the regex is that the sub theme does not need to live in the same tree as the base theme and may in fact be a sub-sub-theme.

This patch includes the fix for #276384 (and would fail if it were committed).

Assigned:dlu» Unassigned
Category:feature» bug
Status:Needs work» Needs review
Issue tags:+Needs tests

Thanks @dlu! Great patch :) Going to mark this as NR so it triggers the test bot. Also changing my mind about being a bug. Yes, from a "programmer's stance" this may seem like a feature request, but as a "themer" we tend to think/expect ../ to work in paths on a daily basis. I'm up in the air either way. @webchick could you weigh in on this, thanks :) If it's a FR, we need to move this to 9.x.

Status:Needs review» Needs work
Issue tags:-Needs tests, -needs backport to D7

The last submitted patch, 466268.subtheme-inheritance.11.patch, failed testing.

Status:Needs work» Needs review

Status:Needs review» Needs work

The last submitted patch, 11: 466268.subtheme-inheritance.11.patch, failed testing.

Status:Needs work» Postponed (maintainer needs more info)

so I have gone away for this for a while and as I come back to this issue. and re read the description. I noticed that the section for testing/reporducing this issue uses minnilli and garland as an example. these themes dont exist in d8. so this decription need to be reworked.

To demonstrate: Copy minelli/ to sites/all/themes/myexample/, create an approporiate myexample.info, and in color/color.inc, replace '../' with '../../../../themes/garland/'. (So ../style.css becomes ../../../../themes/garland/style.css.) Recoloring it won't work: the recolored CSS will have correct colors (e.g. text color, solid background color), but all the image slices it references (gradients etc) will be wrong. Line 443 does exactly that - replace addresses of recolored image slices.

Also the path at #11 needs to be re rolled