In Drupal 5, setting 'status' to 1 enables the block in the theme's default region when the module is enabled. Due to the way drupal_write_record() sets default values, if 'region' isn't specified, it is set to '', regardless of the value of 'status'. If a block is enabled but has no region, it does not appear on the Blocks admin form, and can't be changed without editing the database directly.
_block_rehash() does the appropriate thing when the block is disabled by setting the region to '', but it should also take care of the opposite case.
| Comment | File | Size | Author |
|---|---|---|---|
| #10 | 252707_block_status.patch | 4.5 KB | Island Usurper |
| #6 | 252707_block_status.patch | 1.27 KB | Island Usurper |
| #4 | 252707_block_status.patch | 877 bytes | Island Usurper |
| #1 | block-issue252707.patch | 831 bytes | ztyx |
Comments
Comment #1
ztyx commentedI ended up here because of the bug #311150: Teaser block not visible in block list.
I'm attaching a patch. Is it something like this you are thinking about?
Edit: Wrong reference to bug. Fixed.
Comment #2
bricef commentedAnything new about this bug? I had the same problem on several version of Drupal including the last one.
Comment #3
bricef commentedComment #4
Island Usurper commentedI don't think the patch in #1 works because the problem is actually in the other branch of the if/else. New blocks need to have their region set if their status is 1 so that they will be visible. Also, _block_rehash() ought to be run when modules are enabled so that any active blocks will be automatically visible. Requiring a visit to admin/structure/block makes setting $block['status'] kind of pointless.
Test to follow.
Comment #5
jhodgdonspelling error - should be block_modules_enabled
Maybe we should probably also add something to the docblock for hook_block_info() to explain how to use the status flag and the region?
Comment #6
Island Usurper commentedActually, if this patch goes in, I think the docblock for hook_block_info() is fine. If the 'region' isn't specified, the theme's default region is indeed used, and 'status' enables the block as expected. If anything needs explaining, it's probably what the different values for 'visibility' mean: only the paths in 'pages', all except the values in 'pages', or 'pages' is PHP and returns TRUE or FALSE. They should probably be defined constants, but I guess that's another issue.
Comment #8
jhodgdon#6: 252707_block_status.patch queued for re-testing.
Comment #10
Island Usurper commentedAnd now with tests.
I'm a little concerned because these changes affect the way installation profiles activate blocks when they are run. block_modules_enabled() loads all of the blocks into the database for every theme whenever a module is enabled, so installation profiles can no longer blindly insert rows into {block}. The merge queries I put in the install files can probably be update queries instead, but it would be cleaner with some different data structures.
Install profiles also enable their dependencies one module at a time, so that means _block_rehash() gets called a lot. I would wish there was a way to trigger it at the end of installation, but I don't see an easy way to do that.
Comment #12
Island Usurper commented#10: 252707_block_status.patch queued for re-testing.
I just don't see how I could affect the DrupalRender tests.
Comment #14
Island Usurper commented#10: 252707_block_status.patch queued for re-testing.
Comment #16
jhodgdonThose do seem like consistent failures in drupal render tests due to this patch (I'm not seeing them on other patches being tested in the last few days).
Comment #17
Island Usurper commentedThat's what I thought at first, but then I tested again without this patch and I still got the errors. I don't know if it's just that site is messed up or what.
Comment #18
Island Usurper commentedOK, it does seem like the patch is causing the errors, but I'm not exactly sure how. When DrupalRenderUnitTestCase runs, it loads the common_test.module, but it's hook_theme() implementation isn't being added to the theme registry. I guess calling _block_rehash() in block_modules_enabled() is screwing that up somehow, but really only during the test.
Comment #19
jhodgdonWhen the test framework starts up, it doesn't always do everything it would if you installed Drupal and then a bunch of modules. This is an annoying "feature" of the testing module -- it takes some shortcuts. You might just need to rebuild the theme registry in your test's setup() method.
Comment #20
jhodgdonbump. Going through old issues in my queue... is this still a problem?
Comment #21
Niklas Fiekas commentedProbably yes and also related to #512464: Auto-enable a block's 'status' with a non-empty 'region' when a module is installed.
Comment #22
sunMarking as duplicate of
#1373312: Assign system_main block to 'content' region and help block to 'help' region by default (followup)
#512464: Auto-enable a block's 'status' with a non-empty 'region' when a module is installed