Themes can't use node-story.tpl.php without node.tpl.php

eaton - July 7, 2008 - 20:30
Project:Drupal
Version:7.x-dev
Component:theme system
Category:bug report
Priority:normal
Assigned:Unassigned
Status:needs work
Issue tags:DrupalWTF
Description

Deep in the guts of the theme system, the 'base' template file for a theme function is used to check what directories should be scanned at runtime. For example, if a theme contains node.tpl.php, it will be checked for template files matching any of its suggested variations (node-blog.tpl.php, node-story.tpl.php, node.tpl.php) whenever a node is themed. Unfortunately, if the theme ONLY contains template files that match suggested variations -- and not the underlying 'base' theme function -- all the variations are ignord.

The most obvious way to trigger this is to rename garland's node.tpl.php file to node-story.tpl.php, and rebuild the theme registry. NO nodes -- not even story nodes -- will use that template. If both node.tpl.php AND node-story.tpl.php exist, both will be recognized and used properly. This issue is similar to #258089: Themes cannot have a preprocess function without a corresponding .tpl.php file but has a couple of additional wrinkles.

#1

yched - July 7, 2008 - 20:33

Yep, I just noticed that as well when writing theming instructions for CCK field template overrides.

#2

eaton - July 7, 2008 - 20:49
Status:active» needs review

This may not be the cleanest solution -- I'm looking closer to see if there's a way to avoid checking the $theme global -- but it seems to work for the 'node-story.tpl.php' case.

In a nutshell, *if there are suggestions* the theme system always checks the current theme directory for matching templates.

The downside is that it only fixes the problem for *the currently active theme* -- if a theme only has node-story.tpl.php and its CHILD theme is currently active, the problem pops up again. template inheritance for child themes is a bit sticky anyhow, though; I tend towards thinking that should be solved separately.

AttachmentSizeStatusTest resultOperations
template-suggestions.patch827 bytesIgnoredNoneNone

#3

merlinofchaos - July 7, 2008 - 21:01

IMO the proper solution is that during registration, 'theme paths' for every hook is always updated with the theme being processed, that way suggestions will be looked for in the right place. This can be done in the theme's hook_theme(), much like we have functions to auto-find templates and theme functions. If this doesn't make sense I can try to elaborate.

#4

eaton - July 8, 2008 - 00:20

IMO the proper solution is that during registration, 'theme paths' for every hook is always updated with the theme being processed, that way suggestions will be looked for in the right place. This can be done in the theme's hook_theme(), much like we have functions to auto-find templates and theme functions. If this doesn't make sense I can try to elaborate.

That seems a little bit odd. it would mean that themes with node.tpl.php would work fine, but themes with node-story.tpl.php would have to implement hook_theme(). Or rather, hook_theme_registry_alter(), to add the entry to 'theme paths' for each template they handle.

Since we already scan various directories when 'suggestions' is set, is there a problem with always checking the theme's directory? I'm not saying there isn't -- it just seems like this is a fairly obvious bug, and punting the work of fixing it to themes just makes things trickier. I would rather we remove the ability to automatically find node.tpl.php in a theme's directory than have auto-discovery work for some template and not for others...

#5

mfer - July 8, 2008 - 11:36

@eaton I agree that this seems like a bug. If anything it's an issue that theme system does not do what it used to do (or what people expect), it's not intuitive to figure out, and I have not read an migration information for this topic. This just doesn't seem intuitive and isn't documented. It would be great to have this fixed or documented well before 6.3 goes out.

For theme inheritance shouldn't we check the theme and any parent themes for suggestions?

#6

dvessel - July 31, 2008 - 05:46
Priority:critical» normal

mfer, it is documented (see bottom of page). There's also a note where the suggestions are listed.

Suggestions work on any level as long as that "base" template is there but this problem gets tricky since templates can exist in any sub-directory. It would be difficult to track down any derivatives without that base.

One possibility is to have some sort of naming convention so the base hook is clearly differentiated from the rest of the name. For example, say we have "node.tpl.php" and it doesn't exist in the theme. We could modify the template scanning function so it can determine that "node[story].tpl.php" belongs to a node hook. –If those are legal characters. I mean, whatever makes the most sense.

Every single suggestion doesn't have to be stored in the registry, just the path's that lead to the detected hook. This would keep the flexibility of allowing any template to be organized in any way the themer pleases. Think this would work?

#7

eaton - July 31, 2008 - 15:02

mfer, it is documented (see bottom of page). There's also a note where the suggestions are listed.

Man, I feel dumb now. I tend to think that checking the theme's directory whenever there are suggestions is a good idea -- it's a quick check and it covers the majority of the cases. In the 'olden days' it wasn't that much of a problem, because there just weren't that many items that had suggestions. Now, though, modules are providing lots of potential suggestion candidates and explaining that we need to copy the original template over, DUPLICATE it, and then modify the more-specific duplicate might make it more explicit if we can't bring ourselves to get rid of the blind-spot...

#8

mfer - July 31, 2008 - 17:45

Well, if this isn't going to be a bug fix for drupal 6, it would be a great enhancement for drupal 7! This would be a nice step up in the developer experience realm making drupal more intuitive and easier to theme on.

#9

dvessel - July 31, 2008 - 18:40
Version:6.x-dev» 7.x-dev
Status:needs review» needs work

Eaton, the only problem I can see with defaulting to the base level of the themes directory is exactly what you stated. There can be many templates so IMO, it should be flexible about how it can be managed. Limiting it to that one directory can get out of control and can lead to a pattern of overloading the base directory even for base templates.

What do you guys think of the approach I mentioned. On first glance, it should be relatively simple. We just have to agree on the naming convention. Or if there are any other ideas, lets hear it.

#10

eaton - August 1, 2008 - 01:17

What do you guys think of the approach I mentioned. On first glance, it should be relatively simple. We just have to agree on the naming convention. Or if there are any other ideas, lets hear it.

I'm very very hesitant about that change in the naming convention; between wildcards, suggestions, and other pieces we have so many utterly confusing ways to do things in the theme system that introducing a new file naming convention to work around this bug will only make things worse. I'd rather see this problem remain than introduce a new 'magic' naming scheme on top of all of our other magic.

#11

dvessel - August 1, 2008 - 01:35

Yeah, I see your point but being able to differentiate I think would be helpful too. A naming convention can help spot what is what at a glance. One pattern, one purpose. Having that clarity would be nice but as you stated, it's another thing to learn.

#12

eaton - August 1, 2008 - 01:46

Eaton, the only problem I can see with defaulting to the base level of the themes directory is exactly what you stated. There can be many templates so IMO, it should be flexible about how it can be managed. Limiting it to that one directory can get out of control and can lead to a pattern of overloading the base directory even for base templates

I'm not quite sure what you mean by 'more flexibility' in this context. you mean a naming convention that would make it easier for us to determine what base theme element a given template corresponds to?

#13

dvessel - August 1, 2008 - 15:03

I meant just the placement of the template. Not restricting it to the base level. That's all.

I tend to manage templates into sub-folders now. Even in Drupal 5, I figured out a way and I found it very useful for complex themes.

#14

moshe weitzman - August 23, 2008 - 02:44

So, any resolution here?

#15

dvessel - August 24, 2008 - 01:12

I guess not.. What do you think about defaulting to the base level of the theme? I don't agree with it but I won't argue if everyone else thinks it is acceptable.

#16

moshe weitzman - August 24, 2008 - 03:04

I don't really know the right answer. In general, I have no problem with fie naming conventions. D6 introduced a nice convention for include files, for example. So if we can solve this with some convention like node.story.tpl.php (for example), thats looks pretty clean to me.

#17

sila80 - August 26, 2008 - 08:18

moshe weitzman
+1

-----------------------
Wrinkles treatment - free info!

#18

dvessel - August 29, 2008 - 15:58
Assigned to:Anonymous» dvessel

I'll work on this over the weekend.

#19

Robin Monks - August 30, 2008 - 19:56

Subscribing

#20

dvessel - September 2, 2008 - 15:53

I apologize for the delay. I've been running into a few hiccups. I also discovered a bug where the global $theme_path will not always point to the right place. Due to the current implementation, it rarely encountered.

Specifically, $theme_path is always supposed to point to the location where the theming implementation is located whether it's in a module, theme or sub-theme. $theme_path informs path_to_theme() so it always points to the right place. So, for example, this bug would be exposed if a template suggestion located in a module folder is used while the base template from the theme exists. path_to_theme() would point to the theme and not the module where the template is located.

Very unlikely this would happen in core since there are about 2-3 template suggestions in core but it can crop up in contrib if suggestions are provided.

Now, if I can manage to get this patch up, the bug would be encountered a lot more often. Even with Eatons suggestion's it would be a problem. Lets say the base template is in a module and the themer copied only the suggestion into their theme. path_to_theme() would point to the module and not the theme.

It's impossible to work around unless we rework how paths are determined for a themed implementation.

What I'm trying to do now is feed the theme registry with all the theme paths. There's a key of 'theme paths' for each hook done as templates. As long as the path to the suggestion is there, it will use it. See drupal_discover_template() where the 'theme paths' would be pushed as $paths. I'll keep working on that. It's not as easy as I thought it would be. :/ The chain of functions is complex and I haven't managed to get it in there cleanly.

#21

merlinofchaos - September 2, 2008 - 16:53

Hey dvessel -- using 'theme paths' is what I was suggesting all along, wasn't it?

#22

dvessel - September 2, 2008 - 18:19

Hi Merlin, you definitely suggested that but I thought you meant that it can be left to themers taking care of it. What I'm trying to do now is pushing those paths without it interfering with anything. I'm was trying from drupal_find_theme_templates but getting all the hooks to accept those 'theme paths' screws with what's already there while being processed in _theme_preprocess_registry.

For the global $theme_path, do you have any ideas? Because it's so dependent on the default template, I can't see any way around it.

thanks!

#23

dvessel - September 3, 2008 - 19:33
Status:needs work» needs review

Here's what I have. Suggestions for normal hooks work as well as wild card hooks. When setting up the suggestions from preprocessors, the base hook doesn't have to be added. It's automatically prepended. I thought it was odd to define that "." between the HOOK.SUGGESTION.

I know this won't fly as it is. Theme paths as I mentioned above would have to be reworked to allow the base and suggested template from being in different locations.

It works as long as path_to_theme isn't invoked at the wrong location.

AttachmentSizeStatusTest resultOperations
template_flex_suggest.patch14.92 KBIgnoredNoneNone

#24

dvessel - September 3, 2008 - 19:45

And again..

AttachmentSizeStatusTest resultOperations
template_flex_suggest.b.patch14.85 KBIgnoredNoneNone

#25

arhak - October 8, 2008 - 23:47

subscribing

#26

dvessel - October 9, 2008 - 00:51
Assigned to:dvessel» Anonymous
Status:needs review» postponed

Postponed till we figure out a better way to determine theming paths.

#27

meba - December 9, 2008 - 13:14

Somebody noticed but in order to be more google-searchable, please note that this also affects content-field.tpl.php - content-field-field_name.tpl.php won't work without the former file.

#28

TravisCarden - June 24, 2009 - 20:44

Subscribing.

#29

geerlingguy - August 28, 2009 - 05:46

Subscribe...

And to meba / #27 - this applies to pretty much any kind of .tpl.php - you can't use any children of a main template file without the main one being in your theme folder.

#30

SeanBannister - September 28, 2009 - 11:42

Sub

#31

Chris Charlton - October 13, 2009 - 05:28

Subscribed.

#32

webchick - October 13, 2009 - 05:35
Status:postponed» needs work

Reverting from "postponed" status so other people know it's ok to work on this. This is always a total WTF for themers new and old alike, so I'd love to see this fixed.

#33

mattyoung - October 13, 2009 - 06:53

.

#34

forngren - October 17, 2009 - 08:16

Submarine.

#35

Aren Cambre - November 8, 2009 - 18:32
 
 

Drupal is a registered trademark of Dries Buytaert.