Project

General

Profile

Actions

Bug #75803

closed

7.6 only - some FormEngineValidation checks are not adding has-error class (which also results in save ignoring error)

Added by Andreas Allacher almost 8 years ago. Updated over 7 years ago.

Status:
Closed
Priority:
Must have
Assignee:
-
Category:
FormEngine aka TCEforms
Start date:
2016-04-20
Due date:
% Done:

0%

Estimated time:
TYPO3 Version:
7
PHP Version:
Tags:
Complexity:
Is Regression:
No
Sprint Focus:

Description

I think this is fixed in current master due to this change:
https://git.typo3.org/Packages/TYPO3.CMS.git/commit/003f99038a36c5d9e71ba44bd527246aa2d14fcd
in typo3/sysext/backend/Resources/Public/JavaScript/FormEngineValidation.js

But in 7.6 FormEngineValidation.js adds the has-error class still in each individual error check.
However, that isn't done everywhere.
e.g. inline minitems / maxitems check not added at all and select minitems too.

However, this also results in the error being ignored on save.

Actions #1

Updated by Andreas Fragner almost 8 years ago

The patch provided by Andreas Allacher breaks flexforms type "select" renderType multipleSideBySide - the icons for ordering and removing won't work any more.

Actions #2

Updated by Andreas Allacher almost 8 years ago

Did you copy both files or only FormEngineValidation.js
If used in 7.6 have you tried to apply the patch as patchset (especially FormEngineValidation.js) instead of copying the files - there might have been further changes in master.

Actions #3

Updated by Andreas Fragner almost 8 years ago

Yes I copied both files.
No not tried as patchset - it was working when using files from Typo3-8 branch

Actions #4

Updated by Frank Nägler almost 8 years ago

  • Status changed from New to Needs Feedback

Hi Andreas,
I can't reproduce the problem with your description, also I can't see a reason why the mentioned commit should solve an issue like this.

Can you please give me more detailed information about the problem and provide an example TCA to reproduce the problem?

Actions #5

Updated by Andreas Allacher almost 8 years ago

The commit fixes the behaviour because it adds has-error css class the to the field itself everytime the value
markParent was set to true.

Which was not the case prior.

For instance if you have a TCA configuration like:

<?php
$GLOBALS['TCA']['test_table']['columns']['test_column'] = [
    'config' => [
        'type' => 'select',
        'renderType' => 'selectMultipleSideBySide',
        'minitems' => '1,
        'items' => [
            ['foo', 'foo'],
            ['bar', 'bar']
        ]
    ]
]

And you haven't selected an item, you will be able to store the data and the tab will have an error mark but not the field itself, the reason:
https://git.typo3.org/Packages/TYPO3.CMS.git/blob/refs/heads/TYPO3_7-6:/typo3/sysext/backend/Resources/Public/JavaScript/FormEngineValidation.js#l296

Look at line 307 there. In comparison to 314.

That is not the only place where the corresponding class is not appended, e.g. type group and inline don't add the class at all.

Actions #6

Updated by Andreas Allacher almost 8 years ago

I re-checked there are also some instances where the validation might not work in master either, e.g. SelectSingleElement maxitems = 1 also excepts a selected value that is empty (empty string), e.g. if one wants to create an option like "Please choose".
Although that might be the correct behaviour as one just has to add 'eval' => 'required' to achieve that check but only if one uses strings (as zero would not be an allowed value with required).

Of course if there is not please choose option there always will be a selected value but it might accidentally be the default value instead of the wanted value but that might be a feature request instead.

Actions #7

Updated by Andreas Allacher almost 8 years ago

Just further information: using range and lower would also support disallowing 0 (as long as there are no values lower). So having minitems with a single select can be achieved by using other evals.

But that in 7.6 no error is displayed / checked for certain fields, see comment #5 ( https://forge.typo3.org/issues/75803#note-5 )
for details has to be fixed

Actions #8

Updated by Frank Nägler almost 8 years ago

  • Status changed from Needs Feedback to Accepted
Actions #9

Updated by Gerrit Code Review almost 8 years ago

  • Status changed from Accepted to Under Review

Patch set 1 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48336

Actions #10

Updated by Gerrit Code Review almost 8 years ago

Patch set 2 for branch TYPO3_7-6 of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48336

Actions #11

Updated by Frank Nägler over 7 years ago

  • Status changed from Under Review to Needs Feedback
  • Assignee deleted (Frank Nägler)

it lloks like the bug is fixed with https://review.typo3.org/#/c/48417/3 can you confirm?

Actions #12

Updated by Andreas Fragner over 7 years ago

It's fixed in Typo3 7.6.10 - I tested yesterday.

Actions #13

Updated by Wouter Wolters over 7 years ago

  • Status changed from Needs Feedback to Closed

Thanks for reporting so quickly. I close the ticket.

Actions

Also available in: Atom PDF