I see that there are hooks available to alter the potential references, but hook_block_info_alter() should also be used, since it's part of core and the Block module. Since you're directly querying the database for blocks, that hook is never invoked and thus the changes from it aren't present.

I would suggest adding it in there to keep things uniform.

CommentFileSizeAuthor
#16 blockreference-use_core_block_info_alter_hook-2240497-16.patch4.95 KBmstef
#13 blockreference-use_core_block_info_alter_hook-2240497-13.patch4.13 KBmstef
#1 blockreference-use_core_block_info_alter_hook-2240497-1.patch3.8 KBmstef

Comments

mstef’s picture

mstef commented
StatusFileSize
new blockreference-use_core_block_info_alter_hook-2240497-1.patch3.8 KB

Here's a stab at it..

rudiedirkx’s picture

rudiedirkx commented
Assigned: Unassigned » rudiedirkx

Ha, good point. I've never used that one, so I forgot about that one. Same bug in 2.x.

Is $theme_key the right theme? It should be the front-end theme, not the admin theme. The Drupal blocks table is weird. It doesn't work like that in 2.x anymore.

I'll check it out later.

mstef’s picture

mstef commented

I was thinking about that.. Maybe use variable_get('theme_default', '')?

rudiedirkx’s picture

rudiedirkx commented

That's a way. Maybe $theme_key is smart. There's also global $theme, which is also a string, so I don't know which one's better. I don't know the difference.

Which theme shouldn't be important at all. Blocks don't work like that. All blocks are available in all themes. 1.x is weird. You should try out 2.x, which has cleaned up a lot. (Still this bug though, but with a much easier fix in _blockreference_find_blocks().)

rudiedirkx’s picture

rudiedirkx commented

Wtf happened to the preview button!? That was a useful button!

rudiedirkx’s picture

rudiedirkx commented

Actually, I might've changed my mind... _block_rehash() alters the block list when it's fetched from the db. I don't touch the db. (1.x does, 2.x doesn't.)

What are you trying to do in this block info alter? The only thing you can do (in 2.x at least) is change the admin label (in $block['info']).

The original purpose of the info alter isn't reached with 2.x (or 1.x I think), because all flags (cache, region etc) are ignored, except 'info'.

I rather not add it. Thoughts?

mstef’s picture

mstef commented

I change 'info' in certain blocks under certain conditions that I want replicated in the autocomplete results for blockreference fields. Like I said, I can probably do the same thing using the custom hooks from this module, but it seems weird to duplicate code and to ignore a core hook that is also supposed to do the same thing.

Blocks have different settings per-theme, which is why the theme is passed in along with that hook. $theme_key is the current active theme on that request. I would think using variable_get() would be a better parameter to use, since it's the default theme.

I would jump to 2.x but this is already being used on the project and I don't want to migrate at this point.

rudiedirkx’s picture

rudiedirkx commented

2.x has a full update path =) (Be careful of contrib modules using the Blockreference api though.)

You can choose and I'll do it like that in 2.x. No 'new features' in 1.x.

1. No block info alter, so autocomplete (and label and sorting etc) altering happens in blockreference hooks.
2. Block info alter per block providing module, so your block info alter implementation function is called several times, not once. (If you've configured only Views and Nodeblock to provide blocks, the function is called twice.)

Whatever you want =) Only real bug fixes in 1.x.

mstef’s picture

mstef commented

Alright, I understand. Well I did my part. You can choose how you want to handle it from here. I'll migrate to 2.x when I can - but for now, I'll be on 1.x.

mstef’s picture

mstef commented

Your custom hooks to alter are actually pretty useless without the other info from hook_block_info().

For example, in my hook_block_info_alter(), I change the titles of Menu blocks. There is no direct way for me to know which blocks are from menu via your hooks, since only bid and title are passed in.

rudiedirkx’s picture

rudiedirkx commented

Yes, 1.x sucks.

You should link to the patch in #1 in your build file or PATCHES.txt (however you organize that) and just patch your version. There won't be a new 1.x release for a long time, probably, so shouldn't be any trouble.

2.x should pass in everything from block_info into the custom block list alters. If not, let me know soon, so I don't have to break backward compatibility before 2.0.

mstef’s picture

mstef commented

Oh alright. I didn't know that. Thanks.

If you do ever decide to use my patch for anything, the variable names in the last two foreachs should be changed from $bids and $bid to $deltas and $delta.

mstef’s picture

mstef commented
StatusFileSize
new blockreference-use_core_block_info_alter_hook-2240497-13.patch4.13 KB

Updated patch, just in case someone finds this issue and uses it. I added a line to remove blocks that are no longer available via hook_block_info() (ie, an old block stuck in the DB).

rudiedirkx’s picture

rudiedirkx commented
Status: Active » Needs review
mstef’s picture

mstef commented
Status: Needs review » Needs work

The validation on the field messes up if the title of the block is altered..

Might need to remove this or do it differently..

        $result = db_query('SELECT module, delta FROM {block} WHERE bid = :bid', array(':bid' => $bid));
        $block = $result->fetchObject();
        $info = module_invoke($block->module, 'block_info');
        $real_title = $info[$block->delta]['info'];

        if (trim($title) != trim($real_title)) {
          form_error($element, t('%name: Title mismatch. Please check your selection.', array('%name' => $instance['label'])));
        }
mstef’s picture

mstef commented
Status: Needs work » Needs review
StatusFileSize
new blockreference-use_core_block_info_alter_hook-2240497-16.patch4.95 KB

This removes that validation. I can't see a reason why it's needed.

mstef’s picture

mstef commented
Status: Needs review » Needs work

Looks like blockreference_autocomplete_value() needs to be fixed as well..

pifagor’s picture

pifagor commented

the patch is outdated. Can you upgrade it?

pifagor’s picture

pifagor commented
Assigned: rudiedirkx » Unassigned
alex_optim’s picture

alex_optim
Location Tallinn
commented
Status: Needs work » Reviewed & tested by the community

I don't see a need for working with this patch. Because the module is very changed and has another logic.
This patch is deprecated.
Now module working good. If is some problems with the module work I propose to create a new issue for current the module version.

pifagor’s picture

pifagor commented
Status: Reviewed & tested by the community » Postponed