- Issue created by @kim.pepper
- π¬π§United Kingdom longwave UK
It's not just FieldItemList where we do this:
core/lib/Drupal/Core/Field/FieldItemList.php 272: 'maxMessage' => t('%name: this field cannot hold more than @count values.', ['@count' => $cardinality]), core/lib/Drupal/Core/Field/Plugin/Field/FieldType/EmailItem.php 62: 'maxMessage' => $this->t('%name: the email address can not be longer than @max characters.', [ 63- '%name' => $this->getFieldDefinition()->getLabel(), 64- '@max' => Email::EMAIL_MAX_LENGTH, 65- ]), core/lib/Drupal/Core/Field/Plugin/Field/FieldType/StringItem.php 66: 'maxMessage' => $this->t('%name: may not be longer than @max characters.', ['%name' => $this->getFieldDefinition()->getLabel(), '@max' => $max_length]), core/lib/Drupal/Core/Field/Plugin/Field/FieldType/NumericItemBase.php 101: 'maxMessage' => $this->t('%name: the value may be no greater than %max.', [ 102- '%name' => $label, 103- '%max' => $max, 104- ]), core/modules/telephone/src/Plugin/Field/FieldType/TelephoneItem.php 73: 'maxMessage' => $this->t('%name: the telephone number may not be longer than @max characters.', ['%name' => $this->getFieldDefinition()->getLabel(), '@max' => self::MAX_LENGTH]), core/modules/text/src/Plugin/Field/FieldType/TextItem.php 70: 'maxMessage' => $this->t('%name: the text may not be longer than @max characters.', ['%name' => $this->getFieldDefinition()->getLabel(), '@max' => $max_length]),
- π¬π§United Kingdom longwave UK
Symfony Validator does have a "payload" option for all constraints where you can "attach domain-specific data", I wonder if we should be pushing the field definition/label into here instead.
But there is also the issue that if we remove
t()
/TranslatableMarkup
here then the strings probably won't be picked up by potx for translation, so this is all a bit messy. - π¨πSwitzerland Berdir Switzerland
I think we did those specifically because otherwise we lose important context on those validation messages, the generic validation messages in general are often a pretty big UX problem. That payload thing might be newer than the original implementation, if that could help to make the messages in general better that could be interesting.
- Merge request !7257Deprecate NULL arguments in FormattableMarkup::__construct(). β (Open) created by longwave
- Status changed to Needs work
5 months ago 7:54pm 31 March 2024 - π¬π§United Kingdom longwave UK
Given that we don't allow NULL arguments to be formatted in
FormattableMarkup::placeholderFormat()
I think perhaps we should move the deprecation to the constructor. - π«π·France andypost
Tests showing lots of failures, so it's more theen one place
- π¬π§United Kingdom longwave UK
The payload option doesn't seem to be useful. When the Symfony CountValidator builds the violation it only passes in a fixed set of properties:
$this->context->buildViolation($exactlyOptionEnabled ? $constraint->exactMessage : $constraint->maxMessage) ->setParameter('{{ count }}', $count) ->setParameter('{{ limit }}', $constraint->max) ->setInvalidValue($value) ->setPlural((int) $constraint->max) ->setCode($exactlyOptionEnabled ? Count::NOT_EQUAL_COUNT_ERROR : Count::TOO_MANY_ERROR) ->addViolation();
The message properties are already strings and even if we override that somehow then
buildViolation()
also only takes a string. There is nowhere to add additional data so by the time we are here we need at least partially formatted strings if we want to add data other than count or limit to them.So, the fix is probably not to be passing NULL in the first place, just we weren't catching these before. While we were here we can format plurals correctly too in
FieldItemList::getConstraints()
- "this field cannot hold more than 1 values" was never correct. - π¬π§United Kingdom longwave UK
Regarding potx it has special checks for validation constraint properties that end in "message", added in #1903362: Support for Drupal 8 validation constraint messages β - we can't reuse any of that here, the checks are way too specific.
- π¬π§United Kingdom longwave UK
To save translating messages where possible, maybe we should use the default message in the case that the label is NULL.