Add custom setForm() method for ConfirmedPasswordField

[purplespider]

Fixes bug that makes it impossible to set a ConfirmedPasswordField as "required" due to the two child fields not getting a form assigned to them.

Issue

Adding the names of a ConfirmedPasswordField field to a RequiredFields form validator would have no effect. e.g. The fields would not be marked as required.

This is because the standard setForm() method in FormField sets the form on the parent ConfirmedPasswordField but not on either the two child PasswordFields.

Fix

This PR adds a custom setForm() method to ConfirmedPasswordField which sets the form on the child fields as well as the parent. So the following validator will now work as expected:

$required = new RequiredFields('Password[_Password]', 'Password[_ConfirmPassword]');`

[dhensby]

@purplespider so does $required = new RequiredFields('Password'); not work? That is what I'd expect the correct syntax to be rather than $required = new RequiredFields('Password[_Password]', 'Password[_ConfirmPassword]');? The password fields shouldn't really need addressing individually in my view

[purplespider]

@dhensby Correct. Just 'Password' doesn't work, and I agree that is what I'd expect too.

Not sure how to achieve this though. I noticed that when the form is rendered, FormField->Required() only gets called on the two child fields, e.g. 'Password[_Password]' and 'Password[_ConfirmPassword]'.

Also worth noting that ConfirmedPasswordField doesn't have its own template, it just renders 2x PasswordFields.

Are you aware of any other field types that consist of multiple child fields?

[dhensby]

The ConfirmedPasswordField is a bit unique, because it's not like a CompositeField that holds many independant fields, this is essentially two fields that act like one (to return a single value).

I'm pretty shocked that you can't do new RequiredFields('Password') as this seems like something that would have come up quite some time ago! When you say it doesn't "set them as required" do you mean that the validation is not performed or that something in the markup/template is not set?

[purplespider]

Sorry Dan, yeh you're right. I've run some tests on a fresh Silverstripe install, and it does half work:

When using new RequiredFields('Password'):

• Server-side validation does work.

However:

• HTML 5 validation does not work. (e.g. required="required isn't added to the password fields)
• $Required in the template (e.g. in FormField_holder.ss) doesn't return true. So the password fields don't get marked as "Required".

[dhensby]

Ok, makes sense - so with this fix and using new RequiredFields('Password'), do you get the expected / required behaviour in the front end?

The alternative is to find a way for the nested password fields to see their "parent's" required state, but I'm not sure there's any reference to do that? It looks like it won't and we would need to update the public function Required() to look for it's parent's name instead of its own...

[purplespider]

Ok, makes sense - so with this fix and using new RequiredFields('Password'), do you get the expected / required behaviour in the front end?

Nope. This fix plus $required = new RequiredFields('Password[_Password]', 'Password[_ConfirmPassword]'); is necessary for the expected behaviour (HTML5 validation & "Required" label).

It looks to me like the two "child" fields are effectively added to the form field list, so there is no concept of the parent 'Password' field, meaning Required only gets checked for the 2 child fields, and I don't see a way to get the parent from a child field. 🤔

[dhensby]

I remember having all kinds of trouble around these fields in the past because they just aren't built quite right. As you say, they don't really have a way to know they are required because I don't think there's really a way to set requiredfields to know that all children should be required if the parent "wrapping" field is.

I wonder if we either need to update https://github.com/silverstripe/silverstripe-framework/blob/4/src/Forms/RequiredFields.php#L204-L207 or updated ConfirmedPasswordField logic around the Required().

[GuySartorelli]

Closed in favour of #11099