[development] Preprocess flaw

Aaron Winborn winborn at advomatic.com
Sat Aug 23 13:50:07 UTC 2008


I raised an issue at http://drupal.org/node/297951 originally for the 
Filefield module, and then at http://drupal.org/node/297952 for the 
Drupal core theme preprocess hook architecture. The second issue was 
marked 'by design', but I believe the design is flawed, and that this 
needs to be addressed.

As the preprocess theme hook has only been opened for developers in 
Drupal 6, this wasn't much of an issue before, and the flaw has only now 
become apparent. Without any documentation I can find, the $file 
variable is reserved, meaning that theme developers may not expose a 
variable named $file to a template file when inserting a preprocess 
hook, as outlined at http://drupal.org/node/223430.

The $file variable is well established in Drupal, and often used in 
contributed module theme functions. Now that it's relatively easy for 
themers to override theme files with their own template files, this 
issue will begin to crop up.

The current work-around is that if the theme function you wish to 
override uses a $file variable, then you have to copy that variable in 
your preprocess hook to another variable, such as $original_file. This 
is unwieldy, unfortunate, and currently undocumented (as far as I can tell).

I have a feeling we'll see this problem become more prevalent as 
developers move to Drupal 6. In core, $file appears only 22 times, and 
not in any core theme functions (that I'm aware of). However, that is 
misleading, as core Drupal currently does a poor to middling job of file 
handling (which will hopefully be remedied soon).

In the contributed DRUPAL-6--1/modules branch, $file appears 265 times, 
and 1186 times in the contributions/modules branch. There are several 
contributed modules using $file in theme functions, such as the 
aforementioned Filefield, Imagefield, Media Mover, and File Framework. 
(I'm sure there are others, those are just from a quick grep). And I 
suspect that at the very least, Imagefield and Filefield will warrant 
relatively frequent theme overrides for individual sites.

Assuming hook_file (http://drupal.org/node/142995) makes it into Drupal 
7, I suspect we'll see even more use of the $file variable in themes, 
particularly as more developers jump into file asset management.

Here are some options available to us. There are probably other options 
as well:
1) Do nothing. Let themers struggle when they try to override a theme 
$file variable using a preprocess hook.
2) Document the flaw, so at least developers know the difficulties 
involved in doing this.
3) Declare $file a restricted variable, forbidden in theme functions.
4) Encourage developers to find all instances of $file in contributed 
module themes and rename them.
5) Use another magic variable name for use in theme_render_template, 
such as $file__ or $template_file, and declare it restricted.
6) ???

I believe we are doing a disservice to developers if we tell them to use 
a flawed preprocess system. I strongly suggest we implement option 5 
before this problem becomes more severe, ideally for Drupal 6, as many 
developers are migrating there as we speak.

Thanks,
Aaron Winborn

-- 
Aaron Winborn

Advomatic, LLC
http://advomatic.com/

Drupal Multimedia available in September!
http://www.packtpub.com/create-multimedia-website-with-drupal/book

My blog:
http://aaronwinborn.com/



More information about the development mailing list