Posted by rernst on April 17, 2007 at 4:55pm
6 followers
| Project: | Drupal core |
| Version: | 7.x-dev |
| Component: | block.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
Issue Summary
When switching to a new theme that has never been activated before, information about the blocks that should appear are copied to the new theme configuration but at the same time, the title values of all the blocks are not copied.
What you wind up with is the same block configuration as your old theme, only the blocks have no titles. Is this the intended behavior, or is this a bug? To me it seems like a bug, as I don't know if it makes sense to have titles 'disappear' when you change themes. I've attached a patch which fixes the behavior
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| theme_titlecopy.patch | 1.19 KB | Ignored: Check issue status. | None | None |
Comments
#1
The linked patch is not quite correct. It inserts the title as an integer instead of as a string, so all the titles get set to zero (because php tries to read the titles as numbers and fails to parse anything useful out of them).
To fix this, change the last %d to '%s' (with quotes). Thus, the amended patch would be:
@@ -1063,8 +1063,8 @@if (!array_key_exists($block['region'], $regions)) {
$block['region'] = system_default_region($theme);
}
- db_query("INSERT INTO {blocks} (module, delta, theme, status, weight, region, visibility, pages, custom, throttle) VALUES ('%s', '%s', '%s', %d, %d, '%s', %d, '%s', %d, %d)",
- $block['module'], $block['delta'], $theme, $block['status'], $block['weight'], $block['region'], $block['visibility'], $block['pages'], $block['custom'], $block['throttle']);
+ db_query("INSERT INTO {blocks} (module, delta, theme, status, weight, region, visibility, pages, custom, throttle, title) VALUES ('%s', '%s', '%s', %d, %d, '%s', %d, '%s', %d, %d, '%s')",
+ $block['module'], $block['delta'], $theme, $block['status'], $block['weight'], $block['region'], $block['visibility'], $block['pages'], $block['custom'], $block['throttle'], $block['title']);
}
}
}
#2
Holy cow. I just ran into this in D7.
#3
Updated version of the original patch attached. The premise is sound, and the patch still works.
#4
Setting back to block.module.
#5
Can drupal_write_record() be used here instead?
It seems like that might avoid these kinds of bugs in the future.
#6
Why yes, we can.
#7
The last submitted patch failed testing.
#8
Katherine and I actually beat the testbot to figuring out this was wrong, but good that the testbot saw it too :)
We need to be writing new database records, rather than overwriting existing ones.
So something like this perhaps:
<?phpunset($block['bid']);
drupal_write_record('block', $block);
?>
Or alternatively going back to the original approach would be fine too, but in the long run I feel like a drupal_write_record() solution might be a little cleaner.
#9
I don't really see the need for drupal_write_record() now that we have the new DB layer, to be honest, but it makes no real difference to me. As long as it works. :) Here's a patch with unset($block['bid']) as suggested.
#10
Looks good to me, and I verified that it works.
I think drupal_write_record() is good because it prevents having to keep track of the entire list of database columns in the code, which is kind of fragile :)
#11
Committed to CVS HEAD. Thanks.
#12
Automatically closed -- issue fixed for 2 weeks with no activity.