theme() doesn't replace missing arguments with hook defaults

theme() changed considerably in Drupal 7. I haven't tested, but those changes don't seem to have fixed this bug.

That patch wasn't supposed to pass because it's for drupal 6 (I should have reverted ticket status to "needs work" to prevent testing), but the file didn't even get onto the d.o file system for some reason

This fix means that theme functions and theme function overrides can not (and should not) define default values for arguments in their signatures. This conflicts with common practice in Drupal 6 and therefore isn't recommended there. It might be a good solution for Drupal 7 though.

For example, the item_list theme hook is defined in the theme registry in drupal_common_theme(). as;

'item_list' => array(
  'arguments' => array('items' => array(), 'title' => NULL, 'type' => 'ul', 'attributes' => array()),
),

It's theme function has the signature;
theme_item_list($items = array(), $title = NULL, $type = 'ul', $attributes = array())

A theme called custom can override it by defining a function called custom_item_list(), perhaps with a different signature that has different default values for it's arguments, such as

custom_item_list($items = array(), $title = NULL, $type = 'ol', $attributes = array('class' => 'item-list'))

Without this patch the defaults of the overridden function (ol and the css class item-list) are respected when theme('item_list') is called without those arguments. This is an (undocumented?) feature of Drupal. However the default values in the hook definition in the theme registry are never used (for function theme hooks – they are used for template tpl.php theme hooks).

With this patch, the default argument values in the signature of the overriding function (or the original, theme_item_list()) are never used, thus breaking the undocumented feature and changing how the theme API functions, albeit in order to adhere to documentation.

With or without this patch there are default values for arguments somewhere that are never used, which is redundant and confusing. The possible solutions are to either;

1. correct documentation (a different patch) so that it is clear that argument's default values are ignored for function theme hooks, and remove default values for arguments in function theme hook definitions.
2. adhere to documentation by extending this patch to also remove default values for arguments in theme function signatures.

Since adhering to documentation requires a change to the theme API it will have significant ramifications for contrib and is not recommended for drupal 6, but is probably the best solution for Drupal 7.

The first option is a documentation bug fix and is recommended for Drupal 6.

Pending consensus we should fork this issue into the separate solutions and patches for each of Drupal 6 and 7.

Some grepping (grep -r "function theme_.\+\(.\+=.\+\)" .) reveals 29 theme function signatures in drupal HEAD that define default values for arguments. 20 of those are not NULL (grep -r "function theme_.\+\(.\+=.\+\)" . | grep -v "function theme_.\+\(.\+= NULL.\+\)").

In Drupal 6 these numbers are 28 and 17, but that only counts drupal 6 core, not contrib.

There is no code to review, but the suggestion solution does need review.

wtf!

Oh, the patch was actually there, the URL's are borked though (It's here). This might be because of #310139: drupal_query_string_encode() should not call drupal_urlencode() which was recently released in 6.14. This one has no space chars so should work.

That was a CVS patch. It probably needs to be a git patch now.