Bug #75493
closedstdWrap properties executed twice (patch provided)
100%
Description
The method stdWrap of class \TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer includes a check to prevent duplicate execution of sub-properties for recursive stdWrap functions and cObjects. This check only words in some of the cases.
Example:
page = PAGE
page.10 = TEXT
page.10.stdWrap.append = TEXT
Result: “append” is printed twice in the TypoScript section of the admin panel.
Workaround:
page = PAGE
page.10 = TEXT
page.10.stdWrap =
page.10.stdWrap.append = TEXT
A patch is attached.
I tested the bug fix with version 6.2.19. I also compared the code of current HEAD in git.typo3.org and saw no change – so the fix should be applied to all branches.
Files
Updated by Olaf Schmidt-Wischhöfer over 8 years ago
I could also have selected version 7.6 or version 8 – please feel free to change this as appropriate.
Updated by Olaf Schmidt-Wischhöfer over 8 years ago
- Target version set to next-patchlevel
Updated by Gerrit Code Review over 8 years ago
- Status changed from New 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/48008
Updated by Nicole Cordes over 8 years ago
- File 2016-05-04_232104.jpg 2016-05-04_232104.jpg added
I don't see the issue that "append" is added twice to the TypoScript sections in Admin Panel with current master and the TypoScript you provided.
Can you please help me out to get a better understanding what is wrong here? Can you say which option in Admin Panel must be enabled and which output you get?
Thank you!
Updated by Olaf Schmidt-Wischhöfer over 8 years ago
Thanks for your response!
I have created a different test case to show the problem. I have verified it with Typo3 6.2.22, and will test it with current master either later today or on Monday.
Consider the following TypoScript setup:
page = PAGE
page.10 = TEXT
page.10.value.setCurrent = test1
page.20 = TEXT
page.20.value.current = 1
page.20.stdWrap.setCurrent = test2
The output is: test1
page = PAGE
page.10 = TEXT
page.10.value.setCurrent = test1
page.10.stdWrap.append = TEXT
page.10.stdWrap.append.value.current = 1
page.10.stdWrap.append.setCurrent = test2
The output is: test2
page = PAGE
page.10 = TEXT
page.10.value.setCurrent = test1
page.10.stdWrap =
page.10.stdWrap.append = TEXT
page.10.stdWrap.append.value.current = 1
page.10.stdWrap.append.setCurrent = test2
The output is: test1
Updated by Olaf Schmidt-Wischhöfer over 8 years ago
I can confirm that it was fixed in master with
https://git.typo3.org/Packages/TYPO3.CMS.git/commitdiff/23b63c2bbf87cd104fd42dca84cb861db0f55f47
The fix is also included in 7.6 and 8.1, but not in 6.2.22.
Updated by Wouter Wolters over 8 years ago
http://review.typo3.org/32466 if you read the comments here it was decided to not backport this fix to the 6.2 branch. It was seen as a breaking change.
Updated by Olaf Schmidt-Wischhöfer over 8 years ago
Thanks, I can understand about the breaking change.
However, a close reading of the code shows that the the commit linked above fixed 1 symptom of the problem, but not a related problem:
page = PAGE
page.10 = TEXT
page.10.value = 1+1
page.10.value.prioriCalc =
page.10.value.prioriCalc.wrap = 1
page.10.value.prioriCalc.required = 1
Result: 1+1
page = PAGE
page.10 = TEXT
page.10.value = 1+1
page.10.value.prioriCalc.wrap = 1
page.10.value.prioriCalc.required = 1
Result: 2
The reason for this is that the following condition will always evaluate to true:
$functionType !== 'boolean' || $conf[$functionName]
in function TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer->stdWrap, line 2105
(If $conf[$functionName] is unset, then $functionType is always 'array'.)
Unfortunately, my commit also fails to fix this issue, since I accidentally committed the wrong revision. The new line needs to read:
$functionType = $this->stdWrapOrder[$functionName];
instead of:
$functionType = $this->stdWrapOrder[$stdWrapName];
Should I commit a corrected version of my bugfix?
And should I open a new issue, or is it somehow possible to rename this one?
If the bugfix is accepted, then the following check is no longer needed:
https://git.typo3.org/Packages/TYPO3.CMS.git/commitdiff/23b63c2bbf87cd104fd42dca84cb861db0f55f47
It might also be possible (not sure about this) to remove the “isExecuted” check in:
function TYPO3\CMS\Frontend\ContentObject\LoadRegisterContentObject->render
Updated by Wouter Wolters over 8 years ago
Hi Olaf, I'm not so deep in this logic. In my opinion you can update your existing patch with your proposal. Discussion will probably follow after that. You can keep this ticket number.
Updated by Gerrit Code Review over 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/48008
Updated by Gerrit Code Review over 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/48008
Updated by Olaf Schmidt-Wischhöfer over 8 years ago
Simplified test-case:
page = PAGE
page.10 = TEXT
page.10.value = 1+1
page.10.value.prioriCalc.wrap =
Result (without bug fix): 2
Workaround:
page = PAGE
page.10 = TEXT
page.10.value = 1+1
page.10.value.prioriCalc =
page.10.value.prioriCalc.wrap =
Result: 1+1
Updated by Gerrit Code Review over 8 years ago
Patch set 4 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008
Updated by Gerrit Code Review over 8 years ago
Patch set 5 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008
Updated by Olaf Schmidt-Wischhöfer over 8 years ago
Another symptom of this bug:
page = PAGE
page.10 = TEXT
page.10.value.cObject.wrap = TE|XT
page.10.value.cObject.value = 2
Output: TE2XT
Please notice that page.10.value.cObject itself is nowhere defined.
The wrap TE|XT is evaluated twice. First it is used for the value of page.10.value.cObject (= TEXT), and then it is used for the output as well.
My bug fix solves this problem as well.
Updated by Olaf Schmidt-Wischhöfer over 8 years ago
Even stranger – the following example allows the website visitor to select the content element that will be rendered:
page = PAGE
page.10 = TEXT
page.10.value.cObject.stdWrap.dataWrap = |{gp:sword}
page.10.value.cObject.value = Other text<br>
page.10.value.cObject.template = TEXT
page.10.value.cObject.template.value = <f:form><f:form.textarea /></f:form>
If the URL argument sword=FLUIDTEMPLATE is given, then the output is: <textarea name=""></textarea>FLUIDTEMPLATE
If the URL argument sword=TEXT is given, then the output is: Other text<br>TEXT
If the argument sword is missing, then the output is empty.
The stdWrap properties of type functionName are immune to this bug, because $stdWrapOrder['postUserFunc.'] etc. (with dot) is not defined.
Updated by Gerrit Code Review over 8 years ago
Patch set 6 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008
Updated by Gerrit Code Review over 8 years ago
Patch set 7 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008
Updated by Gerrit Code Review over 8 years ago
Patch set 8 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008
Updated by Gerrit Code Review over 8 years ago
Patch set 9 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008
Updated by Gerrit Code Review over 8 years ago
Patch set 10 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008
Updated by Gerrit Code Review over 8 years ago
Patch set 11 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008
Updated by Gerrit Code Review over 8 years ago
Patch set 12 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008
Updated by Gerrit Code Review over 8 years ago
Patch set 13 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008
Updated by Gerrit Code Review over 8 years ago
Patch set 14 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008
Updated by Gerrit Code Review over 8 years ago
Patch set 15 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008
Updated by Gerrit Code Review over 8 years ago
Patch set 16 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008
Updated by Gerrit Code Review over 8 years ago
Patch set 17 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008
Updated by Gerrit Code Review over 8 years ago
Patch set 18 for branch master of project Packages/TYPO3.CMS has been pushed to the review server.
It is available at https://review.typo3.org/48008
Updated by Olaf Schmidt-Wischhöfer over 8 years ago
- Status changed from Under Review to Resolved
- % Done changed from 80 to 100
Applied in changeset a4897da0aadfac0432e610e370e84ddc6ebfe871.