path_to_theme() is returning the path to the module that called it rather than the path to the theme.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Gábor Hojtsy’s picture

Status: Active » Postponed (maintainer needs more info)

Oh, trying to understand what do you mean. path_to_theme() returns the global $theme_path if it exists or sets it up with init_theme(). Actually init_theme() calls _init_theme() with the $theme object, and _init_theme() sets up the $theme_path basically by doing $theme_path = dirname($theme->filename);.

This seems to be right. If anything goes wrong, it probably goes long later. Please provide more information.

theborg’s picture

Status: Postponed (maintainer needs more info) » Active

Tested and is returning the correct path 'sites/all/themes(custom_theme_name)' or 'themes/(theme_name)'.

theborg’s picture

Status: Active » Postponed (maintainer needs more info)
mfer’s picture

Status: Postponed (maintainer needs more info) » Active

I did a little digging and I'm finding I'm getting inconsistent results from path_to_theme(). When I call it in my template.php file in a function like phptemplate_preprocess_page it returns the path to the theme.

But, if I turn on the php filter, go to node/add/page, create a node with the php filter and make the content print path_to_theme() and click preview it displays modules/node.

I've found this on two separate installs.

merlinofchaos’s picture

Status: Active » Postponed (maintainer needs more info)

1) pretty much fresh install of HEAD, with devel generate for random content
2) edit a node
3) set content to <?php print path_to_theme(); ?>
4) set input type to PHP code
5) save
6) Resulting output:

The Page has been updated.
themes/garland

The theme code will reset path_to_theme() such that if your module provides a theme function, while inside that theme function, *your module* is considered the theme. This is the intended behavior; that way, your module can reference its files. This is because, while inside a theme function, your module has no knowledge of what's in the theme directory, and cannot rely on these files; on the other hand, your module may need to reference its own theme files, and this way it can. This is also very, very important for inherited themes. If I have theme C, which inherits from theme B, which inherits from theme A -- and my site has its theme set to theme C -- and while running a theme function/template from theme A is invoked, as far as theme A is concerned, it *is* the theme that's running so it can reference its files without error. The way this interaction works, a module's theme function/template is just another child in the chain, with no real knowledge of the children.

I have no idea how it is you have a PHP filter which produces modules/node as the result for path_to_theme() but it's going to require specifics to be able to reproduce.

merlinofchaos’s picture

Ok, I realize after being told on IRC that I misread and this happens on preview. Since preview happens within a theme function, node becomes the theme.

I'm not sure there's an easy way to fix this. We could try to 'fix' the theme path check so that it doesn't happen when a module owns the theme, but that doesn't seem right to me, since modules may have subsidiary files as well, and if we did that, those theme functions/templates would have to address the module specifically which means that those subsidiary files are now permanently in the module and can't be overridden.

mfer’s picture

Here's another example from something I tried to put in use. I created a block with a filter of php. Part of the content is print base_path() . path_to_theme();

When I save this block and go view the content the path says '/modules/system'.

In drupal 5 this would have been '/sites/all/themes/mythemename'.

After reading what you said I think I see how this could happen with the module being the theme. Previously if I need to get to something in the module I'd use drupal_get_path. If path_to_theme is going to reference the module rather than the top level theme how do we reference the top level theme?

And, if this is the case the module and theme documentation needs to be updated to reflect this.

Gábor Hojtsy’s picture

Priority: Critical » Normal

This sounds like a pretty hideous edge case, not something which should hold back the D6 release, right?

merlinofchaos’s picture

It is unfortunately not exactly an edge case, as it can happen in a number of different places.

Basically, it affects any code run through drupal_eval() -- in this case, you've got user-supplied PHP code which really is expecting the theme to be where it's supposed to be; but often, drupal_eval is called within a theme() function (mostly during previews).

It's...probably nota critical bug. It is, however, going to cause serious confusion during node and other previews.

theborg’s picture

It would be possible to control node_preview or theme_node_preview on theme.inc ?

@@ -580,8 +580,10 @@ function theme() {
   $info = $hooks[$hook];
   global $theme_path;
   $temp = $theme_path;
-  // point path_to_theme() to the currently used theme path:
-  $theme_path = $hooks[$hook]['theme path'];
+  // point path_to_theme() to the currently used theme path when not in preview mode:
+  if ($hook != 'node_preview') {
+    $theme_path = $hooks[$hook]['theme path'];
+  }
 
   // Include a file if the theme function or preprocess function is held elsewhere.
   if (!empty($info['file'])) {

mfer’s picture

This isn't just during previews. If you save a node or block and go to view it you will get the path to the module when you go to view it.

theborg’s picture

In the case of blocks:

  if ($hook != 'node_preview' && $hook != 'blocks') {
    $theme_path = $hooks[$hook]['theme path'];

Or something more radical like:

  if ($info[type] != 'module') {
    $theme_path = $hooks[$hook]['theme path'];
  }

But as for melinofchaos comentary #6: modules may have subsidiary files so maybe neither solution is correct.

mfer’s picture

Should we have 2 functions. One to call the theme of the module/that level and one to call the top level theme? I like this flexability and i'd like to keep it while still making this easy to use.

merlinofchaos’s picture

Hardcoding a check for 'node_preview' would be wrong. The check for 'module' is inappropriate.

I think we need to put something in drupal_eval() to make a theme switch; that's where the real issue lies.

mfer’s picture

What I think you are saying is to do something like...

function drupal_eval($code) {
  //change global $theme_path to top level theme value
  ob_start();
  print eval('?&#62;'. $code);
  $output = ob_get_contents();
  ob_end_clean();
  //change global $theme_path back to what it was before the function was called
  return $output;
}

This would effect both the php filter and when php is used for block visability.

I'd roll a patch but I have no idea what the right way is to do this change. I would probably do something like set $theme_path = dirname($base_theme_info[some key]->filename) and revert it back right before returning any value.

Is there a better approach?

merlinofchaos’s picture

Status: Postponed (maintainer needs more info) » Active

mfer: Yes, that's what I'm thinking, and that's the best I can think of. Anybody got better ideas?

mfer’s picture

Assigned: Unassigned » mfer
Status: Active » Needs review
FileSize
688 bytes

Here's a patch that does just that.

dvessel’s picture

Status: Needs review » Needs work

Unfortunately, the theme system isn't always initialized by the time drupal_eval is run. Works fine on previews but viewing a saved node returns a notice since $theme_info is NULL.

theborg’s picture

Status: Needs work » Needs review
FileSize
977 bytes

A possible workaround would be to use global default theme variable.
This patch checks if theme_info is not set and then get the path from the default theme.

merlinofchaos’s picture

That looks like a reasonable solution. dvessel, do you agree?

mfer’s picture

@theborg - That's what I was thinking as well.

dvessel’s picture

Status: Needs review » Reviewed & tested by the community

It looks fine to me.

Would be nice if we had another function to point to the actual theme instead of the themed element. Next version maybe?

Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Needs work

Looks generally good, but for better understandability:

- I'd global $conf just as well as the others.
- I'd name $temp $old_theme_path instead, so we know what it is.

theborg’s picture

FileSize
993 bytes

$conf is global now and $temp renamed to $old_theme_path.
Thanks Gábor for your suggestions.

theborg’s picture

Status: Needs work » Reviewed & tested by the community
Gábor Hojtsy’s picture

Status: Reviewed & tested by the community » Fixed

OK, added this comment and committed with that:

  // Restore theme_path to the theme, as long as drupal_eval() executes,
  // so code evaluted will not see the caller module as the current theme.
Anonymous’s picture

Status: Fixed » Closed (fixed)

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

aaron’s picture

Version: 6.x-dev » 6.4
Assigned: mfer » Unassigned
Status: Closed (fixed) » Active

If I create a custom block with a module, and in the content include path_to_theme(), the function still returns that module's path. I know this is an edge case, but want to make sure we're marking it Won't Fix, or planning to find a better solution.

mfer’s picture

Status: Active » Closed (fixed)

Can you please open up a new issue for this with details of your version, situation, theme, sub-theme (if any). This issue was for a different, though similar, case. While this case was fixed I imagine, though I hope not, your case may be a won't fix type of issue in D6.

I'm, also, curious why you would call path_to_theme() in a custom block from a module?

greg.harvey’s picture

Hi,

Same problem here - the reason we are calling path_to_theme in our block is we have one module powering content for several sites, all using different themes in a multi-site installation. We have four sites, with four subtly different themes but we don't want four modules. We just want to pass the current theme path to the module so it loads graphics from the right theme when building the block content.

Where's the new issue? aaron, could you link to it from here?

greg.harvey’s picture

Ps - this is *NOT* occurring inside a theme function, as merlinofchaos describes. It's in a normal function (which probably should be a theme function, but that's by-the-by) called by hook_block(). The function is not registered with hook_theme().

But thinking about it, I'm going to move the images to the module so drupal_get_path() will work (couldn't use drupal_get_path('theme', 'my_theme') because the themes are not available to every site, so the return is null).

Damien Tournoud’s picture

It doesn't make *any sense* to call path_to_theme() outside a theme function (after all, you know *nothing* about the files that should or should not be there) .

Your block logic should call a theme function that itself calls path_to_theme() if required.

greg.harvey’s picture

I don't think you're being totally fair to us there. It's perfectly logical and reasonable to assume path_to_theme() is globally available, like base_path() and other such functions of this ilk, since I know *exactly* what files should be in *my* theme and I want to call them.

But anyway, I see and totally understand the logic here. So path_to_theme() should only ever be called inside a theme function in a module context. That makes sense. Thanks. =)

Damien Tournoud’s picture

@greg.harvey: sorry if I sounded unfair. I know you do know what files are in your theme and the way you want to call them, but your module should not. Your module should be theme-agnostic. So this specific path_to_theme() should really be in your theme's template.php.

greg.harvey’s picture

Sure, that does make sense now I think about it and you explained it well. It might be worth adding a line or two to the documentation explaining the use of path_to_theme() to avoid such misunderstandings - at the moment the explanation is very spartan:
http://api.drupal.org/api/function/path_to_theme

Damien Tournoud’s picture

@greg.harvey: fair enough. Would you be kind enough to open a new issue for this (against 7.x and the documentation component)?

greg.harvey’s picture

merlinofchaos’s picture

Unfortunately, blocks are often built inside a theme('regions') call, so you actually are inside the theme system at that point. :/

You can get the real theme path by doing a global $theme to get the theme key, and retrieving theme info based on that. IT should be cached at this point so it won't cost much resource-wise, it's just a little annoying to have to do.

(And I disagree that there are no valid uses for getting the theme path outside a theme function. They are mostly edge cases, though).

dvessel’s picture

merlinofchaos, it'd be nice to have two distinct functions. One to the themed element and the other to the actual theme.

BTW, another option to get the theme path independent of the themed element by providing something like this in your module:

/**
 * Use this instead of path_to_theme() within preprocess functions since this
 * theme does not own any templates which is a requirement in order for
 * path_to_theme() to be able to point to the current theme directory.
 *
 * @param $name
 *  (optional) The name of the theme. Defaults to the active theme if nothing is passed.
 */
function theme_path($name = '') {
  global $theme;
  static $path = array();

  if ($name && !isset($path[$name])) {
    $path[$name] = drupal_get_path('theme', $name);
  }
  elseif (!isset($path[$theme])) {
    $path[$theme] = drupal_get_path('theme', $theme);
  }

  return $name && isset($path[$name]) ? $path[$name] : $path[$theme];
}

I use the above in a theme without templates which has the same limitations. What merlinofchaos recommended is slightly better since it doesn't need to call any functions. The data is all inside the global $theme_info.

jmlavarenne’s picture

I'm finding using global $theme is not practical on a multisite install when base_path() returns / . My module is in a specific site's module folder, and I'm not finding my way to the theme folder easily.

Also $theme_info does not actually include the path to the theme, but to the theme .info file.

Your function solves this issue for me.

Nick Robillard’s picture

I always solve this with the following. Keeps it contained to one line.

drupal_get_path('theme', $GLOBALS['theme_key'])