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

Disabled settings trigger errors upon saving #2210

Closed
leofeyer opened this issue Aug 28, 2020 · 11 comments · Fixed by #2261
Closed

Disabled settings trigger errors upon saving #2210

leofeyer opened this issue Aug 28, 2020 · 11 comments · Fixed by #2261
Labels
Milestone

Comments

@leofeyer
Copy link
Member

Affected version(s)

Contao 4.10

Description

Since #1382, settings defined in the app configuration are disabled in the back end. When trying to save the form, however, these fields trigger errors:

@leofeyer leofeyer added the bug label Aug 28, 2020
@leofeyer leofeyer added this to the 4.10 milestone Aug 28, 2020
@ausi
Copy link
Member

ausi commented Aug 29, 2020

Adding $GLOBALS['TL_DCA']['tl_settings']['fields'][$field]['eval']['mandatory'] = false; to

$GLOBALS['TL_DCA']['tl_settings']['fields'][$field]['eval']['disabled'] = true;
$GLOBALS['TL_DCA']['tl_settings']['fields'][$field]['eval']['helpwizard'] = false;
$GLOBALS['TL_DCA']['tl_settings']['fields'][$field]['xlabel'][] = [self::class, 'renderHelpIcon'];
fixed the issue in my test.

@fritzmg
Copy link
Contributor

fritzmg commented Aug 29, 2020

But that's not really how disabled fields are supposed to work, is it? 🤔

@ausi
Copy link
Member

ausi commented Aug 29, 2020

I don’t know. Setting a disabled field to mandatory could be seen as a misconfiguration I guess?

@fritzmg
Copy link
Contributor

fritzmg commented Aug 30, 2020

Yeah, I guess you are right.

@ausi
Copy link
Member

ausi commented Aug 30, 2020

But intuitively I would also expect the disabled option to take precedence over all other settings.

To make this work we could change

if (!$this->mandatory)
{
return '';
}
to if (!$this->mandatory || !$this->blnSubmitInput)

@leofeyer
Copy link
Member Author

leofeyer commented Sep 2, 2020

Should we not return immediately if we know that the widget does not submit anything?

	protected function validator($varInput)
	{
		if (!$this->submitInput())
		{
			return null;
		}

		if (\is_array($varInput))

@leofeyer
Copy link
Member Author

leofeyer commented Sep 2, 2020

And is this not the actual cause of the error?

// Validate and save the field
if (\in_array($this->strInputName, $paletteFields) || Input::get('act') == 'overrideAll')
{
$objWidget->validate();
if ($objWidget->hasErrors())
{
// Skip mandatory fields on auto-submit (see #4077)
if (!$objWidget->mandatory || $objWidget->value != '' || Input::post('SUBMIT_TYPE') != 'auto')
{
$this->noReload = true;
}
}
elseif ($objWidget->submitInput())

Why validate a widget that does not submit any input?

@ausi
Copy link
Member

ausi commented Sep 2, 2020

I honestly don’t know for what submitInput() is meant to be used for. Maybe the widget could get the value from somwhere else but still needs to be validated and saved?

I think I would just extend the check in

this should most likely not break existing code.

@aschempp
Copy link
Member

aschempp commented Sep 7, 2020

I don't think returning on empty content is correct (as suggested by @ausi). We must not validate a widget that does not submit input (which is the case if it is disabled, or the widget has only presentational purpose).

@leofeyer
Copy link
Member Author

leofeyer commented Sep 7, 2020

That is what I think, too.

@ausi
Copy link
Member

ausi commented Sep 7, 2020

I just looked at this again: If $objWidget->submitInput() is false the save method is never called. So I think @aschempp is right “We must not validate a widget that does not submit input” 👍

@leofeyer leofeyer linked a pull request Sep 9, 2020 that will close this issue
@leofeyer leofeyer closed this as completed Sep 9, 2020
leofeyer added a commit that referenced this issue Sep 9, 2020
Description
-----------

| Q                | A
| -----------------| ---
| Fixed issues     | Fixes #2210
| Docs PR or issue | -

Commits
-------

2f83d0a Do not validate widgets that do not submit input
AlexejKossmann pushed a commit to AlexejKossmann/contao that referenced this issue Apr 6, 2021
Description
-----------

| Q                | A
| -----------------| ---
| Fixed issues     | Fixes contao#2210
| Docs PR or issue | -

Commits
-------

2f83d0a Do not validate widgets that do not submit input
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 18, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants