The title says it. Whenever I enable gallery.module blocks configuration stops working correctly!
Generally the theme switches when you clicked on a theme in admin/blocks, you can then configure the blocks for that theme and you get a preview of your blocks setup. But with gallery module enabled the theme doesnt switch and therefore prevents the users from configuring block setup.
I'm not 100% sure about that, but maybe the error can be found in gallery_base.inc (line 110+) in function gallery_css_include(). A call to theme('stylesheet_import', ...) is used to generate the import statement for css stylesheet. But this somehow load default theme (could be the additional call to init_theme()!?). Try to comment this line(s) out and block configuration works fine again.

Comments

kiz_0987’s picture

kiz_0987 CreditAttribution: kiz_0987 commented

What version of Drupal are you using? 4.7RC3? Latest CVS (date?)? I ask because there were a range of theme/block related issues in recent versions of 4.7, the patches for some of which ended up being reverted.

profix898’s picture

profix898 CreditAttribution: profix898 commented

Yes, Drupal 4.7 RC3! And latest CVS (from today, April 22), revision of gallery_base.inc is 1.2.

profix898’s picture

profix898 CreditAttribution: profix898 commented

Its definitly the call to theme(...). Replace it with e.g. $output = '<link href="'.base_path().drupal_get_path('module', 'gallery').'/'.$css_file.'" rel="stylesheet" type="text/css" />'; and blocks configurations works. I will take a look at Drupal core to find out behaviour of theme('stylesheet_import', ...) changed since RC3!

profix898’s picture

profix898 CreditAttribution: profix898 commented

From what I see no changes have been made to core since RC3 to solve this problem! But I found core using theme_add_style() to include stylesheets. "Add a theme stylesheet to be included later. This is handled separately from drupal_set_html_head() to enforce the correct CSS cascading order." (from api-doc). So the calls to theme('stylesheet_import', ...) should be replaced by theme_add_style(...). At the time the theme is loaded (through init_theme) there is the theme_get_styles() function (theme.inc) called from theme engine, that uses theme('stylesheet_import', ...) to generate the actual import statements for all stylesheets.
Replace every call to theme('stylesheet_import', ...) with theme_add_style(base_path() . drupal_get_path('module', 'gallery') . '/' . $css_file, 'screen');.

kiz_0987’s picture

kiz_0987 CreditAttribution: kiz_0987 commented
Assigned: Unassigned » kiz_0987

profix898 - thanks for the detailed info. I will take a closer look.

I am uncomfortable with the init_theme that I've used and may take it out. This will lose the functionality of having the css included from the theme dir (as well as the gallery.module dir) -- this functionality is not included in any other module that I know of, so it should be OK. In my view Drupal needs a better way to include css overrides of module styles than to just modify the css file that comes with the module -- but that is a whole other topic.

profix898’s picture

profix898 CreditAttribution: profix898 commented

You shouldnt rely on what I wrote! It was just a quick view through the code! ;)
From what I understand you would 'only' loose the functionality to include css from theme dir, but using theme_add_style() you can still include css from module dir (regardless of the current theme being initialized)!? And I agree that calling init_theme from this function is probably not the best idea! I like the concept of different gallery css's for every drupal theme (to correct gallery appearence and make it look more alike Drupal design). Actually I think Drupal should introduce a new hook (that is called close to/from the theme engine), e.g. hook_css, gathering css files from all modules implementing this hook.

kiz_0987’s picture

kiz_0987 CreditAttribution: kiz_0987 commented

See http://drupal.org/node/52508#comment-79259 . Apparently theme_add_style should never be called by modules. The original method should work. I would still like to remove the init_theme code. Can you please try the following (untested) modified gallery_css_include function:

function gallery_css_include($css_file = 'drupal_g2.css') {
  $output = theme('stylesheet_import', base_path() . drupal_get_path('module', 'gallery') .
                  '/' . $css_file) ."\n";
  return $output;
}

Briefly looking at the Drupal docs suggests that a uniqueness test is done, so maybe gallery_set_html_head is no longer needed. I'll check into it some more.

profix898’s picture

profix898 CreditAttribution: profix898 commented

Hmm! That's confusing! Seems like there are no official docs on that, right? The modified code doesnt work for me! Same problem as before (tested with Drupal 4.7RC3 & CVS)! Its still the call to theme('stylesheet_import', ...) making trouble.

kiz_0987’s picture

kiz_0987 CreditAttribution: kiz_0987 commented
Status: Active » Fixed

I asked in the general forums about this, see http://drupal.org/node/60096 . I can see now why the original (recommended!) method fails, but it's not clear the best way to proceed. In the meantime this is fixed by using theme_add_style.

Fixed in CVS, not yet rolled into 4.7.0 version.

profix898’s picture

profix898 CreditAttribution: profix898 commented

Yes, I read the thread! It didnt reveal anything new! From what I understand the 'recommended' method is still recommended, but (waiting for a better solution) not in hook_menu and hook_init. We should use theme_add_style() then and hope this is getting discussed any further in the future. Really unsatisfying! I was hoping for a much cleaner solution, a code change and vote from the core people. I really dont understand why we should use theme_add_style() for certain situations and theme(...) for others. From my point of view theme(...) is used internally to generate an include statement for stylesheets which are added by theme_add_style, so why not remove this function (or mark it for internal use _function) and favour theme_add_style in every case and in all contrib modules.

However Thanks for fixing this bug.

Heine’s picture

Heine CreditAttribution: Heine commented

theme() is actually a dispatch function that looks whether the current theme implements a function. If not it uses the default.

This means that it's possible to override the stylesheet import when using theme(); some theme authors used this to prevent drupal.css from loading.

Using theme_add_style no longer allows themes to override loading of stylesheets by modules, but it prevents premature initialization of the theme. I agree whole-heartedly that it's ugly.

Anonymous’s picture

(not verified) CreditAttribution: commented
Status: Fixed » Closed (fixed)