EntityAutocomplete form element allows multiple values when #tags is FALSE

Our QA team reported this bug. I've attached a video I created using the EntityReferenceAutocompleteWidgetTest.php using Drupal 8.8 alpha.

This is still an issue. There should be some sort of javascript preventing more than the allowed cardinality from being entered. Or perhaps validation on the backend that returns an error. Nothing terrible happens, but it allows you do something odd.

Assigning to myself to have a dig in over the upcoming weekend. If others want to jump ahead feel free.

EDIT - Ah sorry ignore this patch - Had a bunch of local changes I was playing with for a talk.

Looking at the code it looks like this is desired behaviour - But I think I agree that it should fail validation instead of just grab the last value. Thats too confusing for editors IMO.

I think we need a test for this too if everyone agrees it should work this way.

1. +++ b/core/lib/Drupal/Core/Entity/Element/EntityAutocomplete.php
   @@ -240,8 +240,14 @@ public static function validateEntityAutocomplete(array &$element, FormStateInte
   +        $has_multiple = (sizeof(explode(',', $input)) > 1) ? TRUE : FALSE;
   

   I think this should use \Drupal\Component\Utility\Tags::explode instead of just regular explode

2. +++ b/core/lib/Drupal/Core/Entity/Element/EntityAutocomplete.php
   @@ -240,8 +240,14 @@ public static function validateEntityAutocomplete(array &$element, FormStateInte
   +          $form_state->setError($element, t('This element can only have one value'));
   

   we can return here and avoid the else to reduce complexity

I agree - this sounds like a genuine bug - we should be able to reproduce it in a test

I have edit the patch. When you have references for autocomplete that contain a comma (like node title) previous patch will not allow it.
When Drupal detects a comma in a reference it will put quotes around the hole thing. So now we have a regex that will see if a comma is in the value that is outside of quotes.

Example
This wil be oké
value -> "title part1,2,3 (node_ID)"

This is not
value -> "title part1,2,3 (node_ID)", "title part 4,5,6 (node_ID)"

In previous patch
value -> "title part1,2,3 (node_ID)" Trows Error message

Reapplying patch.

+++ b/core/lib/Drupal/Core/Entity/Element/EntityAutocomplete.php
@@ -240,8 +240,17 @@ public static function validateEntityAutocomplete(array &$element, FormStateInte
+        //$has_multiple = (sizeof(explode(',', $input)) > 1) ? TRUE : FALSE;
+        // instead of checking for comma we check if the field has comma's outside of quotes
+        // drupal puts quotes around references that have comma's in them

why are we commenting this ?

Hi,

Thank you all, after applying the patch, When trying to add multiple values in comma separated instead of showing validation error it throws fatal error, on Drupal core 8.9.20

InvalidArgumentException: Placeholders must have a trailing [] if they are to be expanded with an array of values. in Drupal\Core\Database\Connection->expandArguments() (line 744 of /Applications/MAMP/htdocs/core/lib/Drupal/Core/Database/Connection.php)

Thanks
Karthikeyan

Here's the patch for 9.3.0

This is an interesting issue because, as pointed out in #12, the code spells out this behavior:

+++ b/core/lib/Drupal/Core/Entity/Element/EntityAutocomplete.php
@@ -286,8 +286,15 @@ public static function validateEntityAutocomplete(array &$element, FormStateInte
       // Use only the last value if the form element does not support multiple
       // matches (tags).

The patches above address item #2 in the IS, which might not even be classified as a bug since it is the behavior specified in code. I think we all agree the current situation is bad UX though. But if we add this validation though we clearly must update the comment too.

+++ b/core/lib/Drupal/Core/Entity/Element/EntityAutocomplete.php
@@ -286,8 +286,15 @@ public static function validateEntityAutocomplete(array &$element, FormStateInte
+        $has_multiple = preg_match('/(,)(?=(?:[^"]|"[^"]*")*$)/', $input) ? TRUE : FALSE;

I also think that the regex in #26 should be removed in favor of calling Tags::explode and then calling count on the result. We might as well re-use the existing code that does this.

As far as item #1 in the IS, there's nothing in any of the patches that addresses that. That would probably involve some changes to the core/drupal.autocomplete library.

I would think that we could split those up into separate issues if we wanted. My guess is that messing with core/drupal.autocomplete could get really nasty.

Just tossing in my test results and comments:
- tested with core 9.2.16 and patch from #20 (#26 doesnt apply)

without patch:
- only the LAST item on the list is passed to my form submit handler

with patch:
- if i enter 2 items i get a form validation error

this is independent of setting #tags

IMHO, the proper functionality here is:
- if something is set on the form (#tags seems like an odd attr to define this), nothing should autocomplete past the 1st item - i think currently missing
- if user just enters values manually without aide of autocomplete; then validation error (as the patch does now)
- if single attr is not set; then all items should be passed to form submit - not working now

are these the goals here?

The patch in #25 does not work because of a malformed RegEx. My patch uses Tags::explode instead.

+1 Just stumbled into this ... IMHO it makes more sense to prevent completing multiple values client side in the first place. Just adding serverside validation is good but will just raise questionmarks in the opposite direction.

EDIT: looked a bit further and as the autocomplete js code will be replaced soon https://www.drupal.org/project/drupal/issues/3466088 my comment doesnt worth mention here

Hi,

I made a patch that is working with D11.

I added tests to reproduce the issue, and applied patch for it.

Could use a summary update to include the steps to rerpoduce