Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Spotted a few issues here I think. Just a visual review so far but will verify later.
First it looks like i18nblocks is designed to do two things - provide translatable custom blocks to replace core's 'boxes' table, but also use the i18nstrings method to translate user-provided custom titles for blocks provided by contrib modules.
The i18nblocks_block() function is commented out though, so I'm not clear if the first use case is still valid - although there's all the supporting code for it still there.
A few code snippets which need looking at further:
* @ TODO Add strings on block update.
/**
* Implementation of hook_locale().
*
* This one doesn't need locale refresh because strings are stored from module config form.
*/
function i18nblocks_locale($op = 'groups', $group = NULL) {
switch ($op) {
case 'groups':
return array('blocks' => t('Blocks'));
}
}
On visual inspection it looks like we would need $op = 'refresh' in the case where custom titles are being updated from the block admin interface, and the @TODO seems to indicate the same thing.
As nedjo noted, i18n_remove_string() isn't used, so should also be added.
So - need to check refresh and delete and probably add both in, opened a separate issue for i18n_block() - #356519: i18nblocks_block() is commented out
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | i18nblocks_356350.patch | 2.19 KB | catch |
Comments
Comment #1
catchLooked at that this and found some issues:
When deleting a custom block, there's currently nothing in place to remove the strings from the locale tables. Seems like that would probably have to be done via a form alter on the confirmation form since I can't see an obvious place to hook in otherwise.
When changing the title of a module provided block, tt() isn't called on the title, so that can't be translated. If we were to add that, would we also need to add a setting to the configuration form to make it translatable, or should this just happen by default? I'm leaning towards it happening by default.
However, when a module provided block is removed (i.e. the module is disabled), it seems as if the only thing which indicates its removal would be _block_rehash(). Doesn't seem to be any way to hook into that process at all, except maybe a lot of manual messing about in $op == 'list' inside hook_block...
Posting this now in case I've missed anything obvious, will work up a patch and post a bit later on.
Comment #2
catchOK, here's a patch for just the deletion of strings for custom blocks. I spent some time looking at title for module provided blocks, but unless we leave a lot of stale strings in the database, there's going to be lots and lots of work to do to remove them if the module is disabled - it looks like potentially a re-implementation of _blocks_rehash /inside/ hook_block unless I've missed something obvious.
Since that's currently not supported, I'd suggest we just commit this change, and maybe open a new issue if that functionality is desired.
this patch also includes the necessary change to i18nstrings to let i18nstrings_remove_string() be used directly.
Comment #3
Jose Reyero CreditAttribution: Jose Reyero commentedLooks good, committed, thanks