element key didn't use parents element and override default item with field collection

Here is a quick patch which seems to solve the issue but needs review

In fact, it won't works too if deleting the last field collection item.

Indeed, ajax submission will call other form element handlers and country validate will repopulated form state with element key as it couldn't know which action is done by triggering element. As a result, new element replacing old element deleted will inherit values from it as element_key generated would be the same (using form tree).

As it seems that $items provided into hook_field_widget_form seems to be already updated even through ajax (didn't investigate on it), it doesn't need to store values in form state.

Here is an another quick and dirty patch which just remove element_key storage and simply use items values provided if any, or default from country.

But as say, it needs review as it work in my use case but have any idea for others...

Here is steps to reproduce (tested with minimal profile 7.22, last stable ctools, entity, addressfield(rc3) and field collection (beta5):

- attach a field collection with unlimited values to a content type with the widget "embedded" (and check hidden blank items but should doesn't matter)

- then edit the field collection and attach to it an addressfield. This field instance use the default widget "Dynamic address form" with settings : select one available default country, check "Address form (country-specific)" + "Hide the country when only one is available" and delta value is unique (only one value).

- then create a new content, enter some values into the addressfield of the field collection. Click on the multiple element delta form button "Add another item", the second delta item would be a copy of the first (!).
In reverse process, some behaviours occured : remove the second delta item, then remove the first item. Normally, a new element will be created with default values but in this case, the new first element is populated with values from previous first element delta just previously removed !

Of course, to reproduce it, what is important is to have a multiple value field collection.

As explain above in this use case, what seems to appends is that 'element_key' ignore delta field collection. Instead, it should used form tree to construct this key. But it seems that form API already take care about it ($item in hook_form_widget)...

Could you reproduce the issue ?

- the data is never saved, after pushing the [Save] button!

weird... After submitting the "whole" form (entity host of the field collection), values are saved (i have change above "scenario" with no default country then change cardinality field collection to one but data in both cases are saved...).
how to reproduce it ?

when changing a country, the form is not refreshed with the settings of that country

weird, it's work for me...for example switching from france to united state and reverse...

when changing a country, the already filled other subfields are not cleared.

true ! I confess that i didn't take a look why ! Anyway, with or without patch #2, the behaviour is the same !

There's not enough information here for me to go on

Ok i will try to explain it better :

1 - currently, an element_key is generated and based on a token which is supposed to be unique and identify an element in the form tree.

2 - it is only used to retrieve previous values stored

In fact,

1 - token is wrong because it don't care about all the form tree.

So if the addressfield is attach to a field collection, we could have a conflict of this identifier ('element_key') if the field collection cardinality attach to an another entity is greater than one (don't confused with addressfield cardinality).

Here a quick example with a field collection entity having an addressfield and attach to a node entit host :

-> the key is build with : entity_type + bundle + fieldname + language + delta.

When adding the first item cardinality of the field collection which contains an addressfield, the key will be :

- field_collection_item|FC_FIELDNAME|ADDRESS_FIELDNAME|und|0

if we add an another item of the field collection, the new item key of the addressfield will be :

- field_collection_item|FC_FIELDNAME|ADDRESS_FIELDNAME|und|0

And the element_key are the same for the both addressfield even if they are attach to another delta field collection item (in both field collection item, there is only one addressfield item, so both delta are 0).

So to have a unique identifier in this case, it must be something like :

- node|NODE_TYPE|FC_FIELDNAME|und|0|field_collection_item|field_collection_item|FC_FIELDNAME|ADDRESS_FIELDNAME|und|0.

for the first item and

- node|NODE_TYPE|FC_FIELDNAME|und|1|field_collection_item|field_collection_item|FC_FIELDNAME|ADDRESS_FIELDNAME|und|0.

for the second delta item cardinality of the field collection.

Note the only difference is the field collection delta (0 and 1). Previous key is prefixed with the whole form tree.

but when I look at it, I see that it's keying values based on delta... so I don't know why it wouldn't work in an unlimited value scenario.

So hope it could help to understand why delta isn't enough in this case...

As a consequence, when adding the second field collection item, it will try to retrieve values from the element_key generated : yes, it already exists ! (the first created).

When you removed all field collection item, a new default item will be created. As the key of this new default element is the same as the previous and previous storage in form_state haven't been cleared, it will retrieve values from it...(rather than having an empty field).

2 - it seems that it is useless to use a storage in form_state withthe 'element_key' because field api already set the appropriate value to the $items function parameters.

based on johnv's review, it seems this patch introduces some regressions

As say in #5, i would like to reproduce these regressions but i can't achieve it...
Anyone could reproduce it and explains me how to reproduce it step by step ?

Anyone have the same issue ?

With the new beta4 release, reroll this patch...

Thanks for review

Hi tien.xuan.vo,

In fact, I think that your patch does the tricks because country field have always a default value and so is not empty.
If we could have an empty country field (empty default value), this bug still occured. Because element_key identifier is incorrect. In fact, we never enter into the second or third statement when adding a new element of the field collection which contains an addressfield :

if (!empty($items[$delta]['country'])) {
  // Use the selected value if available.
  //==> ALWAYS enter here, even if for new element because they have a default value
  $address = $items[$delta];
}
//so NEVER enter into these two statements
elseif (!empty($form_state['addressfield'][$element_key])) {
  // Else use the old value before changed.
  $address = $form_state['addressfield'][$element_key];
}
else {
  // Otherwise use the instance default.
  $address = (array) $instance['default_value'][0];
}

As say previously, seems that field API already construct an item with default value or values from previously input. That why we didn't need to use this element_key (furthermore buggy !).

I think this issue isn't related to element_key

if you don't change statement order, element_key is used instead and re-used previous values because it isn't correct. As you are changing order, you don't use element_key.

Have you tried patch in #7 ? It is not solving your issue ?

Indeed, I fear that patch #9 doesn't really solve the real issue but just hide it even if it works because country could never be empty. That's why I would prefer keep patch #7 instead.

Please correct me if I'm wrong somewhere.

thanks