While trying to fix another issue I noticed that on a default install, Garland was adding the following:
<!--[if lt IE 7]>
<link type="text/css" rel="stylesheet" media="all" href="http://127.0.0.1/themepath/modules/system/fix-ie.css" />
<![endif]-->
where this should obviously be
<!--[if lt IE 7]>
<link type="text/css" rel="stylesheet" media="all" href="http://127.0.0.1/themepath/themes/garland/fix-ie.css" />
<![endif]-->
after some pointers by DamZ on IRC I started digging for a good 4 hours and I think I've pinpointed where the issue manifests:
_theme_process_registry sets the theme path right at the top but then doesn't adjust this when a theme's preprocess or process was found (or at least that's what I think went wrong)
attached patch changes the path when a preprocess or process function was found in the theme, but I'm entirely unsure if this is the best way to do this, as I'm horribly unfamiliar with the whole registry shabang
it fixes the problem that originally caught my eye, but I have no clue what other implications it might have
| Comment | File | Size | Author |
|---|---|---|---|
| theme_path.patch | 556 bytes | seutje |
Comments
Comment #1
pbz1912 commentedWorks for me.
I'm fairly new, so not sure how to post stuff like this.
Comment #2
pbz1912 commentedFile changed but patch not applied in latest build. And bug still there.
Means IE style sheets still not working.
Comment #3
pbz1912 commentedComment #5
ksenzeeSubscribing. I'll test this out and see if it breaks anything. It's quite a WTF to be debugging and have your path be "modules/system/whatever" instead of "sites/all/themes/mytheme/whatever".
Comment #6
aspilicious commentedThis need to be committed first, before we can fix #676800: Fieldsets break design badly, cause the ie6.css in the seven theme is useless now...
I put this to RTBC because we tested it with a couple of persons while trying to fix the other issue and it worked!
And its one single line, doesn't need more review.
Or does it?
Comment #7
aspilicious commentedforgot to change the status
Comment #8
webchickOk, committed to HEAD.
Comment #10
effulgentsia commentedCan we revert this? I believe it was the wrong solution. The correct solution is in #1125220: Subthemes of core themes trigger an undesired 404 and fail to inherit their parent theme's IE styling. path_to_theme() should either point to the active theme (when called from outside the context of a theme() invocation), or else the module or theme that implements the invoked hook (when called from inside the context of a theme() invocation). In the case of the 'html' theme hook for which this issue originally started, that is the location of html.tpl.php, which was correctly 'modules/system' prior to this issue. Just because a theme implements a preprocess or process function doesn't make it the implementor of the theme hook.
Anyway, #1125220: Subthemes of core themes trigger an undesired 404 and fail to inherit their parent theme's IE styling needs to get in first. But, anyone have objections to reverting this? And if not, should we roll it into the other issue, or keep it a separate task?
Comment #11
effulgentsia commentedI need to get into the habit of setting tags before clicking "save" :)
Comment #12
dvessel commentedWhat effulgentsia said. This patch is not right. 'theme path' should be pointing to where the theme function or template was implemented, not a preprocess function.
Variable processors can be implemented on many levels, in many places but theme functions and templates effectively exists once per hook. Instead of giving themes a special case here, I think it was better to keep the theme paths consistent here.
Comment #13
lauriiiI believe this problem doesn't exist anymore in Drupal 8. Correct me if I'm wrong.
Comment #18
amit0212 commentedYou don't need to implement hook_theme() if you just want to override an existing theme function.
I've just tried this code in my template.php file:
function MYTHEME_item_list(&$variables) {
// my custom stuff done to theme_item_list
return 'test';
}
And all calls to theme('item_list') now return the string 'test', and nothing else.
You'll need to clear Drupal's caches once you've implemented the function, that might also be an issue.