[drupal-devel] [bug] Block regions not initiated for newly enabled theme, so theme unusable

[nedjo]

Issue status update for 
http://drupal.org/node/29506
Post a follow up: 
http://drupal.org/project/comments/add/29506

 Project:      Drupal
 Version:      cvs
 Component:    system.module
 Category:     bug reports
 Priority:     critical
 Assigned to:  nedjo
 Reported by:  nedjo
 Updated by:   nedjo
 Status:       patch (code needs review)
 Attachment:   http://drupal.org/files/issues/theme-region.patch (6.2 KB)

This bug came from a recent change to function list_themes(), which now
lists only enabled themes.   function system_listing_save() fetched
available regions (using, indirectly, list_themes()) *before* a theme
was enabled. 


As well as the error handling (discussed above), designed to prevent
non-upgraded themes from being enabled and to give meaningful error
messages, the attached patch moves theme region initiation to after
themes are enabled.  On my testing, this fixes the reported error. 
Please test and report back.


I haven't yet tested the error handling changes to updates.inc.  Anyone
able to do that?  To test it, try to upgrade a site that has some
default (and therefore, with an upgrade, updated) themes, and some
themes (already enabled) that are not upgraded.  The expected behaviour
is that the upgraded themes have their regions initiated, while any
non-upgraded themes fail, are disabled (i.e., have their status reset
to 0), with messages given.




nedjo



Previous comments:
------------------------------------------------------------------------

Wed, 24 Aug 2005 03:10:30 +0000 : nedjo

Following the introduction of extentendable regions, see this issue [1],
several users have reported problems:


"A fresh install of Drupal with bluemarine displays blocks correctly,
but when I switch the default theme to pushbutton, all the blocks
disappear. They do show up in ?q=admin/block, but just with the yellow
helper boxes, no blocks are really displayed.--Goba

"
"Same problem as Goba.


The "region" field in the blocks table is sometimes blank. This
makes many themes almost unusable in a Drupal default install. I don't
know how the fields are blank, but the following SQL helped me get back
to theme development:


UPDATE blocks SET region="left" WHERE region=""--Thox

"
"another confirmation. I found out about the exact same sql query as
Thox to fix it:)
We came across this in an update. The theme had the regions defined,
but since it was a non-default theme the blocks dissapeared. A drupal
site, without a navigation block is simply useless.--Ber

"
The issue suggests a bug in the initiation of theme blocks.  When a new
theme is enabled, it's supposed to be initiated with blocks based on the
default theme's blocks.  I'll post a fix ASAP.
[1] http://www.drupal.org/node/16216




------------------------------------------------------------------------

Wed, 24 Aug 2005 04:11:57 +0000 : nedjo

I can't reproduce the reported issue with a clean install.


Are the themes causing these problems "updated" (are they
phptemplate-based or, if not, do they have _regions() functions)?


The specific problem seems to be that the region field isn't being set
when block regions are initiated.  Here's the relevant portion of
function system_initialize_theme_blocks():



<?php
      // If the region isn't supported by the theme, assign the
block to the theme's default region.
      if (!array_key_exists($block['region'], $regions)) {
        $block['region'] = system_default_region($theme);
      }
?>




This is executed if a newly-enabled theme doesn't have blocks already
assigned to it.  It gets the default region's settings--except the
region.  This code portion tests whether the default region's region is
available in the newly-enabled theme, and, if not, substitutes the
theme's default region.  This would all fail if the theme wasn't
returning any regions--but any phptemplate theme, or the core ones,
should be....


Anyone see what's going wrong?




------------------------------------------------------------------------

Wed, 24 Aug 2005 18:02:44 +0000 : nedjo

Attachment: http://drupal.org/files/issues/theme-region-error-catching.patch (5.73 KB)

I'm still not quite sure what the issue is, but clearly better error
handling is needed.  A particular issue is the scenario where a site
admin upgrades a Drupal install without removing non-upgraded themes,
or, on a new install, tries to install non-upgraded themes.   As things
stand, either of these could lead to unusable themes being the default,
and all blocks disappearing.


The attached (initial, untested) patch adds error handling to both
updates.inc and system.module.


New for allinstalls:


* theme region initiation return FALSE on failure TRUE on success.
* on initiation failure, theme is disabled, and, if it was set to
default theme, bluemarine is substituted, with message given


Updating:


* tests if current default theme has been upgraded, and if not sets
bluemarine as the default, issuing a notice
* tries to initialize blocks for each enabled theme and, on failure,
disables the theme.


This should help prevent upgrade problems.  If anyone has a chance to
review and test this, I'd appreciate it, as I don't at present have a
test install to upgrade.




------------------------------------------------------------------------

Thu, 25 Aug 2005 14:07:38 +0000 : Thox

The problem occurs with all of the core themes except Bluemarine. That
means .theme (chameleon) and phptemplate (pushbutton) themes are both
affected equally.




------------------------------------------------------------------------

Thu, 25 Aug 2005 15:33:37 +0000 : Thox

When I enable a theme, it goes through system_initialize_theme_blocks(),
which assigns the system_default_region() to the new theme.
Unfortunately, that's blank.


There's no themes loaded (disabled by system_listing_save()), so
there's no regions available, so there's no default region to use.


Your patch stops old themes from working, can we not simply set the
default region (if not found) to "left"? This works fine for me to fix
this problem.