Add configuration option to show blocks only on pages of certain node type

I hope at some point we can go beyond just "showing blocks on certain node types" to actually determine what blocks to show at the "per page" level (although a page in and of itself is somewhat nebulous).

Essentially, if I have '/home/about/' I would like to be able to target that one page a "add blocks" to it in various regions. Of course, doing this in a cascading way would also be great: if I have '/home/about/' to which I add Blocks X, Y and Z and then I have '/home/about/bios/', I could disable X and Y but leave Z alone and it would be "inherited".

I realize that there was a huge discussion about this topic that I didn't completely follow, so hopefully my comments aren't too far off base.

Chris, have you seen the block configuration in HEAD? Drumm developed that patch...

The coding style (placement of spaces and brackets) needs a bit of work, but the functionality is a welcome addition that I would commit to HEAD.

Plus, I suggest renaming some variables for readability. The following can be improved:
+ if ($enabled && $matched && $typematch) {

1. $typematch -> $type_match
2. $matched -> $page_match

For consistency with the 'page matching', I suggest that you transform the radio-buttons to checkboxes so multiple node types can be selected and that we rename 'block.type' to 'block.types' (cfr. 'block.pages', plural). On drupal.org, I'd like to disable some block on 'project issue' pages which would not be possible with your patch, but which would be possible if checkboxes were used.

Lastly, avoid the word 'node' in output: use 'post' instead.

On a related note, there is also an interest in being able to display blocks on a role-base so if time permits, you might want to introduce a 'block.roles' column too. ;-)

I vaguely remember a patch by Neil to tidy up the block table: not sure if that is still around and whether that needs to be considered.

Some small nits (nothing major):

1. I'd vote against using $foo. Typically, in that context, we use $result but that variable name might already be in use.

2. if ($type_match = ($node->type == $type)) break 1; should be:

 if ($node->type == $type) {
  $type_match = TRUE;
  break; 
}

3. We try not to glue words together. For example: $nodetypes would be $types, or $node_types if $types is already in use.

4. I would change 'node type(s)' to 'content type(s)'.

Otherwise this patch looks perfect! Good job.

The patch doesn't apply:

patching file modules/block.module
patch unexpectedly ends in middle of line

Also, I suggest to remove the new SQL query and to replace it with '$node = node_load(array('nid' => arg(1));'. The function node_load is caching, and will therefore save us an extra SQL query.

Can you try to resubmit your patch?

Saving the block configuration doesn't seem to work.

When I make some configuration changes, save my block, and go to the same block configuration page again, the configuration is lost (the checkboxes are no longer checked). I tried this with the forum block. I configured the forum block to show up on forum pages only, yet it continues to show up everywhere. There is no error in my watchdog; the block table has been upgraded properly. The settings do not appear to be saved.

The update path is dated. You should use update_220 or update_221 instead. I can fix that.

Upon trying this patch I found that the configuration form was somewhat unclear. It's not clear that all conditions need to hold true for the block to show. That is, does it use 'OR' or 'AND' to evaluate the various criteria.

Same for the "Show on specific pages:" setting. Does it apply to the newly added "Show for specific content types:" section or just for the "Pages:" section? I think we need to shuffle the form elements and form_group()s around a bit to communicate this better.

We're getting there ...

Looks a lot better! I have only one small nit: on the screenshot, replace 'node (type)' by 'content (type)'.

I'm going to commit a patch by Neil that updates update.inc so make sure to update your tree first.

Thanks for the great work Andre.

Committed to HEAD. Thanks. (I made some minor changes to the block configuration page.)

Please note that the following lines were changed incorrectly:
- drupal_set_message(t('The block %name has been deleted.', array('%name' => ''. $box['info'] .'')));
+ drupal_set_message(t('The block %name has been deleted.', array('%name' => '' . $box['info'] . '')));

- $form = '

'. t('Are you sure you want to delete the block %name?', array('%name' => ''. $box['info'] .'')) ."

\n";
+ $form = '

' . t('Are you sure you want to delete the block %name?', array('%name' => '' . $box['info'] . '')) . "

\n";

I fixed them in CVS. It may suond like nitpicking, but it is important for readability and for doing searching on code.

Committed to HEAD.