Bug #73125
closed
Story #69617: FormEngine bugs
500 Error in GroupElement.php due to group db fields in flexform containers
100%
Description
There is a bug in the typo3/sysext/backend/Classes/Form/Element/GroupElement.php on line 278. I think it is only happening with nested flexforms.
if (!$this_table && $onlySingleTableAllowed) {
$this_table = $allowed[0];
}
$itemArray[] = array('table' => $this_table, 'id' => $this_uid);
if (!$disabled && $show_thumbs) {
$rr = BackendUtility::getRecordWSOL($this_table, $this_uid);
$this_table should be a string as expected from getRecordWSOL, but as it is coming from a trimExplode it is actually a array. On line 55 you can see.
$allowed = GeneralUtility::trimExplode(',', $config['allowed'], true);
A simple array_shift would solve this issue.
This issue is hiding another issue with container elements in flexform using group-> db fields. The elements in the containers are not being prepend with the table name and this cause later a problem.
Example:
<T3DataStructure>
<meta>
<langDisable>1</langDisable>
</meta>
<ROOT>
<type>array</type>
<el>
<settings.container>
<TCEforms>
<label>Download Container</label>
</TCEforms>
<section>1</section>
<type>array</type>
<el>
<item>
<type>array</type>
<label>New Document</label>
<title>New Document</title>
<el>
<document>
<TCEforms>
<label>Document</label>
<config>
<type>group</type>
<internal_type>db</internal_type>
<allowed>tx_dms_domain_model_document</allowed>
<size>1</size>
<maxitems>1</maxitems>
<minitems>1</minitems>
</config>
</TCEforms>
</document>
<languageContainer>
<TCEforms>
<label>More Languages Documents</label>
</TCEforms>
<section>1</section>
<type>array</type>
<el>
<item>
<type>array</type>
<label>New Localization</label>
<title>New Localization</title>
<el>
<document>
<TCEforms>
<label>Document</label>
<config>
<type>group</type>
<internal_type>db</internal_type>
<allowed>tx_dms_domain_model_document</allowed>
<size>1</size>
<maxitems>1</maxitems>
<minitems>1</minitems>
</config>
</TCEforms>
</document>
</el>
</item>
</el>
</languageContainer>
</el>
</item>
</el>
</settings.container>
</el>
</ROOT>
</T3DataStructure>
This produces a pi_flexform like
<?xml version="1.0" encoding="utf-8" standalone="yes" ?>
<T3FlexForms>
<data>
<sheet index="sDEF">
<language index="lDEF">
<field index="settings.container">
<el index="el">
<section index="1">
<itemType index="item">
<el>
<field index="document">
<value index="vDEF">2</value>
</field>
<field index="languageContainer">
<el index="el">
<section index="1">
<itemType index="item">
<el>
<field index="document">
<value index="vDEF">3</value>
</field>
</el>
</itemType>
<itemType index="_TOGGLE">0</itemType>
</section>
</el>
</field>
</el>
</itemType>
<itemType index="_TOGGLE">0</itemType>
</section>
</el>
</field>
</language>
</sheet>
</data>
</T3FlexForms>
And produces the error.
Updated by Philipp Gampe about 8 years ago
- Category set to FormEngine aka TCEforms
- Is Regression changed from No to Yes
Could you write a patch for this and hand it to gerrit? http://wiki.typo3.org/CWT
Updated by
Updated by Frank Nägler about 8 years ago
- Status changed from New to In Progress
- Assignee set to Frank Nägler
Updated by Gerrit Code Review about 8 years ago
- Status changed from In Progress to Under Review
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47318
Updated by Gerrit Code Review about 8 years ago
Patch set 1 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47369
Updated by Christian Kuhn about 8 years ago
Ok, this took longer than anticipated.
Marcos, the issues you're stumbling upon is only a side effect of a much deeper restriction in the core of CMS 7 and above: The system does not support nesting a section within a container section again. Usually, the TcaGroup data provider takes care that the database content of an existing field is harmonized and always of the structure "table_id|title". You're running into the buggy code section of the GroupElement only, because the data providers are not called at all for nested sections in containers. This code section in GroupElement is actually dead code - that is the reason why you can only trigger it with nested sections where the data preparation didn't happen.
Basically in 6.2 and before, nesting a container section within a container section was technically allowed, but was never really tested and had a long list of bugs for non-trivial things like elements with wizards for instance.
With the refactoring in version 7, it was decided this nesting is not supported at all anymore. The use cases seem to be extremely rare and the efforts to implement this stuff in a properly working way are massive: First, the parsing within the TcaFlexPrepare data provider has to be changed (there is a method that does the parsing that is pretty complex already). But that is the easy part: Afterwards, the path creation within the render part of FormEngine has to be adapted to take care of this nesting. To verify all this works, ext:styleguide has first to be extended for every element to contain these nested scenario, too. I'm personally deep in this code structure, and I'd estimate at least 4 weeks of full time work to implement nested container sections in a proper way, everyone else will need more time. Considering this massive effort and the rare use cases, an implementation of nested section containers will probably not happen at the moment.
A second restriction section containers come with is that inline elements are not supported at all. This has always been the case: It is not possible to connect relations to the anonymous container element list.
Marcos, it would be great if you could have a second look at your data model and see if you could solve your issue in a different way. Maybe you could add a mm relation between your documents instead, or solve your issue on a translation level. Having a plugin flex that adds documents in a container section which then adds other related documents as a nested dependency looks as if something is wrong with the model or data abstraction at this point, which could be solved in better way somehow.
The fact that you run into this issue and then tried to fix the side effect within GroupElement made me come up with a different patch now: First, GroupElement is cleaned up to no longer contain the dead code that is taken care off within TcaGroup. Secondly - and more important - the pending patch now checks for nested sections and inline within the flex preparation data provider and throws an exception if a developer tries to configure those scenarios. This should rule out those questions in the future.
Updated by Gerrit Code Review about 8 years ago
Patch set 2 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47369
Updated by Gerrit Code Review about 8 years ago
Patch set 3 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/47369
Updated by Gerrit Code Review about 8 years ago
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/47379
Updated by Christian Kuhn about 8 years ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 8ae9f625683c86e38a757f176f8e86eb7c11dde2.
Updated by Marcos Fadul about 8 years ago
Hi Christian,
Thank you for your feedback. I realised that the issue was deeper as my fix was fixing.
It is really a pity that this function is leaving the code. I'll check what I can do to refactor the existing code.
Greetings,
Marcos
Updated by Frank Gerards about 8 years ago
+1 to Marcos,
the fact, that core devs speak of "rare use cases" makes me laugh a bit. I used this feature in about 20% of my 200+ mid- to enterprise-level webprojects and the old core had no problems with recursion. Why does the 7.x core have that limitation ? I always was happy, that a system like a CMS takes theoretically endless recursions into consideration, as this is theoretically the case for rootline-calculations etc.
So the fact is, "modern" coding/coders go down a road that abandons core principles of an enterprise-level system ? Not good at all.
Updated by Christian Kuhn almost 8 years ago
Frank, I did not state nested section containers can not be done.
The situation is that the list of bugs triggered by nested section containers is long, and it always has been long before - in 6.2 and before only very simple tca elements worked more or less without problems in nested sections, as soon as you played around with more complex stuff like wizards, tca tree, rte or others, there was trouble ahead. Even simple stuff like the element validation has always been broken.
The refactoring of this code fixed an enormous list of bugs that have always been in the code base, a lot of new features are implemented and the formEngine is much more stable than ever before in a much wider bandwidth of combinations.
Throwing this exception says that nesting elements is currently not implemented and is a fair solution towards developers who would otherwise step into a list of issues when trying to use such a construct.
An implementation can be done, but it comes with some effort and currently nobody is working on an implementation. I'd be happy if anyone steps in and I'll happily help with a kick off to explain what needs to be done and will give some starting points. Contact me (via slack) if anyone can find the time to do so.
Updated by Bernhard Kraft almost 7 years ago
Christian Kuhn wrote:
[...] only very simple tca elements worked more or less without problems in nested sections
I have to counter that. For me nested sections worked quite well. I used them a lot in good ol' times of TemplaVoila where FCE's have been introduced!!! They worked very well and only since FAL (inline records) have been introducted there have been cross-issues between both approaches (flex XML vs. record based nesting).
The issue became prominent when FAL was introduced and people tried to use FAL images inside flux repeating sections. Like for having an element where one can add slides of text+image.
I solved this issue about a year ago by the following core patch:
https://review.typo3.org/#/c/39050/
Some minor tweaks to EXT:vhs for retrieving the images. This worked quite well for me but never made it into the core. I don't know whether dropping the feature is the right decision in this case. Of course I see the additional effort it takes to keep the code working but just declaring something which "should" obviously work as "unsupported" makes the whole product a little bit more lame :/
... open for suggestions.
Updated by Frank Gerards over 6 years ago
Bernhard Kraft wrote:
I have to counter that. For me nested sections worked quite well. I used them a lot in good ol' times of TemplaVoila where FCE's have been introduced!!! They worked very well and only since FAL (inline records) have been introducted there have been cross-issues between both approaches (flex XML vs. record based nesting).
The issue became prominent when FAL was introduced and people tried to use FAL images inside flux repeating sections. Like for having an element where one can add slides of text+image.
+1 for that aspect.
The fact, that introducing a FAL module in the core, which prohibits the use of nested Flexforms is a bad design desicion and limits the way contentelements can be created with
- templavoila
- templavoilaplus
- FluidTYPO3
etc. - so it would be great to finally FIX the FAL issues, so we can start using TYPO3 7.6+ as intended and not abandon a whole bunch (30+) FCEs with nested sections.
Updated by