Which causes warnings if tid isn't set.

CommentFileSizeAuthor
#1 field_is_empty.patch531 bytescatch

Comments

catch’s picture

Status: Active » Needs review
StatusFileSize
new531 bytes

Actually it needs an isset first, then empty().

Found this when trying to do a custom widget which uses built in field API multiple values handling. Haven't worked through all the issues yet (validation is broken due to lack of weight field, issue coming for that soon), but this was one of them.

yched’s picture

Is this still needed ?

catch’s picture

Status: Needs review » Closed (won't fix)
Aurochs’s picture

I still have it on saving nodes with drupal 7.9

Notice: Undefined index: tid в функции taxonomy_field_is_empty() (строка 1394 в файле /var/www/gamepart.org.ua/modules/taxonomy/taxonomy.module).
Notice: Undefined index: tid в функции taxonomy_field_is_empty() (строка 1394 в файле /var/www/gamepart.org.ua/modules/taxonomy/taxonomy.module).
Notice: Undefined index: tid в функции taxonomy_field_is_empty() (строка 1394 в файле /var/www/gamepart.org.ua/modules/taxonomy/taxonomy.module).
Notice: Undefined index: tid в функции taxonomy_field_is_empty() (строка 1394 в файле /var/www/gamepart.org.ua/modules/taxonomy/taxonomy.module).
Notice: Undefined index: tid в функции taxonomy_field_is_empty() (строка 1394 в файле /var/www/gamepart.org.ua/modules/taxonomy/taxonomy.module).
Notice: Undefined index: tid в функции taxonomy_field_is_empty() (строка 1394 в файле /var/www/gamepart.org.ua/modules/taxonomy/taxonomy.module).

I have just moved from D6

guypaddock’s picture

Title: taxonomy_field_is_empty() checking for empty() instead of !empty() » taxonomy_field_is_empty() does not check if tid is set before checking for emptiness
Status: Closed (won't fix) » Needs review

This issue persists in 7.12. I am seeing this when using the Hierarchical Select module with the patch from #1293166. I wrote that patch and am finding that the workaround to this issue is not intuitive.

The real issue is that, anytime a module puts elements under a taxonomy field in the form, this issue will arise because those elements' values will be copied into the form state as the values of the taxonomy field itself.

Here's an example:

  • $form['field_a_tag'] is a taxonomy form element.
  • $form['field_a_tag']['hierarchical_select'] is a Hierarchical Select element inside the taxonomy form element.
  • $form['field_a_tag']['hierarchical_select']['create_new_item'] is a mark-up container that Hierarchical Select uses for displaying a miniature form for users to add new terms.

Assume that we're modifying a taxonomy field in a user profile. When the form is submitted, the form state looks like this after form_state_values_clean() gets invoked:

array(
  ...
  'values' => array(
    'field_a_tag' => array(
      ...
      'und' => array(
        'hierarchical_select' => array(
          'create_new_item' => NULL,
        ),
      ),
      ...
    ),
  ),
  ...
);

Consequently, when taxonomy_field_is_empty() gets called on the "items" of the taxonomy field, the function gets passed hierarchical_select as an item. It is an array, so the check for !is_array() passes. Since it doesn't contain an element with the key of tid, though, a PHP notice is raised.

There are only three possible fixes I can see to this issue from the module side of things:

  1. Use #tree and/or #parents so that the extra form controls don't appear underneath the taxonomy field. This makes code that needs to reference these fields needlessly more complicated, though.
  2. Somehow pull the extra elements out of the $form_state before they are encountered by the code that tries to use them like items. In my experience, this only works if the "extra elements" don't contain buttons. Otherwise, the logic in form_state_values_clean() that uses unset() to remove button values from the form state will actually restore the parent array.
  3. Somehow force the extra form elements to have a value of an array that contains a tid.

This really does appear to need a fix in core.

guypaddock’s picture

Upon further investigation, it looks like a workaround that does work is to set #tree to FALSE on buttons within the affected containing element. At least for Hierarchical Select, the real issue is that form_state_values_clean() puts an array back into the form state under the taxonomy field because it unsets the button values, which were never there in the first place.

In other words, the array that Hierarchical Select maintains in the form state has been cleared by the time that form_state_values_clean() is called, but because the containing element has buttons, form_state_values_clean() goes to unset their values from the form state and inadvertently restores the array in the form state that would contain those values.

So... before the call to form_state_values_clean(), the form state looks like this:

array(
  ...
  'values' => array(
    'field_a_tag' => array(
      ...
      'und' => array(
        'hierarchical_select' => array(
        ),
      ),
      ...
    ),
  ),
  ...
);

After the call, it looks like this:

array(
  ...
  'values' => array(
    'field_a_tag' => array(
      ...
      'und' => array(
        'hierarchical_select' => array(
          'create_new_item' => NULL,
        ),
      ),
      ...
    ),
  ),
  ...
);

Because, in PHP, if you do:

  unset($form_state['values']['field_a_tag']['und']['hierarchichal_select']['create_new_item']['Create']);

...and $form_state['values']['field_a_tag']['und']['hierarchichal_select']['create_new_item'] is not set, PHP will create it.

So, you could look at it as a bug in form_state_values_clean() because it should check to see if the thing it's un-setting actually exists. But, taxonomy_field_is_empty() cannot escape responsibility for being another villain here.

Status: Needs review » Closed (outdated)

Automatically closed because Drupal 7 security and bugfix support has ended as of 5 January 2025. If the issue verifiably applies to later versions, please reopen with details and update the version.