Do not check_plain() usernames more than once

Looks good. The discussion already occurred in http://drupal.org/node/266488 and in http://drupal.org/node/275308 (sigh!).

Nothing more to add.

I've committed this to CVS HEAD.

I briefly looked at the two issues referenced, and could not find references to these exact issues there. Where are usernames escaped in these cases? Just using the username without escaping seems like a bad idea to me, and the code just shows that. Wherever it is escaped earlier, it might need fixing IMHO.

So let's first define how do these end up escaped twice.

It is not escaped earlier but later, in l() and drupal_attributes().

Yes, it can. The patch is correct and still applies.

-      '#value' => l(t('View recent blog entries'), "blog/$user->uid", array('attributes' => array('title' => t("Read @username's latest blog entries.", array('@username' => $user->name))))),
+      '#value' => l(t('View recent blog entries'), "blog/$user->uid", array('attributes' => array('title' => t("Read !username's latest blog entries.", array('!username' => $user->name))))),

This is safe because l() passes $options['attributes'] through drupal_attributes, which runs check_plain on the attribute values.

-    drupal_set_breadcrumb(array(l(t('Home'), NULL), l(t('Blogs'), 'blog'), l(t("@name's blog", array('@name' => $node->name)), 'blog/'. $node->uid)));
+    drupal_set_breadcrumb(array(l(t('Home'), NULL), l(t('Blogs'), 'blog'), l(t("!name's blog", array('!name' => $node->name)), 'blog/'. $node->uid)));

This is safe because l() runs check_plain directly on the $text parameter unless $options['html'] is set, which it is not here.

-        'title' => t("@username's blog", array('@username' => $node->name)),
+        'title' => t("!username's blog", array('!username' => $node->name)),
         'href' => "blog/$node->uid",
-        'attributes' => array('title' => t("Read @username's latest blog entries.", array('@username' => $node->name)))
+        'attributes' => array('title' => t("Read !username's latest blog entries.", array('!username' => $node->name)))

This is safe because template_preprocess_node passes these through theme('links') which calls l() and so is safe for the same reasons as above.

On the other hand, maybe you don't want to change strings in D6 for this, seeing as how the characters that would cause problems are not allowed in usernames anyway without some custom coding. I expect some more custom coding could get around the double escaping too.

Is anything knows when this patch will be applied to the next release of D6? We imported hundreds of members from phpBB, and several of them have an ' in their name... thanks :)

Ok, I think it is important to document why can we use unescaped text, so I've committed this to Drupal 6. I'd like to request a port of the three doc lines to Drupal 7, so we keep this knowledge in the code, and people will not attempt to "fix" this by moving away from !name and !username.

Changing to documentation component.

These are code comments - I think this belongs in the blog module for consideration...

As requested by Gábor in #13, here's a port of the code comments to Drupal 8.x. Patch applies to Drupal 7.x as well.

Looks fine to me. 8.x/7.x - no code change, just some in-code comments to clarify what is going on.

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