Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 UTC on 18 March 2024, to get $100 off your ticket.
Problem/Motivation
When creating an integer field as shown in http://drupal.stackexchange.com/questions/188367/how-to-add-uniquefield-... , the UniqueField constraint throws the following error:
The website encountered an unexpected error. Please try again later. TypeError: Argument 1 passed to Drupal\Core\Form\FormState::setError() must be of the type array, null given, called in /var/www/html/web/core/lib/Drupal/Core/Field/WidgetBase.php on line 454 in Drupal\Core\Form\FormState->setError() (line 1156 of core/lib/Drupal/Core/Form/FormState.php).
I could reproduce this bug on core 8.1.7, 8.1.8, 8.2.0-beta1+33-dev (2016-Aug-14) and 8.3.x-dev (2016-Aug-14).
Steps to reproduce:
- Add a custom constraint to a field type number that uses the NumberWidget. This is what the unique_field module does. (https://www.drupal.org/docs/8/api/entity-validation-api/providing-a-cust...)
- Try to create new node that runs the validation.
- The error indicated is thrown.
Proposed resolution
class NumberWidget extends WidgetBase {
// ...
public function errorElement(array $element, ConstraintViolationInterface $violation, array $form, FormStateInterface $form_state) {
if (!empty($element['value'])) {
return $element['value'];
}
return $element[0]['value'];
}
}
Remaining tasks
Comment | File | Size | Author |
---|---|---|---|
#22 | interdiff-22.txt | 1.77 KB | amateescu |
#22 | 2784015-22.patch | 6.54 KB | amateescu |
#17 | interdiff.txt | 1022 bytes | amateescu |
#17 | 2784015-17.patch | 4.78 KB | amateescu |
#17 | 2784015-17-test-only.patch | 3.05 KB | amateescu |
Comments
Comment #2
Gogowitsch CreditAttribution: Gogowitsch commentedComment #3
jlbellidoI was able to reproduce it against the 8.3.x branch. This error is also happening when you add a custom constraint.
I've added steps to reproduce and updated the summary using the template.
Comment #4
jlbellidoI've tested the proposed solution and it works for me.
I'm attaching a patch based on the proposed solution by @Gogowitsch
Thanks!!
Comment #5
BerdirThanks for the patch. Not sure I fully understand this yet, a test would help to explain in which situation this happens exactly.
Comment #6
krknth CreditAttribution: krknth as a volunteer and at Valuebound commentedI'm able to reproduce this. Here the steps i'v followed as per mentioned in the summary
File : my_module.info.yml
File : my_module.module
File : src/Plugin/Validation/Constraint/CodeUnique.php
I did same steps after applying patch, and it worked for me.
Comment #7
BerdirSee also #2027059: Improve the documentation of WidgetBase::errorElement() for mapping violation property paths to form elements, which talks about a more generic fix and has a test. If that is fixed by this then we could maybe include that and get it in, because this will require a test to be committed.
I also think a fatal error when submitting a form qualifies as a major even though it requires custom code in this example, but #2027059: Improve the documentation of WidgetBase::errorElement() for mapping violation property paths to form elements sounds like it is enough to just have a required number field.
Comment #8
xjmIs this also a duplicate of #2614250: Number widget validation can break AJAX actions?
Comment #10
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedThis will be fixed by #2901943: Content entity form validation does not respect the #limit_validation_errors property from field widgets.
Comment #11
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedI looked into this a bit more after @ibustos's comment from #2901943-15: Content entity form validation does not respect the #limit_validation_errors property from field widgets and it's true that the patch over there does not fix this issue.
This is actually related to #2319719: Widget validation crashes on ItemList violations which didn't fix the problem entirely.
The problem is that
\Drupal\Core\Entity\Entity\EntityFormDisplay::movePropertyPathViolationsRelativeToField()
fails to provide a valid violation property path for widgets that have a custom imeplementation for\Drupal\Core\Field\WidgetInterface::errorElement()
.Here's a test which shows the error. I'm not sure what the right fix should be yet..
Comment #12
andypostComment #14
ibustosGood analysis @amateescu, definitely in the right direction. Looking at that function the problem seems to be that
$violation->getPropertyPath();
does not return the correct property path.That propertyPath property seems to be set at
Drupal\Core\TypedData\Validation\RecursiveContextualValidator::validateNode()
which is a recursive function.I will be looking more into that.
Comment #15
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedI dove into this today and it seems that I was wrong in #11,
\Drupal\Core\Entity\Entity\EntityFormDisplay::movePropertyPathViolationsRelativeToField()
works just fine and the actual problem is two-fold:1. the purpose of
\Drupal\Core\Field\WidgetInterface::errorElement()
is to be called only when a) the widget handles multiple values (deltas) on its own and b) when the calling code already has a specific delta to work with2.
\Drupal\Core\Field\WidgetBaseInterface::flagErrors()
does not split properly violations that are on the field item level from the ones that are on the field item list levelThe only sane solution that I can think of is to make
\Drupal\Core\Field\WidgetBaseInterface::flagErrors()
bypass\Drupal\Core\Field\WidgetInterface::errorElement()
for violations that are on the field item list level, so here's a patch that implements this proposed solution.Note that only the conclusion from #11 was wrong, the test-only patch is correct :)
Comment #17
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedAll the failures in #15 show the sad state of the entity_test module (the fact that all test entity types are installed on every functional test, even when the test itself doesn't use it) :/
Which means that we need to use a field type and a widget that are always available, so I picked the 'integer' field type and the 'number' widget.
Comment #19
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedJust closed a duplicate issue: #2840102: errorElement of Number Widget breaks with multivalue fields
It would be nice to have this fixed in 8.4.1 along with #2901943: Content entity form validation does not respect the #limit_validation_errors property from field widgets, which was already committed. Anyone up for a review? :)
Comment #20
CRZDEV CreditAttribution: CRZDEV at Metadrop commentedPatch 2784015-17.patch #17 seems to solve the problem, thanks!
Comment #21
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedJust closed another duplicate: #2887731: EntityreferenceAutocompleteWidget constraint cannot stop form submission
Comment #22
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedIn the issue mentioned above I found out about #2574095: File upload and entity autocomplete results in fatal and #2569897: Required Long Text With Summary + form rebuild = PHP fatal error, which tried to fix the the
errorElement()
implementation ofEntityReferenceAutocompleteWidget
andTextareaWithSummaryWidget
but didn't address the actual root cause.So I think this issue should also take care of fixing the technical debt introduced by the two issues mentioned above. This interdiff basically reverts the incorrect fixes that were committed over there while still passing their tests.
Comment #23
schrammos CreditAttribution: schrammos commentedPatch #17 fixes the problem of #2614250 Number widget validation can break AJAX actions for me. Applied to D8.4.2
Comment #24
tim_zionandzion CreditAttribution: tim_zionandzion commentedWorks for me as well in resolving the same issue in same version with a paragraph containing a number field on a content type that also contained a mutli taxonomy term field.
Comment #26
aquaphenixI have tested the patch on #22 by amateescu and confirm the patch fixes the problem for me.
Comment #27
amateescu CreditAttribution: amateescu for Pfizer, Inc. commentedComment #28
oknatePatch #22 Fixed a bug where I couldn't remove a paragraph that held number widget, because of fatal error in ajax callback.
Notice: Undefined index: value in Drupal\Core\Field\Plugin\Field\FieldWidget\NumberWidget->errorElement() (line 114 of /Users/oknate/dev/mysite/web/core/lib/Drupal/Core/Field/Plugin/Field/FieldWidget/NumberWidget.php)
Thanks!
Comment #29
alexpottCredited @krknth for explicit steps to reproduce, @Gogowitsch for reporting the issue, @ibustos for review comments,
Comment #30
alexpottCommitted 3b951a0 and pushed to 8.6.x. Thanks!
Leaving this at rtbc against 8.5.x since I think we should consider this for backport to 8.5.0 beta since this bug seems have affected lots of projects.
Comment #33
catchCherry-picked to 8.5.x, agreed on the backport.