Parent theme pattern clobbering

Will dig into this soon...

The patch works perfectly. Tested up to 3 level of subthemes. Tweaked the docs and fixes offset..

Please get this into 7 first.

What's the link with #279573: Themes can't use node-story.tpl.php without node.tpl.php? Are those duplicate?

Putting as code needs work, as this will need a test case before getting in 7.

Damien, the template suggestions and this issue is unrelated. That issue depends on theming paths which depends on the base template.

FYI I helped neclimdul with this original patch, so if it matters I'm very much in favor of it.

In order to describe what happens, we'll still use Views as an example.

We have the theme 'views_view', and through the pattern system, 'views_view__foo' is called. During ordinary operations, views-view--foo.tpl.php will be found and utilized.

However, there is an obscure situation in which a base theme having 'views-view.tpl.php' will register that template and inadvertently destroy the 'pattern'. When the child theme goes to register 'views_view' as a theme, it won't see the pattern, and this it won't search for 'views-view--foo.tpl.php' through the normal mechanism. Thus the theme file is not found.

This patch corrects it; it should be applied to both 7.x and 6.x.

dvessel: I believe that there needs to be two of these. One in drupal_find_theme_templates and one in its sister function to find theme functions. THey should both do essentially the same thing.

merlinofchaos, I thought it needed the fix there too and I did test it. I really tried breaking it but works without any patching.

Do we have any theme functions that work the samew ay though? Having theme_views_view() in a base theme and theme_views_view__foo() in a child theme? I think it should run into the same limitation.

Sounds like this still needs some review.

(also, ninja subscribe ;))

I thought I tested it throughly but apparently it can slip through when naming the function after the theme engine. Bah!!

And here's a patch. I also modified the comments. Makes more sense I think.

Ok, that's more like what I thought. Does the $info['pattern'] in the first part of the patch generate notices? (I'm not sure if it's possible for $info['pattern'] to not be set at that point. If it's registering a new hook that it found from a pattern, $info['pattern'] is always set but I don't think the newly created pattern should actually inherit; that could cause multiple registrations of the same pattern?

Definitely no notices since that first part is within a "if (!empty($info['pattern'])) {".

http://api.drupal.org/api/function/drupal_find_theme_functions/6

I placed that there since a function named after the pattern could be setup in a base theme in addition to a sub-theme. Without it, only the base theme would catch onto the pattern while the sub-theme would not.

Updated patch fixes a stray word in the comments.

Merlinofchaos, oh.. I misunderstood. Never mind what I just said. I think your right about the first part and it should be removed.

Merlinofchaos, I looked through the registry and that first $info['pattern'] did cause newly registered patterned theming hooks to have the same pattern attached. It didn't look harmful since a pattern of "foo__" would register "foo__bar" but "foo__bar" I don't think can further register more patterns on top of that. But I guess it's better to be safe and just remove it.

Everything else looks good to me.

This isn't directly related but I made a few notes to help me grok how patterns fit into the way overrides work.

sub-theme > base theme

hook pattern > base hook
foo--bar.tpl.php vs. foo.tpl.php

function > template
themeName_foo() vs. foo.tpl.php

-----

sub-theme + base hook < base theme + hook pattern

sub-theme + base hook + function < base theme + pattern hook + template

-----

So, patterns have the most weight followed by sub-themes and then theme functions as opposed to templates for a given hook.

Ironically I think we've come full circle back to my original patch that I gave to neclimdul.

Tested on 7. Confirmed to work.

From a discussion with merlin, I created a module with:

Then create 2 themes with theme functions parent_foo() { print "parent"; } and subtheme_foo__a() { print "subtheme"; }

Before the patch, it prints "parent" and after if prints "subtheme" as is the expected behavior. But, it didn't work for me with templates. Attached is the themes and modules I used to test it in case I made a mistake.

neclimdul, it's because the template must use hyphens. Your test files were using underscores.

dope. lets do this thing then!

http://testing.drupal.org/pifr/file/1/parent_theme_pattern_clobbering-e-... gives the patch in #18 a clean pass. We don't (yet) have a way to test theme logic in SimpleTest, but neclimdul's .zip in #20 looks like a really awesome start for when we do.

Committed to HEAD. Thanks! Marking back to 6.x.

SimpleTest gurus, see #343898: Let SimpleTest test the theme system.

Committed to 6.x, thanks.

Committed to 6.x, thanks.

Automatically closed -- issue fixed for two weeks with no activity.

is this fixed in core now? 6.9?