I am very fanatic at semantics... I think the (x)html to represent an breadcrumb list that is used now could be much better...

A breadcrumb is an ordered list with links to the currents page's parents...

/**
 * Return a themed breadcrumb trail.
 *
 * @param $breadcrumb
 *   An array containing the breadcrumb links.
 * @return a string containing the breadcrumb output.
 */
function theme_breadcrumb($breadcrumb) {
  return '<div class="breadcrumb"><ol><li>'. implode($breadcrumb, '</li><li>') .'</li></ol></div>';
}

and with a little piece of css u can define the style of your breadcrumbbar.

Not really a bug, more like an improvement...

Besides that you of course can override this function in your theme_engine, but I think it is important to have a solid default theme.

Comments

gravyface’s picture

I totally agree; semantics (or tight, structural markup in general) makes things alot easier. Why not do up a Book page on " Semantic Markup and Drupal" and use this (and any other tips you can think of) as an example for module developers? I think while most developers are starting to get better with their markup, there are quite a few coders who think throwing in a bunch of
's is still fine. :)

gravyface’s picture

that was supposed to be "bunch of <br>'s" Oops.

eferraiuolo’s picture

There has been a bug in the display of breadcrumbs that was present before this function override, and is present in this function override.

The bug is that if you're on the front_page of a site, an empy div, or in this case an empty ordered list with one empty list-item is outputted. I have found a way to correct this error by checking if there is anything in breadcrumbs first before outputting them.

function phptemplate_breadcrumb($breadcrumb) {
  if (implode($breadcrumb, '') != "") {
    return '<div class="breadcrumb"><ol><li>'. implode($breadcrumb, '</li><li>') .'</li></ol></div>';
	}
}

I am not a PHP guru so if you know how to make the IF statement better, by all means do so! ;)

casey’s picture

You're right; it should be checked whether the breadcrumb array is empty. But my point is that this issue should be fixed at core level, not only in your theme or engine.

I thought you even could remove the div (when u want one u could include these in your page template).

For some more explaination: I used the "ol" tag because the breadcrumb is actually ordered (it has a start and an end).

So it should be something like this:

/**
 * Return a themed breadcrumb trail.
 *
 * @param $breadcrumb
 * An array containing the breadcrumb links.
 * @return a string containing the breadcrumb output.
 */
function theme_breadcrumb($breadcrumb) {
  if (!empty($breadcrumb))
    return '<ol class="breadcrumb"><li>'.implode($breadcrumb, '</li><li>').'</li></ol>';
}
magico’s picture

Version: 4.6.0 » 4.6.9
Priority: Minor » Normal

Is this a feature or a bug? Current HEAD still does not have this, and in fact the breadcrumb should be more semanticly defined....

magico’s picture

Version: 4.6.9 » x.y.z
Category: bug » feature
LAsan’s picture

Version: x.y.z » 7.x-dev

Is this implemented in current version?

Feature request go to cvs.

dvessel’s picture

Status: Active » Closed (works as designed)

I don't see how this is semantic. It's not supposed to be a list of things. It is simply an inline hierarchy. Embedding ul within ul would be more semantic but that's silly.

dman’s picture

I like semantics, but am tired of everything being turned into 'lists'. Turning image galleries into LIs was bad enough.
My content could be modelled as a list of paragraphs if you wanted to be 'semantic' about it.
Not every series of items needs to be a list

Basically, presenting breadcrumbs like this will, by default, make them a vertical list in every single browser. And we never want to present breadcrumbs like this. (Indented and treed, possibly in extreme cases, but that takes nested ULs as mentioned above)

Being forced to use CSS tricks to flatten them again in every single case and every single theme and not just as a matter of design choice, shows that this would be an architectural mistake. It's just make-work.
There's nothing un-semantic about named divs.

Presenting this as a theme snippet for someone that really want it is a good idea.
Adding the check for is_empty() is a good idea.
Making a page require full customized css to render intelligibly is a bad idea.