Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disables html5 validation #267

Open
wants to merge 1 commit into
base: 2.x
Choose a base branch
from

Conversation

markconroy
Copy link
Member

Copy of to use here instead of there.

@ekes
Copy link
Member

ekes commented Jan 20, 2025

Immediate thought: To install most localgov_ modules you have to have this module. This disabled HTML5 validation on all forms. Plenty of sites have been extend to have non-localgov_ modules installed, could any of these modules, or things otherwise built into the site, be expecting HTML5 validation to be enabled?

@andybroomfield
Copy link
Contributor

Suggest we add a setting

$settings['localgov_enable_html5_validation'] = TRUE;

and detect it using

if (Settings::get('localgov_enable_html5_validation') !== TRUE) {
... remove html5 validation.

This is because as above there may be cases where modules would expect html5 validation to be present, or may not have any other validation. So whilst we should encourage the GDS recommendation of using validation errors that allow for more descriptive text, if there are cases where this isn't set up correct there is a chance to opt out of this change.

@finnlewis
Copy link
Member

Or we could just use the module which allows people to turn it off easily.

https://www.drupal.org/project/disable_html5_validation

Then we enable it on a default install, but it is optional for existing installs.

Thoughts @andybroomfield @ekes @markconroy ?

@finnlewis
Copy link
Member

Discussing in Merge Tuesday, general agreement that it would be good to make it optional.

@andybroomfield mentioned that in general it would be good to reduce dependencies.

I'm in favour of using the module!

hmmm.... to be discussed further....

@markconroy
Copy link
Member Author

I have no preference for doing this via the contrib module or via a custom hook.

It's surprising that Drupal core has such good form validation, but doesn't disable it by default. So maybe we should go with the module approach to follow that practice.

@ekes
Copy link
Member

ekes commented Jan 21, 2025

I'm generally not in favour of adding modules for one line that you could include elsewhere. OK maybe more than generally. I can't think of another occasion I think it's a good idea. But in this case. It feels like it makes sense. It is explicit. It's easy for end UI admins to understand.

@andybroomfield
Copy link
Contributor

Discussed on MT on 28/1/2025. We thought that switching off the module is better as it allows site administrators to turn it off if they encountered a problem. We will install the module in the profile and then switch it on by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants