No matter that the current default theme and admin theme, admin/structure/block always shows me the settings for Garland.

CommentFileSizeAuthor
#6 698406_menu_rebuild.patch708 bytesmarcvangend

Comments

doka’s picture

doka commented

I confirm this bug.

Expected behaviour is /admin/structure/block should go to the block settings of default theme. Instead, it will open the blocks for the first theme (in alphabetic order) from all enabled ones.

Add this line

$theme = variable_get('theme_default', $theme_key);

instead of these lines

if (!isset($theme)) {
    // If theme is not specifically set, rehash for the current theme.
    $theme = $theme_key;
}

in line 29 of drupal-7.x-dev/modules/block/block.admin.inc

joachim’s picture

joachim commented

That won't work. We have to come to this function for either:

- the current theme
- whichever theme tab the user clicked.

I'm not sure how to fix this.

First problem: global $theme_key; is not yet set at this point.]
Second problem: if I go to 'admin/structure/block' -- ie should be current theme -- then the parameter $theme is *already* set to the wrong theme.

I think the bug is higher up, in the menu system.

joachim’s picture

joachim commented

It doens't go to the first in alphabetical order, but the theme that was enabled when the menu was cached.

As far as I know, changing a theme does not clear the menu cache, so here is the problem:

function block_menu() {
  $default_theme = variable_get('theme_default', 'garland'); // this is the default theme NOW.... but what about later?
  $items['admin/structure/block'] = array(
    'title' => 'Blocks',
    'description' => 'Configure what block content appears in your site\'s sidebars and other regions.',
    'page callback' => 'block_admin_display',
    'page arguments' => array($default_theme),
    'access arguments' => array('administer blocks'),
    'file' => 'block.admin.inc',
  );
joachim’s picture

joachim commented
Title: admin/structure/block goes to the wrong theme by default (regression) » block_menu() broken: depends on theme at time of menu cache rebuild

...which means that swathes of block_menu() are broken, as lots of items use $default_theme.

To reproduce:

1. Set your theme to garland.
2. Go to admin/structure/block -- you see Garland options.
3. Change your theme.
4. Go to admin/structure/block -- you still see Garland options.
5. Clear the menu cache.
6. Go to admin/structure/block -- you now see the correct theme.

marcvangend’s picture

marcvangend
he/him
Primary language Dutch
Location Amsterdam
commented

Marked #717712: default blocks listing set to Garland theme even though the default is another theme as a duplicate of this one.
This bug was introduced in #581118: Blocks admin user interface should not do theme switching.

I can think of two ways to fix this.

1) Rebuild the menu when the default theme is changed.
pro: it's a simple one-line change in system.admin.inc
con: I don't like fixing a problem in system module, when it is caused in block module

2) Remove the variable_get('theme_default', 'garland'); from the hook_menu and process the default_theme in block_admin_display().
pro: fixing a block module problem in block module
con: more complicated patch, maybe not even possible because of the way local tasks work

My vote goes to method 1, but I'm open to other options.

marcvangend’s picture

marcvangend
he/him
Primary language Dutch
Location Amsterdam
commented
Status: Active » Needs review
StatusFileSize
new 698406_menu_rebuild.patch708 bytes

...and here is the patch, following method 1 above.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Closed (duplicate)

this is a duplicate if #690544: Blocks admin (admin/structure/block) requires menu_rebuild() to display settings for current theme
Same patch