Removed values in multi-value fields reappear

Test-only patch

Trying to debug this but not sure how to solve it. It seems to me the issue is that after clicking "Add another item", or clicking the last Remove button, \Drupal\Core\Form\FormBuilder::retrieveForm will retrieve the form with values from the entity, and merge in values in FormState::$input. The problem is that we are deleting from FormState::$input so there is nothing to merge. We need a way to set the value to something that represents "empty" and handle that when the form is rebuilt, or we need to be updating the entity in form state when we remove values from the form.

Looks like \Drupal\Core\Entity\EntityForm::afterBuild should be responsible for updating the entity in form state, so it might just be a matter of triggering this from \Drupal\Core\Field\WidgetBase::deleteSubmit. I'm not sure the correct way to do this but adding $form_state->getFormObject()->afterBuild($form, $form_state); to the end of \Drupal\Core\Field\WidgetBase::deleteSubmit seems to work, but I'm not sure the form object is always an EntityForm and also not sure if there would be any other side effects. Attaching a patch as a starting point.

This feels major

Discussed with @mstrelan and my initial thoughts are that perhaps \Drupal\Core\Field\WidgetBase::extractFormValues should be handling this? Perhaps an else for the if ($key_exists) that does a $items->setValue()

Not sure #5 would help when we just want to remove one of the values and not all of them. Attached new patch that should fix the MultipleValueWidgetTest test fail.

Adding screencast with patch to proposed resolution

The last submitted patch, 4: 3376692-4.patch, failed testing. View results →

I don't think this is a regression since this is new functionality added in 📌 Allow for deletion of a single value of a multiple value field Fixed , the old way of deleting field items was to clear the field and save the form.

Confirmed the issue in the IS video
Applied patch #6 which solved the issue.

Putting this back to NR because we're not sure if this is the correct approach (see #4)

Here's a solution which does essentially the same as #6 but in a more isolated way.

🤦‍♂️

The last submitted patch, 13: 3376692-13.patch, failed testing. View results →

Looks like Media Library had to workaround this already earlier. Trying to make this more generic to make the tests pass.

The last submitted patch, 15: 3376692-15.patch, failed testing. View results →

Here's a new approach that only updates the field item list in the case of deletion. Let's see what the bot thinks about this.

Thanks for picking this up. I've tested #18 and can confirm it works with the steps to reproduce. I've also tested it with a much more complex widget (extension of date_recur_modular) and didn't find any issues there either.

Per #19 seems to solve the issue.

Also followed the steps in the videos.

I'm using this patch on 9.5.x but and the tests patch are failing, so here I uploaded the same patch from #18 without the tests.

Hiding patches for clarity

@eleonel please make sure to select "Do not test" when uploading a patch for an older version, otherwise we now have to wait for the test run to fail which will set this issue into Needs work.

The last submitted patch, 18: 3376692-18.patch, failed testing. View results →

I'm triaging RTBC issues → . I read the IS and the comments. I didn't find any unanswered questions.

Reading the patch (not a review) I wanted to suggest a change to one of the comment. I have made a new patch.

I also added credit.

+++ b/core/lib/Drupal/Core/Field/WidgetBase.php
@@ -77,6 +76,13 @@ public function form(FieldItemListInterface $items, array &$form, FormStateInter
+    // Remove deleted items from the from the field item list.

Typo: from the

Addressed #27 👍

@lauriii, sorry about that.

Not sure how the status changed. Setting it back to rTBC.

Is it possible to remove that field with link.

Example :

1. Add reference to User in Article (unlimited)
2. Create node with for example 5+ users
3. Create a view of Article, add this reference field and unclick "Display all values in the same row" so you can see 5 + rows of users
4. Optinally add the same field as delta.

And my quesiton is if it is possible also to remove specific user/delta so if we add "edit" link to view it can reach specific delta and only this user will be removed directly with no need to open content and find it manually ????

Very similiar with paragraph here : 3343465 📌 Delete link Needs review which allows you to remove specific paragraph directly with a contextual filter.

The last submitted patch, 28: 3376692-28.patch, failed testing. View results →

Random failed, retriggered but should be back to RTBC

Committed 82b5bc0 and pushed to 11.x. Thanks!
Committed 06b394a and pushed to 10.2.x. Thanks!

There are conflicts with 10.1.x so I did not backport there. I think this is okay in the instance because it gives us a better chance at finding any regressions with contrib that might be caused by this.

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