Invalid HTML being generated

This particular invalidity cause (nested P elements) has apparently been removed in the meantime, but another exists: taking for example this very page on d.o., for unlogged-in users, it is still invalid for two reasons:

1. two errors appear because It contains: <div class="links">» <a href="/"><a href="/user/login?destination=project/comments/add/56944">Login</a> or <a href="/user/register?destination=project/comments/add/56944">register</a> to follow up</a></div>, which is invalid because of nested A elements in the "Follow up" link, which seems to be part of project_issue_link, although I don't see how
2. another is duplicate XHTML IDs (one is for log in, the other for search), but these are outside project scope

Hunmonk,

Just look at : http://validator.w3.org/check?uri=http%3A%2F%2Fdrupal.org%2Fnode%2F56944...

as of today, the invalid HTML is still being generated for the same errors with the version currently running on d.o. (although I don't know which one it is).

It's rather difficult to guess without being able to test on the site itself, but I looks rather like a case of hook_link_alter somewhere on the site adding <a href"/"> to the link in project_issue_link, which already includes a <a href="[...] returned from theme_project_issue_follow_up_forbidden.

In such a case, theme_links, as invoked from node_view, will generate the extraneous <a href="/"> causing this error.

But without access to the actual d.o. code, it's hard to tell.

Yup, I've identified the problem thanks to a fragment of bluebeach killes pasted me. the culprit appears to be function phptemplate_links($links) , which indiscriminately invokes l($link['title'], $link['href'] ... without checking whether a href attribute exists or not.

And since l() does not allow non-linking A elements, it builds a link to the home page, causing what we see.

The obvious solution is to check whether such an attribute exists and invoke l or not depending on whether the attribute is present or not.

The arguably better solution would be to allow l() to work without a href attribute, but IIRC the last time I suggested this, it was rejected on backwards compatibility grounds.

This should fix the problem. However, I don't have access to bluebeach so can't generate a patch or test.

function phptemplate_links($links) {
  $html_links = array();
  foreach ($links as $link) {
    if (isset($link['href'])) {
      $html_links[] = l($link['title'], $link['href'], $link['attributes'], $link['query'],
$link['fragment'], FALSE, $link['html']);
    }
    else {
      if (!$html) {
        $link['title'] = check_plain($link['title']);
        }
      $html_links[] = '<span'. drupal_attributes($link['attributes']) .'>'. $link['title'] .'</span>';    
    }
  }
  return implode(' <C2><B7> ', $html_links);
}

This test is actually lifted from API sample for D5 theme_links.

Forgot to set status

whoops, you're right, I missed the line building $html when pasting.

Issue still present, try the link on http://drupal.org/node/56944#comment-392973

Since bluebeach is not public, I can't test the patch on the current version.

If SVN trunk and 5 is what's currently on d.o., the problem is not fixed.

However, not being a member of the infra team, I don't have SVN access to test it elsewhere: these apparently don't have the same accounts as CVS.

Then it does not completely fix the issue: the same problem is present on scratch.drupal.org with still 16 errors, in comparison with 61 errors on drupal.org for the same page.

Check http://validator.w3.org/check?uri=http%3A%2F%2Fscratch.drupal.org%2Fnode...