Themes cannot have a preprocess function without a corresponding .tpl.php file
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | theme system |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Many times, sub-themes just need to tweak a variable in a tpl file and they don’t need to modify the tpl file's contents.
For example, if a sub-theme wants to modify the $submitted text, it doesn't need to modify the html/php in the base theme’s node.tpl.php.
Unfortunately, the auto-discovery phase of the theme registry building process first looks for tpl files in a theme and then for the corresponding preprocess functions. So if there isn't a tpl file, the theme registry building process won't look for a preprocess function.
No patch yet, but the way I envision this working is similar to the work-around that Zen 6.x-1.x has (#249532: Allow subthemes to have preprocess hooks without tpl files.). Grab the existing theme registry and loop through the known hooks (those that have tpl files), while looking for possible preprocess functions in the theme.
#1
Hm, this might qualify as a D6 bug indeed.
#2
This limitation is documented. A fix would be nice though.
Read the middle of this page in the list of notes:
http://drupal.org/node/223430
#3
Since the theme guide is amazingly accurate and based on the actual implementation (warts and all) and not based on the design, all the documented "limitations" fall into the "maybe this a bug" category. :-D
Let's see what the patch looks like and then decide whether this should be backported to D6.
#4
I am in favor of fixing this in D6 if the fix can be done without breaking the existing API, because it is a limitation that was not intended and causes quite a bit of acrobatics to get around.
#5
Please, please, please put this in. In D7 certainly, and it would be nice in D6 as well, so we don't need to wait a year for the awesomeness.
#277063: The next generation of DHTML Menu
#6
Here is a patch for D6.
I'd submit the D7 patch first, but I've tested this one.
#7
@Arancaytar: The patch is missing... ;)
#8
And in this case I can't even blame d.o. While rolling it, I got caught up in something and actually forgot. :P
#9
The patch is for Drupal 6, but succeeds on HEAD with offset. Still testing if it works too.
#10
I've ported my new DHTML menu module to D7 for testing purposes, and it works. Unfortunately my server is incapable of running simpletest reliably.
#11
I still don't know whether this patch will be allowed into Drupal 6 or will have to wait for Drupal 7...
#12
This is a documentation issue not a code one. Functions should have no preprocess for performance concerns. http://www.lullabot.com/articles/overriding-theme-functions-in-modules for a way .
#13
This has nothing to do with theme functions, it has to do with theme templates, chx.
#14
<?php@@ -596,7 +596,27 @@ function theme() {
}
if (isset($info['function'])) {
// The theme call is a function.
- $output = call_user_func_array($info['function'], $args);
?>
Huh what? This has nothing to with theme functions???? I am confused.
#15
chx: Arancaytar's patch is wrong, but that's no reason to bury the whole issue. The actual issue is about themes having preprocess functions but using an ancestor's template.
Arancaytar: Your patch is wrong. It shouldn't be intercepting theme functions. chx is right on that part; theme functions need to have the speediest route possible.
#16
IMO the proper way to fix this bug is have an additional phase in phptemplate_theme() that searches for preprocess functions that match items in the $existing array that are not in the array that it's returning.
#17
Wow this ridiculously sucks *g
I was just trying to override preprocess_forum_icon without having to modify the (incredibly complex ;) template file because my theme did not yet need any and i wouldn't want THAT one to be the first.. Well now indeed it is!
after putting debug statements into the theme.inc and noticing it looks for _preprocess hooks for EVERY enabled theme (of course it does) but not for themename_preprocess_$hook i can hardly see how not doing it for the theme aswell is a performance issue. The patch imho would be to simply add
$prefixes[] = $theme;in theif ($type == 'module')block.. this would of course require to have the correspondent _theme_process_registry call actually provide the $theme as 4th parameter (this itself probably qualifies as bug, invoking $module->name as $theme parameter is clearly bogus)..anyways, at least in the documentation (#223430: Preprocess functions) putting it in as the last of several notes is as good as not documenting it at all.
#18
Subscribiing. Because I also had this problem when trying to make my first theme in 6.x. Intuitively I thought that I cound preprocess vars for a template in template.php without having to copy the template to the theme when I'm not modifiying it.
#19
This is a patch to 6.x to fix this issue. I think this is an important bug and I am personally responsible for letting this sit far too long; I've known how to fix it but I was unable to adequately explain the process.
This patch adds an extra phase in registry processing and specifically looks for preprocess functions for themes that do not register template overrides. I've tested this on a 3 level subtheme and it works quite well.
#20
How to test:
If you don't have an easy to access subtheme already set up, here is a simple one you can do:
sites/all/themes/marland/marland.info:
name = Marlanddescription = Site specific Garland
base theme = garland
core = 6.x
stylesheets[all][] = marland.css
sites/all/themes/marland/template.php:
<?phpfunction marland_preprocess_page(&$vars) {
$vars['title'] = "If you see this, the test passed.";
}
?>
If you like, copy themes/garland/logo.png to sites/all/themes/marland so you get a logo.
Then set your theme to Marland. The title of all pages should be modified with this test, even though you have no page.tpl.php.
If you make a similar test mod to an existing theme, be sure that:
1) You choose a preprocess that has no associated tpl.php (or theme function) for that theme and
2) You clear the cache.
#21
Subscribing.
#22
Naming the preprocessor after the sub-theme will still omit it. I tested with minnelli with "minnelli_preprocess_block" and it was ignored. Using "phptemplate_preprocess_block" did work though.
#23
dvessel: That's weird, the test I gave uses a subtheme name, and it worked for me in several different attempts. Are you sure you cleared cache?
#24
Yeah, I did and just tested again with the example you provided. Same situation here.
I am using the latest Drupal 6 from CVS.
#25
Ok, the first patch required the presence of hook_theme in the template.php which I hadn't realized. Moving the clause outside of that function_exists() (and providing a default array to $result) fixes that.
#26
Okay, this is fixed but there is another issue when the base theme uses the name of the theme, the preprocessor will register twice.
I tested with Garland by renaming "phptemplate_preprocess_page" to "garland_preprocess_page" resulted in this:
array'template' => string 'page' (length=4)
'path' => string 'themes/garland' (length=14)
'type' => string 'base_theme_engine' (length=17)
'theme path' => string 'themes/garland' (length=14)
'arguments' =>
array
'content' => null
'show_blocks' => boolean true
'show_messages' => boolean true
'theme paths' =>
array
0 => string 'modules/system' (length=14)
1 => string 'themes/garland' (length=14)
'preprocess functions' =>
array
0 => string 'template_preprocess' (length=19)
1 => string 'template_preprocess_page' (length=24)
2 => string 'garland_preprocess_page' (length=23) // first run,
3 => string 'garland_preprocess_page' (length=23) // repeat!! Could cause problems on second run!
4 => string 'marland_preprocess_page' (length=23)
#27
The only change here is the addition of an array_unique. Worked in my testing.
#28
Hmm. Does garland get called for both base_theme and theme? How does that even get into the array twice?
I suppose array_unique is a good enough safety to catch any issues.
#29
It looks like it's already added before running into your new block of code when looped in the "theme" phase. It's within $cache when it loops through the "theme_engine" or "base_theme_engine" phase.
I want to note that this applies to base themes too when they have no .tpl files, not just sub-themes from it's parent. This could get confusing when using path_to_theme() as it will always point to where the template is located. It's not as bad as splitting off of suggestions from its base template but I'm sure this will cause some head scratching. I'll update the docs when this gets in.
It would also be great if we could redo path_to_theme or simply rename it. themed_path() pointing to the template? And maybe path_to_theme() pointing to the theme. Dunno..
#30
Redoing path_to_theme() is probably only doable in Drupal 7, but I agree that it should be done.
#31
Since you think array_unique is okay, marking RTBC. Everything worked while testing.
#32
+1 to finding a solution for this original issue. I haven't tested the patch but I agree with the logic of why it's needed. This is an issue that will affect themers who want to take advantage of base themes (which, quite frankly, will be everyone once they discover how AWESOME it is!). The assumption I propagate in the documentation I write is that subthemes inherit everything from their base (parent) theme and that you only need to add things to the subtheme if you're actually altering their output. Forcing people to copy an unaltered file into the sub-theme's directory is counter-intuitive (for lack of a better word).
#33
Ok, this sounds like something we definitely need to fix, however:
- Merlin's patch doesn't apply for me.
- Dvessel's applies, but puts things in weird places. The $result = array(); appears in the middle of two function declarations, and the other stuff is all out-dented and weird.
Is it possible to re-roll this for 7.x?
#34
And actually let's fix this in the proper order. :)
#35
OK we figured out the conflict (silly concatenation rules).
I ran through the test in #20 (other than making 6.x into 7.x) and confirmed this patch fixes the bug.
Committed to D7. Moving back for consideration in D6. Here's the 6.x and 7.x version of the patch I committed.
#36
Hm. Actually the 6.x version needs to be ported for 6.x's concatenation rules come to think of it. Should be a quick re-roll.
#37
Re-rolled Angie’s patch with the
properfubar-6.x string concat rules.#38
Minor, but there's a comment in there that also needs to end with a period.
#39
Gah. How did I miss that? I am clearly slipping. :P
I guess we'll need to catch that in a monster "Add periods to all comments that don't have them" D7 patch.
#40
Re-rolled 6.x with period.
And the 7.x patch is my entry for Trivial Patch of the Month!
#41
Looks good, moving to RTBC, will review even more later.
#42
Committed to 6.x, thanks. The final trivial patch against Drupal 7 still stands.
#43
LOL.
Committed to HEAD. :)
#44
As soon as 6.7 is released, the docs need to be updated. http://drupal.org/node/223430
I'm just making this note here to remind myself or anyone else if I forget.
#45
Automatically closed -- issue fixed for two weeks with no activity.
#46
With patched D6.8 phptemplate_preprocess_node still gets ignored for me if node.tpl.php is not within the theme directory.
#47
@kramssov: Don’t use the phptemplate_ prefix for your preprocess functions (or theme functions). It causes all sorts of issues. Use mytheme_preprocess_node instead.