[drupal-devel] [bug] Blocks showing up when module disabled

[Uwe Hermann]

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

 Project:      Drupal
 Version:      cvs
 Component:    module system
 Category:     bug reports
 Priority:     minor
 Assigned to:  drumm
 Reported by:  Jose A Reyero
 Updated by:   Uwe Hermann
-Status:       patch (code needs work)
+Status:       patch (code needs review)

Patch still applies, but I didn't test it.




Uwe Hermann



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

Thu, 02 Dec 2004 13:19:31 +0000 : Jose A Reyero

This only happens in the module administration page.


To reproduce this behaviour:
1. enable a module and one block provided by it - I.e. 'archive' module
2. disable the module
The block still shows up in the module administration page


I find this more funny than annoying and it happens only in module
admin page.


Anyway, think it's and undesirable behaviour and should be addressed,
maybe at the block management level.




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

Thu, 16 Jun 2005 04:37:45 +0000 : deekayen

I verified this in HEAD for the archive module.




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

Thu, 11 Aug 2005 20:49:44 +0000 : tostinni

This still apply to HEAD, the procedure to reproduce the bug is the
right one.


However, I don't understand exactly why the block keep appears on this
page, and not on the others pages. The problem is that looking at the
blocks table, "archive" block stay there even if it doesn't appear on
other pages (on module page yes it does). When I enter a new page,
saying home, archive block isn't there but is still on the db.
The cache is flushed, because archive seems not to appear on the entry
giving me the menu, but the current page keep the old cache entry (well
I guess).


Ok so I proposed a radical way to correct this : rehashing blocks after
saving the module configuration. So I just add a _block_rehash(); after
the menu_rebuild();, it's a little dirty but do the trick. If you have
any suggestion about a better way to do this ;)

<?php
function system_listing_save($edit = array()) {
  $op = $_POST['op'];
  $edit = $_POST['edit'];
  if ($op == t('Save configuration')) {
    db_query("UPDATE {system} SET status = 0 WHERE type = '%s'", $edit['type']);
    foreach ($edit['status'] as $name => $status) {
      // Make certain that the default theme is enabled to avoid user error
      if (($edit['type'] == 'theme') && ($edit['theme_default'] == $name)) {
        $status = 1;
      }
      db_query("UPDATE {system} SET status = %d, throttle = %d WHERE type = '%s' AND name = '%s'", $status, $edit['throttle'][$name], $edit['type'], $name);
    }
    if ($edit['type'] == 'theme') {
      variable_set('theme_default', $edit['theme_default']);
    }
    menu_rebuild();
    _block_rehash();
    drupal_set_message(t('The configuration options have been saved.'));
    drupal_goto($_GET['q']);
  }
}
?>






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

Thu, 11 Aug 2005 22:17:26 +0000 : drumm

I can work on this.




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

Fri, 12 Aug 2005 00:20:14 +0000 : drumm

Attachment: http://drupal.org/files/issues/system.module_5.diff (3.54 KB)

I decided to implement a hook system for this. As a side effect:


- system_listing_save() only does database things. The UI things such
as drupal_set_message() are now separated into the calling function.


- The status message for saving module is now:
    * The aggregator module has been disabled.
    * The archive module has been enabled.
instead of "The configuration options have been saved."


- The theme-specific things in system_listing_save() are not there
anymore.


- There is also a hook called when modules are enabled.


I think I'll go write some documentation for the two new hooks now.




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

Fri, 12 Aug 2005 00:48:40 +0000 : drumm

Here is some hook documentation:



<?php
/**
 * This hook is called on every module for when each new module is enabled. The
 * module will be loaded at this point so the module's own hook will be called
 * and other modules may use module_invoke() to call functions in the module.
 *
 * @param $module
 *   The name of the module which has just been enabled.
 */
hook_module_enabled($module) {
}
/**
 * This hook is called on every module for when each new module is disabled.
 * The module will still be loaded at this point so the module's own hook will
 * be called and other modules may use module_invoke() to call functions in the
 * module.
 *
 * @param $module
 *   The name of the module which has just been disabled. T
 */
hook_module_disabled($module) {
}
?>






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

Wed, 07 Sep 2005 19:18:22 +0000 : drumm

This patch is now fubar. Please feel free to resubmit the concept in
update #2 by tostinni in patch form.




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

Wed, 07 Sep 2005 20:17:08 +0000 : tostinni

Sorry I didn't understood "This patch is now fubar." ?
This patch has been abandonned ?




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

Thu, 08 Sep 2005 00:18:24 +0000 : drumm

Yes, I am not going to work on my approach currently. The patch was
broken by the recent addition of multiple block regions. I may have
time in the future.


I recomend making a real patch file for the simpler approach as
describe in update #2.




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

Thu, 08 Sep 2005 15:56:09 +0000 : tostinni

Attachment: http://drupal.org/files/issues/system_block_rehash.patch (510 bytes)

Ok here goes the patch.