Events Module breaks custom themes defined by a module (sections module)

This is because the event.module calls the 'theme' function in its 'event_menu' hook (via the 'event_html_head' function)

That call to 'theme' (in order to simply load a css...) also sets the current theme, before the module sections has the opportunity to set it : it's done in 'sections_menu', that is called after 'event_menu' (alphabetic order ?)

Maybe it would be better to set the '$custom_theme' in a 'sections_init' hook instead ?
Works for me anyway...

Sorry but i am not equipped to submit a patch - here's my modification
Maybe someone more savvy could validate and make the patch ?

comment out lines 185 186 187
// else if ($section = sections_in_section()) {
// $custom_theme = $section->template;
// }

add the following lines :

function sections_init() {
global $custom_theme;
if ($section = sections_in_section()) {
$custom_theme = $section->template;
}
}

doing this in _init creates a severe performance penalty.

I would say that event is the one with the bug here, since it breaks a drupal feature: setting a custom theme.

Bèr

You mean it will be executed even if the page is in cache ? Right.

But i still don't think that the _menu hook is the right place to do this : it's late...
The events module is imho not 'guilty' - it should be entitled to call 'theme' in its _menu hook...

BTW, the taxonomy_theme, which has a quite similar role as the sections module, has just been modified so that it sets the $custom_theme in it's _init hook. And they call 'theme_init' just afterwards, so that the theme is set once and for all...

I've got the same problem with Events clashing with Organic Groups. The groups each are supposed to have their own theme but Events is hammering it.

It seems like Events needs to be modified since it is apparently clashing with mulitple packages. Anyone have any good ideas on how to do this? Please keep in mind I know plenty of regular HTML but very little PHP.

I asked about this weeks ago in Events (since I thought then that it was that module causing the trouble) but no one ever responded.

Thanks
Steve

section used to use the _init hook. But as said before, the performance penalty is huge. I'd rather not see us move back. But you are right, any module should be able to call a theme function anywhere, even in _init.

So that makes this whol issue boil down to this ever recurring issue of modules being loaded on alphabetical order.

Hm, so is there no way without dealing with _init, to force Event to subject itself to the theme settings of other modules? Instead of, as it is now, forcing other modules to obey *its* settings.

Steve

I'll argue that the offender here is event.module. It uses theme('stylesheet_import') instead of theme_stylesheet_import(). Futhermore, it doesn't add the stylesheet the right way - theme_add_style(). Admittedly most modules, including some of mine, currently don't use this.

We don't need to run this call through theme() because it will never be overridden. This function emits no presentation HTML.

Even if the theme is initalized, it looks to me like a later module can still change the theme by setting global $custom_theme can then calling init_theme(). I haven't tried that though.

Ber - I appreciate your attention to performance but you are spreading FUD. There are legitimate uses for hook_init(). It is only a severe performance drain if the things that are done there are time consuming. Evaluating a visibility pref and setting global $theme is not time consuming.

@moshe. It was not me who came up with this. As sid, we had it before, untill someone proved to me that this really was a big perfomance issue. So please, if you wish it in _init, fine, ill put it back there.

But i'd love to see a benchmark with that.

All right then so... philosophical and performance issues aside... is there some practical way to easily edit a few lines and get event to stop clobbering the modules that follow on after it in sequence (or whatever is happening)? Is there a simple edit I can do to make it call the right functions or something?

Thanks,
Steve

Sure there is a simple modification to event.module that solves it all - for now...

line 20, in function event_html_head()
(my post is rejected if I put this code here... see attached text file...)
Once again :
- I can't submit a patch
- I'm don't know if this is very "Drupal orthodox". Maybe more experienced users could confirm that ?

As for the _init hook : I would'nt like to look self-contradicting, but I can undestand Ber's point of view : it's somewhat absurd and not really "clean" to extract a page's current theme when the entire html will be taken from the cache anyway... There should be another way around ?

Thanks! I will try that modification and see how it goes.

Steve

Yes, that one-line fix did just the trick. Thank'ee.

Steve

Care to make a patch for this?
http://drupal.org/diffandpatch

I am attaching a patch for cvs version of event.module, with the one-line modification which was posted above. I guess it has already been reviewed, but I haven't posted a patch in a while...

event_html_head function is removed, theme_add_style is used.