Steps to reproduce:

1) Install and enable Webform 7.x-3.17 and Options Element 7.x-1.7.
2) Create a webform adding a 'Select options' component and checking ON 'Customize keys (Advanced)' and selecting any of the options under 'Load a pre-built option list' under the component settings.
3) Save the component.
4) Edit the component. Notice the 'Option Settings' are disabled.
5) Save the component.

Notice: Undefined index: options_text in _form_options_validate() (line 200 of /.../options_element/options_element.inc).

See screenshot attached.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Liam Morland’s picture

Fix attached.

Liam Morland’s picture

Status: Active » Needs review
quicksketch’s picture

Thanks Liam! I'll review this and your other patches when I get a chance. Much appreciated!

quicksketch’s picture

Status: Needs review » Needs work

Hm, this patch reveals deeper problems with the #input form element type. It doesn't look like we can use the #disabled property and still control any of the other properties of the form. While this patch makes it so that the notice is suppressed, you still can't edit the default value of any options element that has had its #disable property set to TRUE. This is because FormAPI calls the _form_builder_handle_input_element() function (which in turn calls the #value_callback property) just before the #process functions. In your other patch at #1513894: Pre-built options lists do not work with Options Element, you set #disabled to the proper FALSE, making it so the field would be displayed properly. However as far as FAPI is concerned, the element is disabled because the #value_callback is called before the #process callback (from form.inc):

  // Handle input elements.
  if (!empty($element['#input'])) {
    _form_builder_handle_input_element($form_id, $element, $form_state);
  }
  // Allow for elements to expand to multiple elements, e.g., radios,
  // checkboxes and files.
  if (isset($element['#process']) && !$element['#processed']) {
    foreach ($element['#process'] as $process) {
      $element = $process($element, $form_state, $form_state['complete form']);
    }
    $element['#processed'] = TRUE;
  }

Inside of _form_builder_handle_input_element() is where #value_callback's are called, and thus the handling of setting the "options_text" and "default_value" properties.

To fix this problem, we need to stop using #disabled I think. Perhaps something like #disabled_options would be more appropriate? Since we're just locking the option-entry, not other properties in the field (like the default_value).

Liam Morland’s picture

Status: Needs work » Needs review
FileSize
812 bytes

I didn't follow all of what you wrote above, but I had another look at it. In _form_options_validate() when $element['#value']['options_text'] is not set (resulting in the error), it is because $element['#value']['options'] already an array of the options. So, there is no need to convert the options_text into the options. The attached patch checks for this and only attempts to convert options_text into the options array if it exists. Otherwise, it uses the existing options array.

quicksketch’s picture

Hi Liam,

What I meant above is that this problem wouldn't happen if the #value_callback were being executed properly. Since the element has its #disabled property still set when the #value_callback is executed, the whole thing is skipped and options_text is not set.

Here's the line in _form_builder_handle_input() that causes the problem:

  $process_input = empty($element['#disabled']) && ($form_state['programmed'] || ($form_state['process_input'] && (!isset($element['#access']) || $element['#access'])));

This ends up calculating to FALSE because $element['#disabled'] is TRUE. Because it is FALSE, the values are not set properly. Making it so that options_text is not populated and (more importantly) you can't set a default value on a field that has been disabled. Your patch only solves the PHP notice. There is a larger problem here from the same root cause that prevents using default values also.

Liam Morland’s picture

But why does options_text need to be set? All that it is used for in _form_options_validate() is for generating an array of the options. That array already exists.

quicksketch’s picture

The problem isn't only with with options_text. If the #value_callback is not executed, the 'default_value' is not set either, which is a different problem but it comes from the same cause.

quicksketch’s picture

To replicate (with or without the patch):

- Create a new Webform select component. Use a pre-built list like Days of the Week. Set the default to "Monday".
- Save the component.
- Edit the component again, and try to set the default to "Tuesday". Save.
- The default is still "Monday".

This problem (and "options_text" being empty) are both because the #value_callback is not being passed any of the $_POST values. Instead Drupal thinks that because the element has #disabled = TRUE, it should not listen to any of the $_POST values and uses the defaults that were set on the element.

Liam Morland’s picture

Yes, I see the problem now. So where is #disabled set to TRUE?

quicksketch’s picture

So where is #disabled set to TRUE?

That's being set by Webform, based on if a pre-built list is being used. So we'll need to switch *both* Options Element and Webform from using #disabled to a new property that only locks the options-portion of things.

quicksketch’s picture

This set of patches both to Webform and Options Element is the right way to go I think. Though this introduces some inconsistency between the term "disabled" (used by the JS) and "readonly". However considering the element isn't truly disabled on the backend (or really even on the front-end if users know how to use Firebug), I think readonly is a better term. The readonly attribute is only used to avoid confusion, it's not needed for security or data integrity purposes.

Liam Morland’s picture

Status: Needs review » Reviewed & tested by the community

Works great!

quicksketch’s picture

Title: Notice: Undefined index: options_text in _form_options_validate() » Notice: Undefined index: options_text in _form_options_validate() (Default values in Options Elements not editable)
Status: Reviewed & tested by the community » Fixed

Sweet. Committed to both Webform (3.x branches) and Options Element (1.x branches).

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary: View changes

Added attention to screenshot attached.