Problem/Motivation

Symptom

If you have a field widget that can display different form elements for different field items of the same field and you use that widget for a multi-value field inside of a paragraphs widget, re-ordering of those field items and subsequently adding another field item will lead to unexpected and broken results.

A use-case for showing different form elements for different field items is to have a select or radios form element that differentiates different types or configurations of the same field type and shows the respective form elements for that type or configuration based on the selection of the differentiating field. A concrete, prominent example of this behavior is the address widget, which allows selecting a country and then shows the rest of the form elements according to that country's address format. Due to how the address widget is implemented*, this bug is not in fact exposed with it, but it serves to prove that the use-case of having a dynamic set of form elements is valid, even if it is a bit advanced.

To serve as an example use-case the issue branch provides an "SVG" widget that allows configuring a shape and then saves an SVG string that produces the configured shape. It allows creating circles with a configurable radius and squares with a configurable side length. Thus, a "Radius" form element is shown if the "Circle" shape is selected and a "Side length" form element is shown if the "Square" shape is selected.

An animation showing the two states of the SVG widget and the changing between them

What happens now due to this bug is that when you have a multiple-value field with this SVG widget and the first item is a circle and the second is a square, if you switch the two using drag-and-drop and then click on "Add another item" the radius and side length, respectively will not be mapped correctly and the widget will show a circle with a side length and a square with a radius. Doing the same thing outside of the paragraphs widget works fine, so this is not an issue with the widget itself.

An animation showing the described bug with the SVG widget inside of a paragraphs widget

Cause

To understand what is different when doing reorder-then-add with the paragraphs widget vs. without and also why this bug only surfaces with widgets with dynamic sets of form elements, first a detailed description of what happens in the reorder-then-add flow in Drupal without the paragraphs widget:

  1. We start out with two field items: item A at delta 0 with weight -2 at the top and item B at delta 1 with weight -1 at the bottom
  2. We drag item B to be in front of item A. Now item A has weight -1 and item B has weight -2. Nothing has been sent to the server yet.
  3. We now press the Add another item button. The browser submits the item values and the weights for the respective deltas.
  4. The Add another item button limits validation to the respective field, but that means that field's values will be validated and submitted. For that WidgetBase::extractFormValues() will reorder the values according to the weight and update the field items on the entity being validated. However, $this->entity in the form is not updated because validation happens on a clone of that (see EntityForm::buildEntity())
  5. Assuming there were no validation errors, WidgetBase::addMoreSubmit() will mark the form for rebuilding
  6. The form will then be rebuilt. Because the entity was not updated, the form will be built by WidgetBase::formMultipleElements() in the same way as initially, with item A at delta 0 with weight -2 and item B at delta 1 with weight -1*
  7. During form processing, the input values will be processed (again) so that item values for delta 0 - which was item A - will be applied to delta 0 - which is still item A, so that's good - and in particular the weight for delta 0 will be set to -1. Similarly, item values for delta 1 and weight -2 will be applied to delta 1, which is item B. At this point the form values of all form elements are as expected, but item A still comes before item B in the actual form structure (because it is delta 0, which comes before delta 1).
  8. When the multiple value form is actually rendered template_preprocess_field_multiple_value_form() will order the table rows to be rendered by the values of the weight elements, so that item B's row will in fact be rendered before item A's row.
  9. The response is sent to the browser where it replaces the previous multiple value form and the two items are exactly as they were before clicking the Add another item. (Of course, now there is an additional row, which is the entire point of the Add another item button, but that is immaterial to this bug.)

Given the above it is possible to understand why the underlying problem of this bug is that paragraphs modifies the paragraph entity during validation and submission, because both ParagraphsWidget::elementValidate() and ParagraphsWidget::massageFormValues() operate on the paragraphs entity ($widget_state['paragraphs'][$item['_original_delta']]['entity']) directly and not on a clone. (Note that ParagraphsWidget::massageFormValues() is called during both validation and submission.)* The complete flow with a field inside of a paragraphs widget goes like this:

  1. (Same as above.)
  2. (Same as above.)
  3. (Same as above.)
  4. This time the paragraphs entity that will be used for rendering will be updated by WidgetBase::extractFormValues() during validation and submission*. That means that item B is now delta 0 and item A is now delta 1
  5. (Same as above.)
  6. The form will again be rebuilt. This time however, the form will already be built with updated values, so that item B will be delta 0 with weight -2 and item A will be delta 1 with weight -1. This time item B is already before item A in the form structure at this point.
  7. During form processing, the input values are again processed. Item values for delta 0 - which was item A - will be applied to delta 0 - which is now item B - and item values for delta 1 - which was item B - will be applied to delta 1 - which is now item A. So the field values for the field items will be switched! Because the weight for delta 0 will again be set to -1 and the weight for delta 1 to -2, respectively, the resulting value structure is still correct. The fact that the items were switched can only be seen when looking at the respective #default_value attributes of the form elements. If no item values were changed and only the reordering was done, without the paragraphs widget #default_value and #value will match at this point in the process, whereas with the paragraphs widget they will be switched. But in terms of what will be actually be rendered the state is exactly as above.
  8. template_preprocess_field_multiple_value_form() will again switch the rows during rendering. Because the element values are at the same places as before, the resulting output is also the same. The fact that the items were switched does not surface in any way at this point.
  9. (Same as above.)

Given the above one can now understand why the fact that different field items have different form elements plays an important role here. Step 7 above, where the switching of the items happens, implicitly assumes that it can take the form structure built for item B, apply the values from item A on top and effectively have item A again (and vice-versa). If item A, however, has a form element that item B does not have, that form value cannot possibly be applied to the form structure built for item B. And for any form element that item B has, but item A does not, there will not be a corresponding input value from item A. In effect you then have items which have some values from item A and some from item B.

Steps to reproduce

  1. Check out the issue branch
  2. Enable the Paragraphs test (paragraphs_test) module and create a node type with a paragraphs field
  3. Add a "Text (formatted, long)" (text_long) field to a paragraph type and - for comparison - to the node type directly
  4. Configure the "SVG" widget to be used in both cases. Optionally configure the "Full HTML" text format to be able to easily verify that the widget actually works as expected.
  5. Reorder-then-add on the node field and see that it works as expected. Reorder-then-add on the paragraphs field and see that it breaks as described above.

Proposed resolution

?

Remaining tasks

User interface changes

API changes

Data model changes

Notes

* The reason that the address widget does not expose this bug is that it uses a custom render element which handles the showing and hiding of elements internally. It does not, for example, have a separate form element for the "administrative area" of an address directly in the widget form structure, that is then shown and hidden depending on the value of a separate "country" element in the widget form structure. Back

* Actually the weights at this point will be 0 and 1, not -2 and -1. The effect is the same, though, and using -2 and -1 makes it easier to distinguish the weights from the deltas and also because the Javascript reordering does in fact use -2 and -1, 0 will end up becoming -2 and 1 will end up becoming -1 so pretending that the form building uses -2 and -1, as well, just makes it less confusing in general. Back

* Note that one could argue that from a conceptual perspective it is wrong for paragraphs to populate the entity that will be used for rendering with unvalidated values (as when WidgetBase::extractFormValues() is called they are not yet validated) as core specifically clones the entity prior to validation to ensure that only validated values end up in $this->entity. Arguing that is not the goal of this issue, though. The point of this issue is to fix the concrete bug described here in whichever way is deemed best, which may or may not be cloning the paragraph entities prior to validation and submission. Back

* The entity will in fact be updated 3 times, once by ParagraphsWidget::elementValidate() and twice by ParagraphsWidget::massageFormValues() (once during validation and once during submission). This is immaterial, however, because the values that are set on the entity are taken from the form input each time and that does not change. The resulting values that are set on the entity are, thus, identical each time, so it does not matter, whether that call happens once, three times or ten times. Back

CommentFileSizeAuthor
paragraphs-svg-widget.gif147.68 KBtstoeckler
svg-widget.gif390.3 KBtstoeckler

Issue fork paragraphs-3251383

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tstoeckler created an issue. See original summary.

tstoeckler’s picture

Issue summary: View changes
tstoeckler’s picture

Issue summary: View changes

tstoeckler’s picture

Status: Active » Needs review

Sending the added test for a spin, should fail.

tstoeckler’s picture

Issue summary: View changes
tstoeckler’s picture

The added test failed as expected. Not sure what all the Search API related failures are, assuming for now that they are random failures.

Added another commit with a first stab fix which is simply cloning the paragraph entities before calling extractFormValues(). This is probably not a proper solution at least not in itself, because massageFormValues() actually mutates a bunch of things on that entity, i.e. setting a langcode, calling setNeedsSave() and submitting the behaviors plugins, which presumably assumes that it is being performed on the actual entity, not a clone. Not sure what to do with that, though, at this point, without some feedback on this issue in general.

When adding the clones the test passed directly, but the Ajax form processing of the widget broke. Changing the way the paragraph entity is accessed before rendering the form fixed this, but that is a workaround at best, I didn't dig too deeply into this particular detail (yet), but still interested to see what fails this causes.