We hard code the path themes/engines and thus don't allow engines to reside in the sites directory .. Discovered while porting the fine PHPTAL engine to 5.0 - http://drupal.org/node/105702

CommentFileSizeAuthor
#4 patch_1811.47 KBmoshe weitzman
patch_1781.51 KBmoshe weitzman
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drumm’s picture

Status: Needs review » Needs work

- Remove commented debug code
- While we are here, those changed lines need to punctuate '. $variables .' according to code style.

What is a good way to test this? Does PHPTAL in contrib HEAD work well enough?

ChrisKennedy’s picture

I don't know much about theme engines so I can't really comment on the merits of the patch (it looks good to me), but just to nitpick the debugging line needs to be removed and the string concatenation spacing needs to be fixed.

Dries’s picture

Why is this critical? It's not a regression compared to Drupal 4.7, is it?

moshe weitzman’s picture

Priority: Critical » Normal
Status: Needs work » Reviewed & tested by the community
FileSize
1.47 KB

i made the requested changes, and downgraded priority.

eaton’s picture

This isn't a regression; rather, it's a feature that is intended to work in a particular way (allowing themes, modules, engines, etc to be stored in sites/all rather than the root drupal directory). It's not working in that intended fashion, mostly because we never tested it with additional non-core theme engines.

It changes no APIs, as none of the code involved is used externally; it just makes things work the way we say they do in the documentation, etc. :-)

Dries’s picture

Eaton: I know and we should fix this. It's just that it doesn't have to hold up a release, IMO. Critical bugs are meant to be showstopping bugs.

eaton’s picture

Fair enough -- normal is probably the best priority level for it. :)

drumm’s picture

Status: Reviewed & tested by the community » Fixed

Committed to HEAD.

Anonymous’s picture

Status: Fixed » Closed (fixed)