Pointed by sun while discussing #626354: Remove #process pattern from number field:
field_default_extract_form_values() expects field values in a specific place in $form_state['values'], so we should set the #parents property to make sure the form resists possible form_alters on its structure.

Along the way, it should also lets us get rid of the $langcode wrapper ($form[$field_name][$langcode]) in the form structure, that was there to reflect the expected structure of form values.

CommentFileSizeAuthor
#3 field_form_parents.patch881 bytesyched
field_form_parents.patch2.19 KByched

Comments

Status: Needs review » Needs work

The last submitted patch failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Not sure whether dropping langcode from the structure is a good idea... wouldn't it prevent the possibility of editing two languages on the same page? (in contrib)

yched’s picture

yched commented
Status: Needs work » Needs review
StatusFileSize
new field_form_parents.patch881 bytes

The current implementation doesn't really allow editing two languages on the same page either, because we still have a single #language property:

$addition[$field['field_name']] = array(
  (...)
  '#language' => $langcode,
  $langcode => $form_element,
);

so we cannot really have another $form[$field['field_name']][$other_langcode] at the same place...

But true, we don't really have to make that change, + it seems to break quite a few other stuff, so let's try with a simpler one.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Hm. This patch looks fine.

However, ideally, I'd like to pass the #parents upfront to all widgets, so they are able to shift around values (*within* the widget's own elements) and base the operations on the passed in #parents.

As mentioned in the other issue, one-element widgets like the number widget would be perfect candidates for that. They only need 1 form element, and therefore such widgets would just have to extend the existing #parents - instead of altering the form structure.

yched’s picture

yched commented
Status: Needs review » Closed (duplicate)

I think this is actually outdated by #641670: Move $form['#field'] meta information into $form_state

yched’s picture

yched commented
Status: Closed (duplicate) » Active

Undupe, I reduced the scope of #641670: Move $form['#field'] meta information into $form_state for now.

I still think the approach in the initial patch over there (automatic extraction of #parents at after_build time, + using the 'parents' in field_default_extract_form_values()) makes sense, but this can be discussed here.

Additionally, other places in core that currently rely on a hardcoded location in $form_state['values'][$field_name][$langcode]: file_field_widget(), locale_field_node_form_update_field_language().

Status: Active » 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.