Please: Which field ist to add to the xml-file?

Updated comment, it was not displayed

there also another fix for this #7335
but I also have a better idea (from my point of view ), just need some time...

Note:
<field name="foobar" multiple="true" required="true" /> is incorrect test for "multiple select", as it produce <input name="foobar[]" type="text" /> that has different behavior from <select multiple>

and, if your target is test such text fields for "required", then I would suggest change your code to:
if (($value === '') || ($value === null) || (is_array($value) && !count($value)))

for such text field browser send array(""), so count($value) will be always true

This example with <field name="foobar" multiple="true" required="true" /> was an example for the current behavior in Joomla! it was never outputed, just an example, that the "require" test fail, when the multiple value is empty.
But now as you say it: array("") should also handled as "empty" when the multiple field is required, any suggestions?

Or is there a way, that we can merge your patch and my patch to find a complete solution for empty multiple values and the required attribute for every combination?

technically array("") is not empty
in theory this can be tested by empty($value[0]) but I would stay away from such thing in the core, as it can make unexpected surprise for the extension developers that has a custom fields ..

I would suggest you to review what you tries to achieve here, if the problem with "multiselect", then it because it caused by that #2617 pull that force array instead of 'null' (that is wrong, from my point of view)

If you want to test for required the multiple text field <field name="foobar" type="text" multiple="true" required="true" /> ... then I not sure .... I think if you have such field in your component, then will be more simple to make special validation rule for this field.

You're right, array("") is not empty() - empty, but the field we submitted was empty. So if we ask: "was there any value submitted?" the answer is "no".

Regardless of your patch (which I think should be definitely in the core) I also think, that we could kill two problems here and the question is: do we do it with your patch or should I improve mine?

#7381 do not tries to fix the validation, it about check whether all fields was submitted and if some field is missed then set it as null (empty) ... instead of array (for multiple) as in #2617

I think better do it separately, if you have an idea about validation

actually, if #7381 will be rejected, your pull still valid for case :

<field type="checkboxes" name="checkboxes" label="Checkboxes" required="true">
    <option value="1">1</option>
    <option value="2">2</option>
    <option value="text">text</option>
</field>
<field type="list" name="list" label="List" multiple="true"  required="true">
    <option value="1">1</option>
    <option value="2">2</option>
    <option value="text">text</option>
</field>

that is broken currently,

only thing that I would change here is to be sure that $value is array,

if (($value === '') || ($value === null) || (is_array($value) && !count($value)))

because no guaranty that some custom field with the attribute "multiple" will not send string

@Fedik I updated the patch, now array('') is invalid, too (like the default check). Could you please look again.

looks good, but I still have a doubt that we need to check for array("") (empty values, inside array),
better if someone from PLT will make review

Other thought: if we already check that $value is array, then maybe we do not need to check that field is $multiple, at all (cause still possible to make the custom field that will be not multiple, but still submit array())

@Fedik That "strlen" is a very good idea, thanks for that.
I still think, an empty input field (array('')) is invalid, but yeah, some other opinions would be good.

ok, I tried make some tests with couple custom fields.

• FieldTest1 - some simple "repeatable" field emulation, multiple.
• FieldTest2 - some complex field emulation, with color (simple text inpup) and caption, not multiple.

For repeat my test.
Place these fields in some form xml, (I use Protostar parameters form templates/protostar/templateDetails.xml):

<field name="test1" type="test1" multiple="true" required="true" label="FieldTest1" />
<field name="test2" type="test2" required="true" label="FieldTest2" />

then make two field classes and place them in to libraries/cms/form/field,
FieldTest1 libraries/cms/form/field/test1.php:

<?php
/**
 * test field
 * @link   https://github.com/joomla/joomla-cms/pull/7372
 */
class JFormFieldTest1 extends JFormField
{
    /**
     * The form field type.
     */
    protected $type = 'Test1';

    /**
     * Method to get the field input markup.
     */
    protected function getInput()
    {
        $required = ''; //$this->required ? ' required aria-required="true"' : '';

        $html = array();
        for($i = 0; $i < 3; $i++ ){
            $value = empty($this->value[$i]) ? '' : $this->value[$i];
            $html[] = '<input type="text" name="' . $this->name . '" id="' . $this->id . $i . '" value="' . $value . '" '.$required.'/>';
        }

        return implode($html);
    }

}

FieldTest2 libraries/cms/form/field/test2.php:

<?php
/**
 * test field
 * @link   https://github.com/joomla/joomla-cms/pull/7372
 */
class JFormFieldTest2 extends JFormField
{
    /**
     * The form field type.
     */
    protected $type = 'Test2';

    /**
     * Method to get the field input markup.
     */
    protected function getInput()
    {
        $required = ''; //$this->required ? ' required aria-required="true"' : '';

        $color   = empty($this->value['color']) ? '' : $this->value['color'];
        $caption = empty($this->value['caption']) ? '' : $this->value['caption'];

        $html = array();
        $html[] = '<input type="text" name="' . $this->name . '[color]" id="' . $this->id . '-color" placeholder="Color #fff" value="' . $color . '" '.$required.'/>';
        $html[] = '<input type="text" name="' . $this->name . '[caption]" id="' . $this->id . '-caption" placeholder="Color caption" value="' . $caption . '" '.$required.'/>';
        return implode($html);
    }

}

And then try save the form with this fields (with empty values).
before patch: form saved without warning that field is required
after patch: form warning that FieldTest1 is required, but still no warning that FieldTest2 is required

test passed on 50%, and faill on FieldTest2

Note: with required I mean at least ONE input from the fields collections has the value

Hello @Fedik thank you for your example.

I think, the problem with your example ist, that the "Joomla! default" is using a field element (named "test2") wich has two fields (color + caption)

@bembelimen first "test1" field use 3 field element (that can name as [0] [1] and [2]) ...
difference between them ("test1" and "test2") that first has the attribute "multiple" and second does not ...

if you replace
if ($value === '' || $value === null || ($multiple && is_array($value) && !count($value)))
to
if ($value === '' || $value === null || (is_array($value) && !count($value)))
then both field will work.

but, was just example

sorry, i had a typo, i ment "fields": <fields name="field2"><field name="color" /><field name="caption" /></fields>

yes you are right, for my "test2 example" can use two separated field,
but, as I said it was just a simple example of possible "complex field", that no realy "multiple"... can be some dynamically(by javascript) changed thing

You're (as always) right, but I think we cannot (or shouldn't try) to handle such specific cases, because you can always nest your fields with custom fields (fieldname[foo][bar][baz]) which cannot be caught with the default validation when the default XML structure is not used (fields group)

So back to the problem: could you please test if my patch fixes the multiple + require problem? I would be really grateful.

I tried explain that better do check the value depend from value type (if array then use count()), and not depend from if field multiple or not ... but maybe it more personal preference ...

if this is not important, then test is good

Tested and tested successfull. Can be merged as far as I'm concerned.

Thanks. RTC

I tried it today and couldn't reproduce the error, seems it is fixed with another PR

I have tested this item unsuccessfully on 4886723

How did you test it?

It works before applying the patch...

I have tested this item unsuccessfully on 4886723

unsuccessful because it works also without the patch

I have tested this item unsuccessfully on 4886723

Not able to reproduce the error. Also works without the patch applied.

The patch #7381 change the behavior of the validation, so the test above is not valid, but the problem still exists.
Now you have to output the text-field in the artice view (components/com_content/views/article/tmpl/default.php)

$this->form->getField('foobar');

And submit the form without filling this text field.
Saving should work, though the multiple field is required and empty

@bembelimen please provide easy to follow test instructions and what this fix is going to fix.

@bembelimen Thank you for your contribution.

The last comment here was on 25 October. Can you please follow-up on this issue? If not this will be closed within 4 weeks

Thanks for understanding!

Closed as per comment above - it can always be reopened if updated