The content.module provides a function called content_allowed_values() which for certain types of fields fetches the allowed values set, and also calls drupal_alter('content_allowed_values', ...) on the options found. This allows other modules to extend or modify the allowed values of a CCK field.

However, it looks like this function is only used for cck fields which don't have an implementation of hook_content_allowed_values(). The nodereference module is one example. Allowed values returned by hook implementations don't appear to be run through a drupal_alter() preventing other modules from altering them.

I'm not entirely sure of the best way to approach this. One solution is to call drupal_alter() after you call the hook implementation (see one instance in attached patch), but this current implementation ends up calling drupal_alter() twice if content_allowed_values() is used on the line before. This is not my preferred solution.

A better solution might be to move the hook_content_allowed_values call into content_allowed_values() itself and allow it to go through the drupal_alter() before returning. This is probably the tidiest solution, but not being that familiar with the code, I'm afraid of the repercussions this might have.

One final solution might be to call drupal_alter() from within the hook implementation itself, which is probably the easiest to implement, but you're relying on all contributed add-ons to do this - ideally the cck module should do it. Maybe this is the short term solution for 6.x, but not for 7.x?

I don't know if this is also an issue in 6.x-3.x, or even in fields in core, but it is something I'd like to see implemented if possible. I'm more than willing to create a proper patch for this for all instances, but would like to know which approach to take first.

Cheers,
Stella

CommentFileSizeAuthor
#1 780566.patch886 bytesstella
drupal_alter_allowed_values.patch638 bytesstella

Comments

stella’s picture

stella commented
StatusFileSize
new 780566.patch886 bytes

The more I think about it, the more I dislike the first solution. I still prefer the 2nd solution, but it was quicker to put together a patch for the 3rd solution for the hook implementations in cck core (just 2 lines of code!). See attached.

karens’s picture

karens commented
Status: Needs review » Fixed

OK, I like the last option the best. It's simple and shouldn't break anything. I committed it and a similar change to the D7 version of these modules.

markus_petrux’s picture

markus_petrux commented

hook_content_allowed_values_alter() is implemented by i18ncck module (part of i18n suite) to translate allowed values as provided by optionwidgets module.

If this hook is now invoked by other user/node reference modules, i18ncck will attempt to translate these values as well. hmm...

Maybe this new use case for a drupal_alter() could use a different name, or add a new argument so that i18ncck module can be patched to ignore the strings that could come from user/node references.

karens’s picture

karens commented

Hmm, does that mean the fix WILL break things? If so I should revert that change.

markus_petrux’s picture

markus_petrux commented

Look at the bottom of i18ncck.module.

- CCK issue where this new drupal_alter() was discussed/implemented: #531662: Internationalization support
- Issue in the i18n queue: #531660: i18n + CCK
- Also, see Translation Table, who consumes information generated by i18ncck module. Issue: #499668: Possibility to integrate cck fields?

karens’s picture

karens commented
Category: bug » feature
Status: Fixed » Needs work

OK, I think it is best if I revert the change and let someone confirm how to fix this without breaking i18n.

And it's probably more of a feature request than a bug report.

markus_petrux’s picture

markus_petrux commented

One possible approach could be to use hook_nodereference_allowed_values_alter() and hook_userreference_allowed_values_alter()

That way current behavior is not affected, while still offering the new feature.

yched’s picture

yched commented

Hm. Do we really want to support *extending* the list of referenceable nodes for a given noderef field ?

danielb’s picture

danielb commented

I am also having trouble with this function. Apparently the only way to resolve key|value pairs properly is to use hook_content_allowed_values.... but then when you try to submit the node form it does some crazy validation thing telling you that you've got an 'illegal value'. Well I don't use 'allowed values' at all, so this is a pain. So I have to do hacky things like if ($_POST['op'] != t('Save') && $_POST['op'] != t('Delete')) around my code to prevent the validation errors upon submission, but those aren't very reliable methods as someone could form_alter the buttons on the form.
So to me it sounds like this function is being used for more things than it should. You want one function for allowed values, and another function for rewording the key|value stuff.

kenorb’s picture

kenorb commented
Status: Needs work » Closed (outdated)

Drupal 6 is no longer officially supported. If you think this issue is still relevant for 8.x, feel free to re-open.