Posted by IrishGringo on September 13, 2008 at 4:16pm
| Project: | Image Assist |
| Version: | 6.x-2.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
This file seems to be included in to locations
/modules/image_assist/img_assist.views001.inc
and /modules/image_assist/include/img_assist.views001.inc
I was mucking around with it to see if I could fix it, but I just deactivated it instead.
I only checked it with the Internet Services theme
and I do have a lot of modules running.
Comments
#1
Sorry, I am not able to understand what bug you are experiencing.
#2
well, I have disabled the module and not using it now... but i think that there werre references to the img_assist.views.inc file being in both the...
/site/modules/image_assist dir
and the..
/site/modules/image_assist/include dir
it could be a themeing issue. sorry about including the /modules/image_assist/include/img_assist.views001.inc
I had renamed img_assist.views.inc as img_assist.views001.inc to try to track down the problem
#3
I can confirm this bug exists. I thought that Views would always use the supplied path of hook_views_api() to determine Views include files, but that does not seem to be the case.
A workaround for this bug is to remove img_assist.views*.inc and img_assist_token.inc from the root directory of the module. Only the files in /includes should be used.
Description
Since hook_views_api() is a requirement for contrib modules to integrate with Views 2, modules are also able to specify the path to its Views include files. For Image Assist, we took this chance to move views*.inc files into a new sub-directory, and passing this new path over to Views.
However, users who had 6.x-2.x (development snapshot) installed before, are now faced with:
PHP Fatal error: Cannot redeclare img_assist_views_plugins() (previously declared in \sites\all\modules\img_assist\includes\img_assist.views.inc:15) in \sites\all\modules\img_assist\img_assist.views.inc on line 29I tried to find the code in views.module, resp. includes/cache.inc, that is causing this - without luck.
Marking as critical, because users are faced with an unrecoverable fatal error, resp. WSOD, when this happens.
#4
For the sake of completeness, this is IA's implementation of hook_views_api():
<?phpfunction img_assist_views_api() {
return array(
'api' => 2,
'path' => drupal_get_path('module', 'img_assist') .'/includes',
);
}
?>
#5
Devel's theme registry revealed:
IA's default view img_assist_browser_thumbnail (residing in img_assist/includes/img_assist.views_default.inc) has been registered with:
However, I'm unsure whether this might be caused by IA having
template_preprocess_img_assist_browser_thumbnail()inimg_assist/includes/img_assist.views.inc, but the template file inimg_assist/img-assist-browser-thumbnail.tpl.php(not in /includes).Moving back to IA for now.
#6
That looks, to me, like a Views row style plugin definition, which is created by hook_views_plugins() and registered by Views using information provided to the look. The plugin likely needs to set 'theme file' and 'theme path' properly.
#7
Thanks to merlinofchaos' input on IRC, we also have to move img-assist-browser-thumbnail.tpl.php into /includes, because we need to define
theme pathfor IA's row style plugin and the theme registry requires us to have the template file in the same directory as the file containing the template_preprocess function.Without
theme pathdefined in the row style plugin, Views uses the path of the module defining the plugin as theme path. This lead to the double inclusion of img_assist.views.inc, which has been moved into the includes directory.#8
Note that a second solution could be to have a theme.inc in the same directory as the .tpl.php and put the preprocess in that file. Either one is fine, so whichever way works best for this module is what should be chosen.
#9
Taking into account, that IA implements some other theme functions that could live in template files, having all theme functions and templates in /themes (like Views 2) is probably the best and most future-proof solution.
#10
just a question... I was also using tiniMCE... and there was a time when I had it working a way I really liked, and I am wondering if this is that module that did it. The Image insert button would bring up a popup to insert the image in the html. It had a button right next to the image select line that did another popup of the images on the File system. It was really a nice feature... but I have never been able to make it work since then. would anyone know what I did right or wrong? I suppsoe I should post this in the tiniMCE area as well.
I am running drup6.4 so I will try this module again in a couple of weeks, or when it seems to be resolved.
#11
I agree that putting the template file in includes/ is awkward, so the solution with theme.inc is probably the way to go. We could move more theming functions to this file in a followup patch. (We'd just add 'file' elements in hook_theme.) And maybe move some of them to .tpl.php files ('template' and 'path' elements in hook_theme).
* Should theme.inc be called img_assist_theme.inc? Is there a convention for this?
* Also, is the directory name "theme" an established standard? I seem to remember seeing something about a "templates" directory.
More comments on the patch:
* In the changed doxygen comment for img_assist_views_plugins(), I think we should use "Define" rather than "Defines". I think that's the standard grammar for these things.
* I was getting all sorts of PHP warnings before I realized I wasn't using the current version of Views on my test site. Maybe we need to add a notice on the project page about Views versions?
So, looks good and seems to work! RTBC after you've looked at the questions above. Nice work!
#12
I have double-checked this now; both Views and CCK have a /theme folder, both use a theme.inc, so we can follow them.
Fixed the PHPdoc issue and committed to 2.x.
I'm reluctant to document that users should use the latest release of Views, because Views 2 is still a RC, so they should do this anyway or they will get all sorts of errors. So let's see whether we have to... ;)
#13
Automatically closed -- issue fixed for two weeks with no activity.