to reproduce:

1. fresh install of D7 HEAD standard profile
2. go to #overlay=admin/structure/types
3. follow link to edit article fields --> #overlay=admin/structure/types/manage/article/fields
4. add a new field, name doesn't matter, type doesn't seem to matter (i tested file, image, text, boolean)

error messages:

    *  Notice: Undefined index: _add_new_field in _field_ui_field_overview_form_validate_add_new()  (line 518 of /var/www/drupal/core/7/content/modules/field_ui/field_ui.admin.inc).
    * Warning: array_shift() expects parameter 1 to be array, null given in _form_set_value() (line 2033 of /var/www/drupal/core/7/content/includes/form.inc).

we can't release D7 with basic functionality throwing errors, so setting as critical.

CommentFileSizeAuthor
#11 field_ui.patch1.85 KBpflame
#5 field.ui_.patch804 bytesAnonymous (not verified)

Comments

Anonymous’s picture

Anonymous (not verified) commented
Assigned: Unassigned »

i'm working on a test that will fail (because of the notices).

Anonymous’s picture

Anonymous (not verified) commented

seems like we have a test for creating fields, FieldUITestCase::createField(), that should fail. now trying to figure out why that doesn't, but real life does.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Priority: Critical » Normal

Unable to reproduce, not in an old install (did get an exception thrown without stack trace error there though... i really thought we got rid of them ?!), not in a fresh install, not in a fresh install with all core modules and a dozen contrib modules.

Not critical unless someone else can reproduce imho...

Anonymous’s picture

Anonymous (not verified) commented
Priority: Normal » Critical

@berdir thanks for trying to reproduce. putting back to critical until we get another "can't reproduce".

i just double checked that i've got a fresh cvs checkout, fresh install, etc, and i can still reproduce.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Active
StatusFileSize
new field.ui_.patch804 bytes

ok, just managed to avoid poking my own eyes out to find this.

the field_ui tests for this prepend 'field_' to the test field names, which means that test avoid the real-life borked code in _field_ui_field_overview_form_validate_add_new():

      // Add the 'field_' prefix.
      if (substr($field_name, 0, 6) != 'field_') {
        $field_name = 'field_' . $field_name;
        form_set_value($form['_add_new_field']['field_name'], $field_name, $form_state);
      }

the problem with this code is that '_add_new_field' doesn't live at the top level of $form, but in $form['table'].

so the naive fix is to change 

        form_set_value($form['_add_new_field']['field_name'], $field_name, $form_state);

to 

        form_set_value($form['table']['_add_new_field']['field_name'], $field_name, $form_state);

patch attached, but this needs some field people to look and see if there's a deeper issue, and more work to fix the tests.

Anonymous’s picture

Anonymous (not verified) commented
Status: Active » Needs review
chx’s picture

chx commented
Status: Active » Needs review

I can confirm the issue exists and the patch works and i doubt there is anything deeper going on isnt this the result of a recent patch , extend field ui from contrib?

Stevel’s picture

Stevel commented

Looks like this fixes the issue indeed. getting notices would depend on the PHP configuratino I think.

pflame’s picture

pflame commented
Status: Needs review » Reviewed & tested by the community

I am able to reproduce this bug with latest Drupal HEAD codebase. And the patch worked fine.

Anonymous’s picture

Anonymous (not verified) commented
Status: Reviewed & tested by the community » Needs work

woah, we need to fix the tests so they exercise this code.

pflame’s picture

pflame commented
StatusFileSize
new field_ui.patch1.85 KB

In the test case form element field_name value assigned as 'field_'. strtolower($this->randomName(8));
But when we submit the manage fields form the field_name value does not contain the 'field_' prefix.
The bellow code is trying to add the prefix as 'field_'

      // Add the 'field_' prefix.
      if (substr($field_name, 0, 6) != 'field_') {
        $field_name = 'field_' . $field_name;
        form_set_value($form['_add_new_field']['field_name'], $field_name, $form_state);
      }

but the code raising notice and warning because the form_set_value statement should be

form_set_value($form['table']['_add_new_field']['field_name'], $field_name, $form_state);

I fixed the test case and attached the patch.

marcingy’s picture

marcingy commented
Status: Needs work » Needs review
Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Reviewed & tested by the community

works for me, simple patch, RTBC.

dhthwy’s picture

dhthwy commented

+1 RTBC.

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

zserno’s picture

zserno commented
Status: Fixed » Reviewed & tested by the community

I'm sorry to say, but I can't see this patch in my recent checkout neither here: http://drupal.org/project/cvs/3060, so setting it back to RTBC.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Ok, committed it this time. :) Thanks!

yched’s picture

yched commented

Aw - sorry about the bug, I did rely on test passes.

Anonymous’s picture

Anonymous (not verified) commented

yched: what, your code is not always completely bug free?!? ;-)

yched’s picture

yched commented

re justinrandell : Of course it is. Must've been my cat walking on the keyboard.

Status: Fixed » Closed (fixed)

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