OptionsetField_holder.ss is not conform with template changes introduced in 3.2

[anselmdk]

OptionsetField_holder.ss is still using id="$Name" whereas FormField_holder.ss is using id="$HolderID" as was introduced in 3.2 I believe.
This causes non-consistent holder naming (see image) and thus breaks modules such as display logic (cc @unclecheese).
I'm not sure if this is by design (as it might have other implications?) or an oversight.

[anselmdk]

@unclecheese just a note that I went away from using the checkboxsetfield for display logic, as it just felt too buggy. I'm instead using individual checkboxes now, which works just fine.

Above still feels a little inconsistent however.

[tractorcow]

Yes I think it needs to be fixed; I'm not happy to leave it as id="$Name" but it would be hard to change it without breaking existing 3.x.

Can we fix this in 4.x?

[dhensby]

This is a bug, surely. Not a feature... I'd fix in 3.2

[tractorcow]

It can't be fixed in 3.2 without change in behaviour. Yes it's a bugfix, but semver requires it to be fixed without regressions. We've already broken a lot of templates in the new form IDs, which we promised we wouldn't under semver.

[dhensby]

I thought we had an "old form markup" config fallback?

[tractorcow]

Not that I recall!

[tractorcow]

A whole bunch of changes have been merged with #5826.

I've applied this fix to both templates (one for default and cms-forms theme):

New cms-forms theme version: https://github.com/open-sausages/silverstripe-framework/blob/bdb8e4016a82129de791fead4e5daccd3079111a/admin/themes/cms-forms/templates/forms/OptionsetField_holder.ss

Legacy (front-end theme) version: https://github.com/open-sausages/silverstripe-framework/blob/bdb8e4016a82129de791fead4e5daccd3079111a/templates/forms/OptionsetField_holder.ss

[dhensby]

Not that I recall!

yep: FormTemplateHelper_Pre32 is what you need for this old (broken) behaviour. so users should be using that if they want the broken markup and using the default otherwise