TextareaWidget::errorElement() does not consult violation property path.

I have just ran across this issue and I do not think this is a good solution. It fixes the TextAreaWidget and descendants but not any other type of field will be similar (Single widget which has multiple fields).

From the example above.

$this->context->buildViolation($constraint->someMessage)
          ->atPath((string) $delta . '.value')
          ->setInvalidValue($text)
          ->addViolation();

When creating the violation as above using the `::AtPath('value')`, the WidgetBase::flagErrors() is then splitting all the violations into 2 groups. A delta list and an item list. The delta list is basically, the violation has the path of `body.0.value` and everything else goes into the item list.

Since this is a delta list it uses the delta and anything after the delta is discarded (In this case the `value` is not used). In reality it should use the rest of the path after the delta to find the element that the error should be added to.

This re-roll is to make the patch from #9 apply cleanly to 10.3.x. It involved adding some more use statements and adjusting line numbers from the previous patch.

idimopoulos → made their first commit to this issue’s fork.

You are correct @gordon. It seems to be a more generic issue of the form errors method. I am going to work on it for a bit and try to write a test on this. Setting this to needs review only for an initial test run.

The Needs Review Queue Bot → tested this issue.

While you are making the above changes, we recommend that you convert this patch to a merge request → . Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)

Hiding the patch files and setting this to be a bug in the field system instead.

Mainly, from what I could see, many widgets that represents element with a single property (like the mainProperty is just a single one like value or target_id), they often tend to return that property as part of the \Drupal\Core\Field\WidgetBase::errorElement. This is why it does work for some fields. For others, that simply return the element, more specific targeting is required.
I have created a patch that tries to descend further into the element in order to try and pinpoint the exact property to add the violation to, according to the property path. It also further checks whether the error element is already returning the appropriate target.

Note, that for this to work properly, a violation constraint needs to be added properly as well, i.e. using the ::addPropertyConstraints instead of the generic addConstraint.

The initial MR seemed to have broken multiple tests which revealed the second point about the ::errorElement already returning the correct property from widgets.

I am still not sure where to write tests for this though :/ any ideas? because it would be easier to write it as a FunctionalTest using the inline_form_error which adds links that can be clicked (that reveals the problem) but seems too much for a widget base. But a PhpUnit test would require quite some mocking (?).

I am adding the label needs tests.

May need a test module that does what the steps mention, or if an existing module comes close and can be extended.

I will try to create a test but I cannot do it this week (sorry).
What I have found is that there is a generic error with the inline_form_errors and its attempt to create links to the fields.

The problem is when the constraint is added to the element rather than a property of the element e.g. (example from our code)

function ..._entity_bundle_field_info_alter(array &$fields, EntityTypeInterface $entity_type, string $bundle): void {
.
.  
    /** @var \Drupal\field\Entity\FieldConfig[] $fields */
      $fields['short_id']->addPropertyConstraints('value', [
        'Regex' => ['pattern' => '/^[a-z0-9-]{4,26}$/'],
      ]);
      $fields['short_id']->addConstraint('UniqueShortIdInsensitive');
    }

In the above, we are adding one property constraint, and one constraint on the whole element.
You may consider a case where you want all values of a multi cardinality field to be different even though above, we are trying to keep a unique value in the database.
Disclaimer: You may claim that the addConstraint method should be instead addPropertyConstraint. You are correct, it might be wrong design. However, it does not solve the underlying issue, as a constraint that is needed to be set on the element level is easily to imagine as I mentioned above.

So, there are 2 issues with the inline_form_errors. The first one was fixed (for us, though it needs tests and review) with my patch above. There was a problem that WidgetBase::flagErrors did not descend more than the delta, thus the ID constructed for the inline_form_errors, was edit-my-field-0 instead of edit-my-field-0-value. By default, only the input child is receiving an ID, thus the link from inline_form_errors which would point to href="#edit-my-field-0" would not lead anywhere. As mentioned, now it properly descends.

However, the main problem here, is with the element level constraints. By default, form-element.html.twig does not add an ID on the element level. So, the element level link from inline_form_errors will not point anywhere with the href="#edit-my-field".
The problem further expands because, while I did workaround it a bit on my project by trying to force an ID to the elements, there are some cases where this would create conflicts. For example, a select filter and a field with the same name, would have to produce the same ID, so I cannot force it for all form elements in any case without considering what else is in the page. So I have built some exceptions in our code.
But even further, \Drupal\inline_form_errors\FormErrorHandler::displayErrorMessages seems to detect the wrong ID anyway, so if I set it to the element level, it will still add the fragment edit-my-field even if I add a custom ID, so the above cannot be fixed with the \Drupal\Component\Utility\Html::getUniqueId as it might make it edit-my-field--2 but the fragment will not change accordingly.
The complexity (not complaining about the complexity) of the element -> widget -> delta -> property of the form API does not seem to be respected from the inline_form_errors.

An easy way to reproduce this (I have to write the test with the inline_form_errors module and a custom constraint) is to simply create a constraint, add it to an element with the field_info_alter hook, fail the constraint, check the link from inline_form_errors, and confirm that the fragment does not match the ID.

MR !12521 seems to solve things partially.

Maybe the inline_form_error deserves its own issue, not sure...
But many errors are attached to the field itself (not properties) and in such cases inline_form_error links points to anchors that do not exist in the form. This happens on single element fields.

To me it's very strange and suspicious why field-multiple-value-form.html.twig does not print the outer div when not multiple, along with attributes and #id of the element. So something like
{% if multiple %}
...
{% else %}

{% endif %}

This may be a decent way to solve that issue.