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/
participants (1)
-
Aaron Winborn