Illegal choice log entry from unselected radio buttons field [#356208]

If you have a radio buttons field that has no item selected by default, when you submit the form you will get an error saying the field is required. However, an error is also reported in the logs (undesirable). For example:
"Illegal choice in test_radio element."

Note: It is necessary to have radio buttons fields where no item is selected by default because they force the user to make a choice. Otherwise, the user can submit the form without reading and changing the default value.

1. Install Drupal 5.14 and CCK 5.x-1.10. Enable Content, Option Widgets, and Text modules. Create a content type. Within that content type, create a required radio buttons field with two choices (e.g., Yes/No), but set Default Value to N/A.
2. Submit the form without making any changes.
3. See above.
4. See above.

Comments

Comment #1

threexk CreditAttribution: threexk commented 10 January 2009 at 18:57

I hereby offer a 5 USD bounty to anyone who can fix this issue within two months. Payment would be via PayPal.

The money is surely not commensurate with the work, but perhaps will provide some extra motivation to someone else who is thinking of fixing it.

Comment #2

markus_petrux CreditAttribution: markus_petrux commented 10 January 2009 at 20:14

I would say this is not a CCK fault, but a FormAPI issue.

FormAPI performs a series of validations before any other validation handler is executed. One of the things it does is check that radios have an option selected, and generate that message when it doesn't. The thing is that if no option is selected on a radios element, there's no way to know which is the value that should be taken in that case.

In other words, it is FormAPI who is rejecting further processing on an unselected radios element.

That's why CCK adds the N/A option on top of radio options to allow the user to not enter a value when the field is not required. But when the field is required, a valid option should be selected.

In this case it would be much user-friendly to issue an error similar to "field xxx is required", but that's a FormAPI issue because it is the one who performs this validation. There's nothing CCK can do about it, AFAICT.

Comment #3

threexk CreditAttribution: threexk commented 12 January 2009 at 06:29

Thanks very much markus_petrux for explaining why this happens.

I think you're right that it is a Forms API issue: form.inc is checking whether a radio's value is in the list of its options (code below). If not, "illegal choice" is indicated. Since the empty string is not in the list of options, you get "illegal choice". I think ideally Forms API would differentiate the case where value is empty from the case where value is non-empty but not in the list of options, and perform actions accordingly.

The main problem with the current behavior is the error messages that get recorded in the site log. Every time someone forgets to enter a field in a form, you get an error message under Recent Log Entries. If they neglect to enter 20 fields, you get 20 error messages!

I think it would be possible for CCK to somehow work around this. Perhaps CCK could supplement the #options array with an empty value prior to Forms API validation so it passes the legal-choice check? However, if CCK did work around Forms API's behavior I think unselected non-CCK radios and unselected CCK radios would then behave inconsistently.

Any opinion on how this issue should be handled? I think I should probably open an issue on D7, since Forms API is not likely to get changed in D5/D6. Also I think the extraneous log messages are bad enough that CCK should work around the issue, if possible.

       // Add legal choice check if element has #options. Can be skipped, but
       // then you must validate your own element.
      if (isset($elements['#options']) && isset($elements['#value']) && !isset($elements['#DANGEROUS_SKIP_CHECK'])) {
        if ($elements['#type'] == 'select') {
          $options = form_options_flatten($elements['#options']);
        }
        else {
          $options = $elements['#options'];
        }
        if (is_array($elements['#value'])) {
          $value = $elements['#type'] == 'checkboxes' ? array_keys(array_filter($elements['#value'])) : $elements['#value'];
          foreach ($value as $v) {
            if (!isset($options[$v])) {
              form_error($elements, t('An illegal choice has been detected. Please contact the site administrator.'));
              watchdog('form', t('Illegal choice %choice in !name element.', array('%choice' => $v, '!name' => empty($elements['#title']) ? $elements['#parents'][0] : $elements['#title'])), WATCHDOG_ERROR);
            }
          }
        }
        elseif (!isset($options[$elements['#value']])) {
          form_error($elements, t('An illegal choice has been detected. Please contact the site administrator.'));
          watchdog('form', t('Illegal choice %choice in %name element.', array('%choice' => $elements['#value'], '%name' => empty($elements['#title']) ? $elements['#parents'][0] : $elements['#title'])), WATCHDOG_ERROR);
        }
      }