Currently the term page ALWAYS outputs a header, even if the term doesn't have a description. This is a problem when the description needs styling. I mentioned this on my blog. See the attachments to understand the problem.

This is the code outputted:

<div class="term-listing-heading">
  <div class="taxonomy-term vocabulary-tags clearfix" id="taxonomy-term-6">
    <div class="content">
      <div class="taxonomy-term-description"></div>
    </div>
  </div>
</div>

My suggestion would be to only show this piece of HTML if the term has a description. See the patch!

Files: 
CommentFileSizeAuthor
#36 796692-term-descr.patch3.52 KBandypost
PASSED: [[SimpleTest]]: [MySQL] 29,404 pass(es).
[ View ]
#34 796692-term-description.patch3.17 KBandypost
FAILED: [[SimpleTest]]: [MySQL] 29,407 pass(es), 0 fail(s), and 3 exception(es).
[ View ]
#19 taxo_header_revert-796692-19.patch2.03 KByched
PASSED: [[SimpleTest]]: [MySQL] 27,375 pass(es).
[ View ]
#14 taxonomy_description_796692_3.patch2.67 KBdman
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy_description_796692_3.patch.
[ View ]
#9 taxonomy_description_796692_2.patch2.37 KBdman
PASSED: [[SimpleTest]]: [MySQL] 22,952 pass(es).
[ View ]
#8 taxonomy_description_796692_2.patch0 bytesdman
PASSED: [[SimpleTest]]: [MySQL] 22,969 pass(es).
[ View ]
#7 taxonomy_description_796692.patch1.06 KBcross
PASSED: [[SimpleTest]]: [MySQL] 20,827 pass(es).
[ View ]
#6 taxonomy_description_796692.patch1004 bytesaaronbauman
PASSED: [[SimpleTest]]: [MySQL] 20,824 pass(es).
[ View ]
#4 taxonomy_description_796692.patch1.04 KBcross
PASSED: [[SimpleTest]]: [MySQL] 20,834 pass(es).
[ View ]
taxonomy_description.patch1.21 KBBarisW
PASSED: [[SimpleTest]]: [MySQL] 20,297 pass(es).
[ View ]
without-description.png21.45 KBBarisW
with-description.png44.13 KBBarisW

Comments

A month without review. Can anyone have a look at this?

I searched you blog but didn't find the description of problem. Can you provide direct link?
What you mean "term page"? page with path like taxonomy/term/1 ? If so i can't notice the bug.

Hi cross,

I applied the patch on my blog so the error is fixed. See my code example: empty tags are generated if a term doesn't have a description. This is unnecessary - and ugly when you apply styling to .term-listing-heading or .taxonomy-term-description. See the screenshots.

And indeed; I mean urls like taxonomy/term/1

StatusFileSize
new1.04 KB
PASSED: [[SimpleTest]]: [MySQL] 20,834 pass(es).
[ View ]

+  ¶
+  $build = array();
+  ¶

what is it for?

Add fixed version

I have reviewed this bug and tested patch http://drupal.org/files/issues/taxonomy_description_4.patch. I think this patch is a good fix for it.

cross, when I tried to use your version of patch I had error:
Fatal error: Unsupported operand types in C:\sites\drupal7\modules\taxonomy\taxonomy.pages.inc on line 45
I don't know why we need
$build = array();
but without this piece of code I can see only php error message.

StatusFileSize
new1004 bytes
PASSED: [[SimpleTest]]: [MySQL] 20,824 pass(es).
[ View ]

You absolutely need $build = array();, otherwise, when there is no term description (a prerequisite for this issue) you're trying to do shorthand array addition with an uninstantiated variable.

The patch in #4 results in a fatal error when the term does not have a description -- try it on a clean d7 install:

Fatal error: Unsupported operand types in modules/taxonomy/taxonomy.pages.inc on line 41
Call Stack:
index.php: menu_execute_active_handler()
index.php: call_user_func_array()
includes/menu.inc: taxonomy_term_page()
includes/menu.inc:0

Re-roll of OP @ HEAD attached.

StatusFileSize
new1.06 KB
PASSED: [[SimpleTest]]: [MySQL] 20,827 pass(es).
[ View ]

#5 true. Dunno how it pass tests.
Re roll patch with only whitespace fixed.

Status:Needs review» Reviewed & tested by the community
StatusFileSize
new0 bytes
PASSED: [[SimpleTest]]: [MySQL] 22,969 pass(es).
[ View ]

Patch is fine as far as it goes, but different themers make different assumptions about whether blank placeholders are appropriate in the page or not. Sometimes that space is desired.
I generally think the themer should take steps to deal with that, and that the code should not output blank elements, so I'm happy with this 'fix'.

Anyway. This a pretty good fix.
HERE is a re-roll for HEAD
and a test that proves the problem and then proves that the fix works
So I'll flag this as tested (the patch from cross is good)

And I'm adding the simpletest, to make the fix eligible for core..

Status:Reviewed & tested by the community» Needs review
StatusFileSize
new2.37 KB
PASSED: [[SimpleTest]]: [MySQL] 22,952 pass(es).
[ View ]

Whoops, here's the roll.

Working great! :)
Thanks!

Status:Needs review» Reviewed & tested by the community

Bump it up a little.
Ready to go (I think)

Status:Reviewed & tested by the community» Fixed

Committed to CVS HEAD. Thanks.

Status:Fixed» Active

Er - that logic might have been valid in D6, but not anymore in D7.
Terms can have fields. A term can have non-empty content while its description is empty.

Can we revert this patch ?

Category:bug» feature
StatusFileSize
new2.67 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch taxonomy_description_796692_3.patch.
[ View ]

Dang. :(
Looking at taxonomy_term_view() I don't see any meaningful way we can run an empty() on the $build return value there.

Does this mean that the renderer naturally creates a lot of placeholder divs for null content in many other cases too?
Hm. Would be a lot of work to deal with that well. And potentially unpredictable. We may have to live with horrible layers of Drupal div-itis for a few more years if there is no clean way to say "don't print wrappers around empty content".

Or maybe all $build functions just have to be a lot more careful about what they build...
Sorta like this: Only fill in the $build array with rendering detail if there is genuinely content to render.
This means empty($build) can still be used further up the chain.

<?php
  $build
= array();
  if (!empty(
$term->description)) {
   
$build['description'] = array(
     
'#markup' => check_markup($term->description, $term->format, '', TRUE),
     
'#weight' => 0,
     
'#prefix' => '<div class="taxonomy-term-description">',
     
'#suffix' => '</div>',
    );
  }
 
$build += field_attach_view('taxonomy_term', $term, $view_mode);
  if (! empty(
$build)) {
   
// Only return markup if there is content to be rendered.
   
$build = array(
     
'#theme' => 'taxonomy_term',
     
'#term' => $term,
     
'#view_mode' => $view_mode,
    );
   
$build['#attached']['css'][] = drupal_get_path('module', 'taxonomy') . '/taxonomy.css';
  }
?>

Title:Only show the term heading if the term has a description[please revert] Only show the term heading if the term has a description
Category:feature» bug
Priority:Normal» Major

As I stated in #13, the patch that got committed is absolutely wrong, this needs to be reverted.
Reported in #953928: taxonomy/term/X only displays the term's fields if term has a description (marked as dupe).

subscribe

Status:Active» Reviewed & tested by the community

Let's start with a straight rollback of #9 first.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, taxonomy_description_796692_3.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new2.03 KB
PASSED: [[SimpleTest]]: [MySQL] 27,375 pass(es).
[ View ]

Patch reverting #9

Status:Needs review» Reviewed & tested by the community

Yes the original patch doesn't make any sense for D7 at all, This is a straight revert, let's get it in, then open a new issue to discuss empty divs, which is not an issue specific to taxonomy module at all.

Also term description would very likely have been a field in D7 if we'd had a bit more time, so adding even more special casing for it only makes that process harder in D8.

Yeah
I'm ok with a revert. Although the issue should still be addressed, my patch had some d6 assumptions which don't hold up under new testing

Hm. I'm a bit torn here because this is a UI change well after UI freeze. Insanely, utterly past.

And yet, it can't be denied that the way this was implemented originally wasn't correct, and this point was first brought up < 24 hours after the patch went in. And a field API maintainer thinks it should be rolled back, as well as one of the people who originally worked on it... hmmm...

I'm going to let this sit overnight and look at it again in the morning.

webchick, any further thought on this? It seems utterly broken to require data within the description field to display other fields which do have data in them.

Subscribe.

Compile and hide description for show image is insane :P

Priority:Major» Normal
Status:Reviewed & tested by the community» Needs work

Priority:Normal» Major
Status:Needs work» Reviewed & tested by the community

ops.. sorry

Just as a note, I've created a module which addresses this problem since it looks like it will not be fixed in core for D7 taxonomy_display.

+1

Subscribe.
It's really serious for sites actively using custom fields for taxonomy.

Subscribe.

Version:7.x-dev» 8.x-dev
Issue tags:+needs backport to D7

For the record : this is indeed a no-brainer for D8, but webchick had some concerns about committing this change in D7 (although I stand by the fact that the current D7 behavior is plain broken)

Subscribe...this affects contib modules like Metatags that want to use hook_entity_view_alter() with all entities since terms with no descriptions never have the hook invoked.

Title:[please revert] Only show the term heading if the term has a descriptionOnly show the term heading if the term has a description
Status:Reviewed & tested by the community» Needs review
StatusFileSize
new3.17 KB
FAILED: [[SimpleTest]]: [MySQL] 29,407 pass(es), 0 fail(s), and 3 exception(es).
[ View ]

Suppose we just need a proper fix for that

Status:Needs review» Needs work

The last submitted patch, 796692-term-description.patch, failed testing.

Status:Needs work» Needs review
StatusFileSize
new3.52 KB
PASSED: [[SimpleTest]]: [MySQL] 29,404 pass(es).
[ View ]

Seems 'content' should be initialized to fix notices

Status:Needs review» Reviewed & tested by the community

Looks good, thks @andypost.

Would love to see some movement on this, and hopefully have it present in 7.1 considering how broken term pages are at the moment without this. :-(

I can confirm that it's working.

Status:Reviewed & tested by the community» Fixed

Committed to 7.x and 8.x. Thanks!

Status:Fixed» Closed (fixed)
Issue tags:-needs backport to D7

Automatically closed -- issue fixed for 2 weeks with no activity.