If you are a Drupal module developer, I have a request for you. A plea for you to help make my life easier, and likely the lives of many other site builders.
I'm currently building a website that uses a theme override to display a prettily formatted username. Drupal provides a way to do this via the theme_username() API call, so in all places where Drupal core displays a user's name, my pretty string now shows.
However, when I started adding contrib modules for extra functionality, I found that quite a few of these don't use the theme call to display a user's name. Instead, they simply print out the value of the name field in the user object. If I want to change that as a site builder, my only choice is to hack up the module so it displays what I need.
For example, a Drupal 6 module might set a page title on a tab somewhere in the user account like this:
drupal_set_title(t('Widgets for !username', array('!username' => $account->name)));
Whilst what it should be doing is passing the name through the theme layer first, like this:
drupal_set_title(t('Widgets for @username', array('@username' => strip_tags(theme('username', $account)))));
And the same goes for calls to drupal_set_message() for status or error notifications, or anywhere else a user's name might be displayed.
This way the site builder is in charge of how users are displayed and won't need to hack up any contrib modules. Though that's not hard, it makes updating them unnecessarily painful, as any changes would need to be re-applied.
So my plea is, if you're a module developer and your module displays $account->name somewhere, please change your module to use theme('username') instead.
Conversely, if you're using a contrib module and find that it prints $account->name when it really should use theme('username'), please open an issue and attach a patch if you can. I will :-)
Comments
You can't always use theme_username()
theme_username() displays a HTMl link (given the required permissions) to the profile. That is in many cases not desired (e.g. exactly when displaying the name in the title or in a select list).
Drupal 6 has no real solution for this problem. realname.module supports a plain option in it's theme_username() but that doesn't work if that is not installed.
In Drupal 7, there is http://api.drupal.org/api/drupal/includes--common.inc/function/format_us... and that is what you should use in these cases. Obviously, that also means that in Drupal 7, if you want to display the real name or similar, you need to implement http://api.drupal.org/api/drupal/modules--system--system.api.php/functio... instead of overriding theme_username(). Like realname.module in Drupal 7 is doing.
Re: You can't always use theme_username()
I think that if a link isn't desirable, a module can simply pass the output of theme_username() through strip_tags() or check_plain() or use the @username placeholder instead of !username, like dalin said.
At the end of the day, as builder I still have control over what my theme_username() implementation does, so I can check arg() and various global variables to control whether a link is output or not. Without the theme call I don't have that choice, so my only recourse then is to modify the module code.
On the site I'm building, users can't specify a username - login is done via email+password only, so having a module display whatever happens to be in $account->name makes no sense.
Re: You can't always use theme_username()
A good reminder to use theme functions to render output wherever possible. Things like theme_list and theme_image (run through theme() of course) are also underused.
But I don't think that a module should be making the decision to strip the html from the output of a theme function - it kind of defeats the purpose of overriding a theme in the first place, and you are back to the original problem.
A better approach in this case might be for the module to provide its own theme function, allowing the themer to override it if necessary. A module should never be making those decisions about markup without wrapping it in a theme function.
Re: Don't hate your builders and themers
The bigger problem with
drupal_set_title(t('Widgets for !username', array('!username' => $account->name)));
is that it's a gaping XSS vulnerability. The account name is sent directly to the browser without being stripped of HTML tags (or UTF8 hacks). What should happen is (note the @):
drupal_set_title(t('Widgets for @username', array('@username' => $account->name)));
See the t function for more info.
Re: Don't hate your builders and themers
Oh yes - definitely in case of page titles. Maybe I shouldn't have used that as example, I'll update it. Status messages are less so an issue; I actually don't mind having them be clickable.
Re: Don't hate your builders and themers
You say "hack up contrib" as if it's totally bad, but might I suggest another approach that makes this less scary, and yes, maybe beneficial? If you keep the instructions to build your site in a makefile (used by drush make), then when you are forced to hack up a contrib module, you simply git-clone the module, make your changes, commit them, an then run "git diff --no-prefix --relative " and a diff will be created back to the stable version tag you create.
Then just post that to the modules issue queue (hooray! You improved drupal :)
Once it's posted to the queue, just use drush make to apply that patch and rebuilt, so your version of that contrib will be built correctly, you'll have your whole site in code (which is good and grand), and you'll have fixed the issue you've found (which is much better than putting the onus on the maintainer :)
These guys did a good job of summarizing this approach of using makefiles (and install profiles, but that's another story):
http://wiredcraft.com/blog/drush-make-and-install-profiles-drupal-7
http://walkah.net/blog/every-drupal-site-install-profile
Anyhow, cheers! And thanks for the post!
Re: Don't hate your builders and themers
Community rules, so you could try to commit your "hacks" back to drupal community.
Just create an issue on the project page, and commit the patch.
Re: Don't hate your builders and themers
Indeed! When I wrote the final paragraph I did mean to imply that's what I was going to do - and I did one earlier today (for userpoints.module).
Add new comment