Boxes may be themed without a title; don't spit the header.

needs a review.

+1

Patch applies and works as advertised. This is something I do in all my themes anyway, so it makes sense to me that the default themes also behave nicely.

Looks lovely. And works as advertised. Its something I do in all my themes too, so its gets a warm +1 from me :)

LESS HTML = LESS clutter == good

applied

The Chameleon-theme and all other PHP-based themes don't rely on PHPTemplate, so theme.inc should be updated as well..

Unfortunatly I'm not in the position to make a proper patch ATM, but replacing theme_box (line 799 of theme.inc) with:

function theme_block($block) {
  $output  = "<div class=\"block block-$block->module\" id=\"block-$block->module-$block->delta\">\n";
  if ($block->subject) {
    $output .= " <h2 class=\"title\">$block->subject</h2>\n";
  }
  $output .= " <div class=\"content\">$block->content</div>\n";
  $output .= "</div>\n";
  return $output;
}

This does the job.. Could someone please roll this into a patch and directly set this RTBC? I did test this on all my 6 custom themes and Chameleon and it works..

Maybe it's a little late, but hey... Here it is!

This also cleans up theme_page() a little which was pretty outdated..

I would say, RTBC bt killes would kill me for that... ;-)

Not sure we should apply this one. This will change the default for all themes (inclusive custom ones). I am reluctant to do that this late in the release cycle. Can't we make chameleon implement implement theme_block?

Killes, I am not adding a new feature here.
I am only taking away the difference between the way PHPTemplate handles this and the Plain PHP themes. For sake of consistency that is.. IMO we could consider this as a bug, because we have different functionality in PHPTemplate and Plain PHP themes..

Killes, what do you think about this? Should I update the patch?

Rerolled to fresh head

This commit broke nickl's patch in #13

http://drupal.org/cvs?commit=39798

Re-rerolled.

regards,
--AjK

The patch includes more than there needs to be. The first two hunks can be moved to another issue (esp. I am not sure the "begin content" and "end content" ought to be removed).

Only the third hunk is relevant here.

if ($block->subject) will create an E_ALL notice. Use !empty().

Here is a simplified patch. We removed all the extra stuff and just created a patch for theme_block. We also use !empty to make things safer.

The patch does not apply because of non-standard line endings.
Please see:
http://drupal.org/patch#endings

Patch converted to Linux line endings.

The latest patch applies. Looks reasonable to me.
Is there a conscious effort in D5 to remove linebreaks from generated HTML?
(Just wondering, I don't think it should affect the patch's acceptance).

What about if $block->subject == '0'? In your patch that title wouldn't be printed. Consider using isset() and/or !== '' if it is always set.

Ahh, you learn something new every day, I had actually assumed that empty worked just the same as isset. Now using isset.

This patch is also removing all the extra newlines that were in the original output, I can't see it adding any.

And what if a block has no content? Can it happen?

I tested with chameleon, a theme that does not use phptemplate.

@dries#22: a block without body in not printed at all (I tried). No problem here.

The check should be isset() rather than !isset().

Any news about this bug??

The second part of this could break expected functionality at this point and doesn't apply to d6 and beyond.